[malibio]

Part of #3453 (its project.status item; the issue stays open for the rest).

What changed

project.status was a plain String in the wire structs while task.status was an enum, although both are user-extensible. ADR-086 §4 says an extensible enum is an enum with a user-value variant, so project now follows task.

• ProjectStatus in nodespace-types: Planning, Active, Completed, Cancelled, User(String), with from_value / as_str / is_core and hand-written serde, mirroring TaskStatus. Its default is Planning, the schema's default.
• ProjectNode.status and ProjectNodeUpdate.status use it. DEFAULT_PROJECT_STATUS is deleted; the conversion uses the enum's default.
• The daemon's UpdateProjectNode handler and the Tauri command convert at the proto boundary, as the task path does.
• Generated TypeScript: new generated/project-status.ts, and ProjectNode.status is status: ProjectStatus. The generator's literal check covers the new enum.

The wire format is unchanged: status still travels as a bare string, and a user-defined value still round-trips.

Tests

• core_schemas.rs: project.status's coreValues equal the enum's named variants, in order, each parses to a named variant, and the schema default equals ProjectStatus::default().
• project.rs: a core value parses to its variant, a user value is kept as User and written back unchanged.
• Existing service tests (unknown value rejected, archived rejected, typed update writes) now build the update with the enum and still pass.

Verification

• bun run quality:fix clean.
• bun run test:changed passes: frontend, scripts, browser, types:check, Rust workspace.

Docs

nodespace-docs is updated alongside (committed separately, direct to main): the task and project sheets in components/node-types.md name the enums, and components/creating-node-types.md states the per-type status enum rule.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3476

Review Type: Initial Review (no prior reviews or comments on the PR)

Reviewed at: edabe921

Summary

A small, atomic change that does what it says. ProjectStatus is a line-for-line mirror of TaskStatus, the wire format is unchanged, DEFAULT_PROJECT_STATUS is gone with no leftover references, and there is no compatibility code. Nothing blocks the merge. One test gap is worth closing in this PR, because it is the one claim in the description that nothing exercises end to end.

Requirements Check

Only the project.status half of this criterion of #3453 is in scope (priority landed earlier; the issue stays open).

• ✅ project.status is an extensible enum. ProjectStatus has four named variants plus User(String), used by both ProjectNode.status and ProjectNodeUpdate.status.
• ✅ An unknown non-user value is rejected (test). project_update_rejects_status_outside_the_schema_vocabulary and project_status_archived_is_rejected build the update with the enum and still fail in the shared pipeline's schema check.
• ✅ Node type sequence. Schema unchanged, wire type defined once in nodespace-types, TypeScript regenerated and listed in the generator, typed write path updated at both proto boundaries, node-types.md updated alongside.
• ✅ None of the Critical node-type patterns appear in the diff: no undeclared key, no new literal node_type comparison, no lifecycle_status read, no second wire-type definition, no hand-copied TypeScript, no compatibility shim.

What I verified

• Wire format is unchanged. Serialize writes as_str() and deserialize goes through from_value, so status is still a bare JSON string in both directions. null and an absent key both still mean "no change" on the update; a non-string is still a decode error.
• Default is right. A missing or non-string stored status becomes ProjectStatus::default() = planning, the same result as the deleted constant. The new test pins it to the schema's default, and project_without_status_defaults_to_planning still covers the conversion.
• User value round-trip, by reading the code. Tauri command (as_str().to_string()) → proto optional string → daemon (from_value) → to_properties_patch → schema validation → node_to_typed_value (from_value, then serialize_str). No step narrows a User value.
• No remaining raw-string treatment. The dev-proxy forwards status as a string to the same RPC. The CLI and agent have no project-specific status code; they write through the generic update, which the schema validates.
• Checks run: nodespace-types tests (112 passed), bun run types:check (up to date, 51 files), and the seven project/task status tests in nodespace-core (passed). I did not run the full suite or the merge gate.

Code Review Findings

🟡 Important

1. A user-defined project status is never exercised past the update struct
📁 packages/core/src/services/node_service/schema.rs:1710-1790, packages/nodespace-types/src/convert.rs:456-462

The description says "a user-defined value still round-trips", but the only test of a User value is status_is_typed_and_keeps_a_user_value, which stops at deserialize and to_properties_patch. Two legs are untested for project:

• the service accepting a value that was added to the schema's user_values;
• the read path, node_to_typed_value on a stored user status.

Task has the first one: update_task_node_accepts_status_added_via_add_field_values in node_service/mod.rs. The rejection tests alone would still pass if the typed path rejected every non-core value.

Recommendation: add one service test that mirrors the task one. Add on_hold to project.status with add_field_values, call update_project_node with ProjectStatus::User("on_hold"), and assert node_to_typed_value(updated)["status"] == "on_hold". That one test covers validation, the write and the read conversion.

Principle: test the behaviour the change claims. The user-value variant is the reason this is an enum and not a closed one.

🟢 Suggestions

2. The pin test catches schema drift, but not a new enum variant
📁 packages/core/src/models/core_schemas.rs:2427-2469

The schema-to-enum direction is solid, and stricter than the task test: it compares the ordered lists, so a value added, removed, renamed or reordered in the schema fails, and so does a changed default. The enum-to-schema direction depends on the hand-written named array. A fifth variant added to ProjectStatus (with its from_value and as_str arms) passes this test unless the author also remembers to edit the array. The task test and check_hand_serialized_enums in gen_ts.rs have the same gap.

The risk is low, because writing a variant the schema lacks is rejected loudly by the service. But the test name says "bidirectionally".

Recommendation: make the compiler enforce the list, for both enums. Either an exhaustive match in the test with no wildcard arm, or a CORE const beside each enum (as SchemaFieldType::ALL does) that the test and gen_ts.rs both read.

3. The task test does not pin the default, though the docs now say it does
📁 packages/core/src/models/core_schemas.rs:2377-2425; docs: components/creating-node-types.md, "Wire struct" row

The docs row says "A test pins the named variants and the default to the schema's coreValues and default". That is true for project after this PR. It is not true for task: nothing compares TaskStatus::default() with task.status's schema default.

Recommendation: add the same three-line default assertion to the task test in this PR, so the documented rule holds for both types it names.

4. Nit: rustdoc syntax leaks into the generated TypeScript
📁 packages/nodespace-types/src/project.rs:8-13 → packages/desktop-app/src/lib/types/generated/project-status.ts:3-10

The doc comment is copied into the TypeScript file, where [`Self::Planning`] and User(String) mean nothing. Word it for both languages, for example "any other string is a status a user added to the schema; the default is planning". TaskStatus has no doc comment at all, so the two generated files also differ in that respect.

5. Nit: stale comment on the re-export
📁 packages/core/src/models/mod.rs:65-67

The comment still reads "task.status, and the priority scale the two types share", while the line now also exports ProjectStatus.

6. Nit: the project sheet omits "required"
📁 docs: components/node-types.md, project table

The task table's column is "Required / default" and shows required / open. The project table's column is "Default" and shows only planning, although the schema declares required: true for project.status. The "Typed update" bullet covers the consequence (set but not cleared), so this is only a consistency point between the two sheets.

7. Nit, not for this PR: three copies of the same enum boilerplate
📁 packages/nodespace-types/src/project.rs:15-68, task.rs:7-61, priority.rs

from_value, as_str, is_core and the two serde impls are now written three times, and the AI-chat types will likely add more. Mirroring by hand is the right call for this slice. A small macro_rules! for "extensible string enum" would be worth it at the next one, and it would close finding 2 by construction.

Docs accuracy

• components/node-types.md: the task and project rows and the "own vocabulary" bullet match the code.
• components/creating-node-types.md: accurate, except the default-pinning claim for task (finding 3).
• development/node-type-sequence.md, "TypeScript regenerated" line: accurate. ProjectStatus derives TS, is listed in the generator, and types:check passes.
• ADR-086 §2: accurate. The generated ProjectNode carries the envelope fields inline.

Recommendation

APPROVE. The change is correct and consistent with TaskStatus. Finding 1 is worth folding in before merge, and findings 2 and 3 are cheap to fix in the same pass. None of them blocks.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review addressed

Addressed

• 1. User-defined status untested past the update struct. Added project_update_accepts_a_status_added_to_the_schema: adds on_hold to project.status with add_field_values, writes it through update_project_node as ProjectStatus::User, and asserts node_to_typed_value reads it back as on_hold. It covers validation, the write and the read conversion.
• 2. Pin test and generator each kept their own variant list. TaskStatus::CORE and ProjectStatus::CORE now hold the named variants once; both pin tests and the generator's literal check read them. This removes the duplicate lists. It does not make the compiler catch a new variant left out of CORE; the service still rejects a value the schema lacks.
• 3. Task's default was not pinned. The task pin test now asserts the schema default equals TaskStatus::default(), so the documented rule holds for both types.
• 4. ProjectStatus's doc comment is reworded to read correctly in the generated TypeScript.
• 5. The stale comment on the re-export in core/src/models/mod.rs is fixed.
• 6. The project sheet in nodespace-docs now has the same "Required / default" column as task's.

Skipped

• 7. A macro for the extensible-enum boilerplate. As the review says, not for this PR. Worth doing when the next such enum is added.

Verification: bun run quality:fix clean; bun run test:changed passes (frontend, scripts, browser, types:check, Rust workspace).

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The review approved with no critical findings. The follow-up is one new test, one assertion, a const that replaces two inline lists, and comment and doc wording. No behaviour changed.