feat(session): ServerMessage::CommandFailed + StateLayer Hash/Ord — v0.1.1 (card 1dbcefce)

[joelteply]

Loud failures, no success-ack: a failed CommandEnvelope previously
had NO reporting channel (silent failure, forbidden); a successful
command's ack IS the state change it causes, so the unidirectional
model stands. CommandFailed{correlation_id, error} is additive
(tagged union) — v0.1.0 consumers are unaffected.

StateLayer gains Hash/PartialOrd/Ord so substrate Subscription sets
can be HashSet/BTreeSet-typed (continuum-positron slice 2A currently
sort+dedups a Vec as a workaround).

Requested by claude-continuum at the 2B seam (continuum#1601 thread).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

[joelteply]

Adversarial review — VERDICT: BLOCK (request changes; all fixes are doc-line-sized)

The code is sound: 19/19 tests pass at 308e639, cargo fmt --check clean, clippy --workspace --tests -- -D warnings clean, and the tree is porcelain-clean after cargo test — so the committed ServerMessage.ts provably matches what ts-rs regenerates, and the command_failed tag is pinned by test. The TS union shape is correct (& binds tighter than |; type narrowing works). What blocks is that this is a contract crate whose product is the normative text, and the new text has three coherence defects:

1. CommandFailed delivery scope is unspecified (the real gap)

The module spec is rigorously MUST-shaped everywhere else (skip rule, replace-not-merge, watermark scoping) — but never says who receives CommandFailed. The additive-safety claim ('v0.1.0 consumers unaffected') silently depends on it being sent only to the connection that issued the command. A substrate that broadcasts it would (a) hard-error every older Rust consumer — serde internally-tagged enums reject unknown variants — and (b) leak failure details to bystanders. One sentence fixes this: 'Sent only on the connection that sent the failing Command; never broadcast.'

2. 'Additive and non-breaking' is overstated without an unknown-frame rule

True at the schema level for freshly-compiled consumers; false as stated for deployed ones. An old TS client that switches on msg.type with only a "state" case silently drops the frame (ironically, the loud-failure frame fails silently); one that assumes every frame is state reads undefined kind/payload; an old Rust consumer gets a hard deserialize error. The spec needs a forward-compat rule (consumers MUST ignore frames with unknown type) — or the claim needs qualifying. With #1's sender-only scoping this is mostly theoretical for this PR, which is why it's a doc fix, not a redesign.

3. 'v0.1.1' / 'v0.1.0' exist nowhere

Workspace Cargo.toml = 0.0.1, npm/core/package.json = 0.0.1, and lib.rs § Versioning says 'v0.0.x — contract design'. The PR bumps nothing, yet the module doc now permanently asserts CommandFailed 'exists (v0.1.1)'. Either bump the manifests or drop the invented labels — module docs shouldn't state version facts the repo contradicts.

Risk notes (non-blocking)

• Ord on StateLayer: derive order (Ephemeral < Session < Persistent < Semantic) is now public API. It's almost descending-cadence, except Semantic ('on-demand') is off that axis — and inserting a future layer mid-enum silently reorders comparisons consumers may have written (layer >= Persistent). Add one doc line declaring the ordering either meaningful or incidental-for-collections-only.
• error: String: acceptable v0 judgment given the doc already names a structured Result frame as the v0.x candidate; note the test's "chat/send: room not found" prefix style is becoming an implicit convention — name it or expect drift.
• ts-rs 'failed to parse serde attribute' warning during tests pre-exists on main (wire.rs skip_serializing_if), not introduced here.

Evidence

• Diff: git diff origin/main...origin/feat/command-failed-1dbcefce (3 files; session.rs, wire.rs, generated TS)
• Temp worktree at 308e639: cargo test -p positron-core → 19 passed, 0 failed; fmt --check OK; clippy -D warnings OK; git status --porcelain empty post-test (generated TS in sync). Worktree removed.

Approve follows immediately once #1–#3 land — each is a sentence.

🤖 Generated with Claude Code

[joelteply]

Re-review verdict: APPROVE (fix commit 25a4dfe)

All three blocking findings from the prior review are addressed with evidence; the non-blocking note is addressed too.

1. CommandFailed delivery scope — FIXED. positron-core/src/session.rs now documents on the CommandFailed variant: substrate MUST send the frame ONLY to the connection that sent the failing command, never broadcast.

2. Forward-compat rule — FIXED. session.rs module docs now state the explicit rule: unknown type on a ServerMessage = skip-and-log, never connection-fatal, and notes that Rust consumers of the typed enum see unknown variants as a per-frame deserialize error and must apply the same rule (log, skip frame, keep connection).

3. Version honesty — FIXED. Real bump, not doc edits: workspace Cargo.toml version = "0.1.1", npm/core/package.json "version": "0.1.1", and lib.rs versioning text rewritten to v0.x semantics. session.rs's "exists (v0.1.1)" now matches the manifests. Repo-wide grep found no remaining 0.0.1 / stale-version references.

4. (non-blocking) Ord on StateLayer — DOCUMENTED. Both wire.rs and the regenerated npm/core/src/generated/StateLayer.ts state the ordering is declaration order with no semantic meaning.

Verification (clean temp worktree at 25a4dfe):

• cargo test -p positron-core: 19 passed, 0 failed
• cargo fmt --check: clean
• cargo clippy --workspace --tests -- -D warnings: clean
• git status --porcelain after tests: clean (generated TS matches ts-rs output exactly — no drift)

Minor note (non-blocking): ts-rs does not emit variant-level docs into the generated union, so the delivery-scope and forward-compat rules are visible only in the Rust contract docs, not in ServerMessage.ts. That is a ts-rs limitation, not drift — delivery scope binds the substrate (Rust) side, and the contract crate is the documented source of truth. If TS-client visibility is wanted later, a hand-written doc block in the npm README would cover it.

🤖 Generated with Claude Code