Skip to content

Drop project.status archived and the AI chat viewer's archived read-only gate - #3468

Merged
malibio merged 2 commits into
mainfrom
issue-3445-drop-archived-meanings
Oct 1, 2026
Merged

malibio merged 2 commits into
mainfrom
issue-3445-drop-archived-meanings

Conversation

@malibio

@malibio malibio commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Closes #3445

Summary

Two types stop giving "archived" a meaning of their own (ADR-087 §1, §3).

  • project.status loses archived. The values are planning, active, completed, cancelled (plus user values). Retiring a project is archiving the node through lifecycle.
  • The AI-chat viewer always renders its message input and no longer reads lifecycleStatus. The "archived and read-only" notice and its style are gone.

No migration: reset the database.

Node type sequence

  • Type: project, a flat core type (kind and category unchanged).
  • Changed: the status field's coreValues drop archived; the field description no longer mentions it. No field, relationship or rule is added.
  • Wire type: ProjectNode.status stays a string (the schema owns the vocabulary), so nothing is regenerated. The hand-written TypeScript ProjectStatus union drops archived.
  • Docs: the project sheet in components/node-types.md and ADR-087 in nodespace-docs already describe this shape, so there is no docs change.
  • CLI help and skill references list no project statuses, so they are unchanged and the SKILL.md drift check is unaffected.

Tests

  • test_project_status_values: the schema's core values are exactly the four.
  • project_status_archived_is_rejected: both the typed update and node creation reject status: "archived".
  • ai-chat-viewer-lifecycle.test.ts: the rendered viewer shows an enabled input for an active and an archived chat, and the viewer source reads no lifecycle status.
  • Removed two placeholder "archived read-only" tests that asserted on local constants.

bun run test:changed passes (frontend, scripts, browser, Rust).

🤖 Generated with Claude Code

@malibio

malibio commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review — PR #3468

Review Type: Initial Review (no prior reviews or comments on the PR; one commit, 47709318)

Summary

Small, atomic change that does what ADR-087 §1 and §3 ask: project.status loses archived, and the AI-chat viewer stops reading lifecycle. No compatibility code, no migration, no new field or wire type. The PR description's claims check out against the repo. Nothing blocking; five optional items below.

Requirements Check (issue #3445)

  • ✅ project.status has no archived, and writing it is rejected (test) — core_schemas.rs drops the value and the description text. test_project_status_values pins the core values to exactly the four; project_status_archived_is_rejected covers the typed update and node creation. Both would fail on the pre-change code (the value was a valid core value, so unwrap_err() would panic).
  • ✅ Viewer renders the input whatever the lifecycle; reads no lifecycle_status (frontend test) — the gate, the notice and its CSS are gone. The new test renders the real component for an active and an archived chat. The archived case would fail before the change: setNode with a database source runs nodeToAiChatNode, which carries lifecycleStatus through, so the old viewer would have shown the notice instead of the input.
  • ✅ CLI help and skill guidance listing project statuses updated; SKILL.md drift check passes — nothing to update. I searched packages/cli, packages/skill, packages/agent, packages/daemon, packages/dev-tools and scripts for the status list and for archived next to project: no hits outside the files this PR touches.
  • ✅ Follows the node type sequence; node-types.md project sheet updated — the sheet in nodespace-docs already lists planning, active, completed, cancelled and says "status has no archived value". participation.md, ai-chat-node.md and data-layer.md already say archived is not read-only. I read these on disk only; I did not check that the docs repo has them pushed.
  • ✅ bun run test:changed passes; quality:fix clean — I ran the three touched frontend test files: 38 tests pass. I did not run the Rust tests (they need the machine slot), so the Rust result rests on the PR's own claim plus reading the code.

Non-goal respected: session_status: "archived" (schema line 641, ai-chat-pty-session.svelte, capture service) is untouched.

Node-type reviewer checklist

No undeclared key, no literal node_type comparison added, no second wire-type definition (ProjectNode.status stays a string; nothing to regenerate), no ids-as-relationships, no compatibility code. The change removes a lifecycle_status read outside governance rather than adding one.

Code Review Findings

No 🔴 Critical or 🟡 Important findings.

🟢 Suggestion — 📁 packages/core/src/services/node_service/schema.rs:1888-1927
The doc comment says "both write paths reject it", but there are three: the typed update, creation, and the generic flat update the CLI and agent use. The sequence says the flat update goes through the same pipeline, so this is very likely covered already; either add a third assertion through the generic update or reword the comment to name the two paths tested. Principle: a test's stated claim should match what it exercises.

🟢 Suggestion — 📁 packages/core/src/models/core_schemas.rs:283-296
project.status stays extensible: true, so a user can presumably add archived back as a user value. I did not check whether the add-value path refuses governance vocabulary. ADR-087 §3 only constrains the type's own enum, and this is outside the issue's scope, so no action is needed here; worth knowing when the governance module lands.

🟢 Nit — 📁 packages/desktop-app/src/tests/components/ai-chat-viewer-lifecycle.test.ts:65-67
The "reads no lifecycle status" test greps the viewer's source text. It meets the acceptance criterion, but it guards one file and would fail on a comment that mentions the word. ADR-087 plans a general "nothing else reads it" test with the governance module; this one becomes redundant then. Fine to keep until that exists.

🟢 Nit — 📁 packages/core/src/ops/query_ops.rs:934-940
The rf_project fixture still creates proj-archived with status: "archived". It is a user-defined test type, so it is not wrong, but the PR changed the equivalent non-core fixture in behaviors/mod.rs:4131 to completed; doing the same here keeps the repo free of examples that model the vocabulary ADR-087 §3 discourages.

🟢 Nit — 📁 packages/desktop-app/src/lib/types/ai-chat-node.ts:142,173
AiChatNode.lifecycleStatus now has no production reader. It is an envelope field (ADR-087 §4), so keeping it is correct; noting it only so nobody reads it as an oversight.

What I checked beyond the diff

  • No remaining "archived and read-only" text or .archived-notice style anywhere in packages/.
  • No other lifecycleStatus read in packages/desktop-app/src/lib apart from the envelope carry-through above.
  • kanban-grouping.test.ts uses archived as a user value on a generic schema, not project; unaffected.
  • No goldens, fixtures, dev-proxy code or agent prompts list project statuses.

Recommendation

APPROVE — net improvement, requirements met, tests prove the behaviour. The findings are optional.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@malibio

malibio commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed (5135e22)

Addressed (2)

  • schema.rs test comment vs. coverage: project_status_archived_is_rejected now also asserts the generic update, and the comment names the three paths it exercises (typed update, generic update, creation).
  • query_ops.rs related-filter fixture: the second project is proj-completed with status: "completed".

Skipped (3)

  • project.status is extensible, so a user could add archived as a user value: outside this issue. ADR-087 §3 constrains the type's own enum; whether user values may reuse governance vocabulary belongs with the governance module.
  • The source-text check in ai-chat-viewer-lifecycle.test.ts: kept. It is the acceptance criterion's "reads no lifecycle_status" check until the general "nothing outside governance reads it" test exists.
  • AiChatNode.lifecycleStatus has no production reader: it is the envelope field (ADR-087 §4), so it stays.

Tests: the two touched Rust tests pass.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The review was APPROVE with suggestions only. The follow-up commit changes test code only: one extra assertion and a fixture value rename, with no production logic touched.

malibio and others added 2 commits October 1, 2026 19:20
…nly gate (closes #3445)

project.status keeps planning, active, completed and cancelled; retiring a
project is archiving the node through lifecycle (ADR-087 §3). The AI-chat
viewer renders its input whatever the chat's lifecycle and no longer reads
lifecycleStatus (ADR-087 §1).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- project_status_archived_is_rejected now asserts the generic update too, so
  its comment names every write path it exercises.
- The related-filter fixture's second project uses status completed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@malibio
malibio force-pushed the issue-3445-drop-archived-meanings branch from 5135e22 to e856707 Compare October 1, 2026 17:20
@malibio
malibio merged commit 5d29261 into main Oct 1, 2026
@malibio
malibio deleted the issue-3445-drop-archived-meanings branch October 1, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop project.status archived and the AI chat viewer's archived read-only gate

1 participant