Preserve primary runtime ownership when project registration requests fail

[sussdorff]

Summary

t3 project add can delete a running primary's discovery descriptor after a request timeout, then report success after writing offline. This focused reliability fix preserves the descriptor and refuses that fallback whenever a primary is recorded.

 recorded primary + failed request
- delete server-runtime.json; write offline; report success
+ preserve server-runtime.json; report unavailable or unknown live outcome

A missing descriptor still permits offline registration. Unreadable or malformed state fails closed. Successful mutations identify Mode: live. or Mode: offline.; an uncertain mutation response explains that the server may have committed and that no offline write occurred.

Related: #7504 and #15624. I checked open PR #15624 at 8ab1305485d418435293fee7d24bfe049ac897ad before submitting: it still deletes a recorded descriptor after a failed probe when the PID is not locally visible, and does not add uncertain live-mutation-outcome reporting or live/offline success output. This proposal preserves the recorded descriptor without PID-based cleanup, whose ownership assumptions do not hold across PID namespaces or successor races. It addresses the same underlying registration failure, with no server restart, release-pointer change or unrelated cleanup. It uses the focused obvious-bug exception in CONTRIBUTING.md; maintainer acceptance is still required.

Evidence

• Before: exact v0.0.46-nightly.20261008.2813, source 30cc788975500a8c00d32a50f348174d1ce578d1. An isolated loopback primary was paused using its captured PID. Registration exited 0 with Added project ..., removed server-runtime.json, and left the primary alive.
  After: the locally rebuilt native executable exited 1 with ProjectLiveServerUnavailableError; descriptor bytes were identical and no project was added. After resuming that same primary, registration exited 0 with Mode: live., persisted exactly one new project and retained the earlier project ID and runtime ownership.
• For lost mutation responses, an isolated loopback proxy forwarded one POST, observed upstream HTTP 200 and then dropped or stalled the response. Both native commands reported ProjectLiveMutationOutcomeUnknownError; each primary contained exactly one new project, retained prior IDs and received no offline fallback. The isolated routing descriptor stayed byte-identical through both commands.
• Test-first repair: 11 intended failures before repair; 50 focused tests and 167 CLI/runtime-state tests pass. Server typecheck and scoped lint/format pass. Native archive build and archive smoke test pass.
• Source checks used uv run --no-project python tools/repair_check.py to invoke the documented native toolchain: vp test run src/cli/project.test.ts src/cli/pair.test.ts src/serverRuntimeState.test.ts, then vp test run src/cli src/serverRuntimeState.test.ts; scoped vp lint, vp fmt --check, and the server typecheck. These orchestration helpers live outside the repository.

All runtime checks used private state, an explicit --base-dir, loopback listeners and temporary auth sessions held in memory. No active development service, installed executable or real T3 state was changed. Every owned primary/proxy was stopped and temporary sessions were revoked.

Merge Danger

Door: two-way; revert the source change.

Blast Radius: CLI

A leftover descriptor after a crash now blocks project mutations until a primary replaces it. This deliberately avoids inferring exclusive ownership from a failed request or a local PID lookup.

Work order

https://git.cognovis.de/cognovis/fleet/issues/264 stays open pending an upstream maintainer release and verification of that official artifact.

Verification

Independent non-author verdict: PASS+NOTES for 2c181d7f5009994b92269e49fdd0b2c22cebf02d. Native timeout, successful registration and committed-but-lost-response paths were executed with the observed outcomes above.

The local executable is 0.0.46-nightly.20261008.2813+fleet264.repair-v2.local-unreleased, SHA256 41cc97f65a628763a6651c1aade6369a8c89fbd450acd884a966e880be2c081f. Patch SHA256: 5a86ca1ba36e2fbf7ae6875e1a4f6afe6eac08e53b2dba94b9a401693596d39b. This is unreleased evidence. The unchanged resource monitor was copied from the exact installed nightly, not rebuilt.

Reviewer routes

Three fresh read-only reviewers examined the same original candidate: Claude Opus via ccore agent run --model claude-opus --harness claude; GPT-6 Astra via ccore agent run --model gpt-6-astra --harness codex; GPT-6.1 Sol natively via Codex with xhigh reasoning. Opus then authored the single triaged repair round. Sol independently verified the repaired native artifact; no author verified its own repair.

Merge assessment

Product decision: none deviating from the work order and selected upstream-contribution route; registration preserves recorded ownership and reports the actual execution mode or uncertainty.

Economic damage: none expected from this candidate; it removes destructive offline fallback, introduces no access/data boundary and changes no deployment or customer-followed release pointer. Our account cannot merge or publish upstream; maintainers own acceptance and release.

Known residuals

Official release and subsequent Fleet acceptance remain pending. No maintained distribution fork is proposed. Automatic stale-state cleanup is intentionally absent because it lacks an ownership-safe primitive. Low/nit review notes are recorded below rather than expanded into unrelated work.

Review decisions

Local review findings not repaired in this pull request, with the reason:

• L1 (low, Opus F3): Preflight error wording conflates server responses and auth failures with no answer, and nothing written overstates auth-store side effects. -- deferred, below Medium severity.
• L2 (low, Opus F5): Failure assertions should distinguish expected error classes from unrelated fixture/auth failures. -- deferred, below Medium severity.
• N1 (nit, Opus F7): The Mode suffix is observable on add/rename/remove; upstream maintainers may prefer another representation. -- deferred, below Medium severity.

Repaired in the repair round:

• M1 (high, Opus F1/F4; Astra R1/R2; Sol SOL-1/SOL-2): Stale-primary fallback lacks trustworthy exclusive ownership: namespace-dependent PID liveness, failed descriptor reads and non-atomic re-read/delete can delete a valid successor descriptor and write offline alongside a primary. -- repaired.
• M2 (medium, Opus F2; Astra R3): A mutation-response timeout reports generic request failure even when the live registration may already have committed; report unknown live outcome and no offline fallback, and test the committed/lost-response case. -- repaired.
• M3 (medium, Opus F6; Astra R4; Sol SOL-3): Live-success evidence must observe actual persisted registration and preserved pre-existing project IDs; current fake and native output checks do not establish that postcondition. -- repaired.

Implementation and repair: Claude Opus through the Claude harness via ccore. Delivery and independent verification: distinct GPT-6.1 Sol sessions through the Codex harness.

[macroscopeapp]

Approvability

Verdict: Approved at 2c181d7

Macroscope's review found this PR approvable — This is a contained CLI bug fix that prevents project registrations from falling back to offline writes when a recorded primary may still own the data, including cases where a live mutation’s result is uncertain. Production behavior changes are limited to this failure-handling path, with focused tests covering ownership preservation and live/offline outcomes.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: c7413041-1fe9-434b-8b3b-cdee9f84272c

📥 Commits

Reviewing files that changed from the base of the PR and between a4c9494 and 2c181d7.

📒 Files selected for processing (2)

• apps/server/src/cli/project.test.ts
• apps/server/src/cli/project.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The project CLI now distinguishes absent runtime state from unreadable state. It no longer falls back to offline registration when a recorded primary is unavailable. Live mutation failures can report an unknown outcome, and completion output identifies live or offline mode.

Changes

Project CLI registration

Layer / File(s)	Summary
Decode and resolve runtime state apps/server/src/cli/project.ts	The CLI treats only a missing runtime-state file as absence. Read or decode failures return an error instead of allowing offline execution.
Preserve live mutation outcomes apps/server/src/cli/project.ts, apps/server/src/cli/project.test.ts	The CLI retains runtime state when a recorded server is unavailable. Non-server-error failures during live mutations report an unknown outcome. Tests cover unavailable and changing primaries, unreadable state, and lost responses after a commit.
Report registration mode apps/server/src/cli/project.ts, apps/server/src/cli/project.test.ts	Successful live registration reports live mode. Registration without a recorded primary writes locally and reports offline mode.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ProjectCLI
  participant RuntimeStateFile
  participant PrimaryServer
  participant OfflineProjectStorage
  ProjectCLI->>RuntimeStateFile: Read persisted primary descriptor
  RuntimeStateFile-->>ProjectCLI: Return descriptor
  ProjectCLI->>PrimaryServer: Send project.create
  PrimaryServer->>PrimaryServer: Commit project
  Note over ProjectCLI,PrimaryServer: Response is lost
  ProjectCLI-->>ProjectCLI: Report unknown mutation outcome
  Note over ProjectCLI,OfflineProjectStorage: No offline write

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 2c181

No actionable issue remains from this review; the CLI preserves uncertainty when a primary may have committed a registration.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2c181

The change strengthens failure containment: failed requests no longer erase recorded server ownership or permit conflicting offline writes. No introduced security weakness was identified. Recovery still depends on existing startup and mutation-response behavior, which was not fully verified under interruption and post-commit failures.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected authority is project mutation for the selected base directory and its recorded server, including existing forced deletion behavior. The comparison adds no remote route or broader privilege; it restricts when the CLI can become an offline writer.

Security Findings and Attack Paths

• observed — The routed HTTP helpers and mutable fake-primary state are test fixtures bound to loopback and cleaned up after use. They do not establish a new production attacker-controlled entrypoint. The canonical security brief contains no retained findings.

Trust Boundaries and Controls

• observed — The CLI continues to trust the descriptor's origin as the destination for administrative bearer sessions. Sessions use acquire/use/release with revocation on release; server snapshot and mutation handlers retain read and operate scope checks. This trust relationship predates the PR and is not broadened by its failure handling.

Resilience and Maintainability Implications

• observed — The existing project service plans commands under project/workspace locks and uses receipts to resolve repeated command IDs. CLI addition checks the current snapshot before generating fresh identities. The PR preserves these controls and adds no automatic retry following an uncertain live outcome.
• inferred — Two preexisting limits remain: descriptor absence can race server activation, and a declared internal response need not prove rejection because create commits before enrichment invalidation and readback. Both still terminate without live-to-offline fallback; neither is attributed to this PR as a new security concern.

Hardening Proposals

• proposed — A separate contract improvement could distinguish definitive pre-commit rejection from internal failures after commitment, with fault-injection coverage for post-commit readback failures. This would strengthen outcome reporting beyond the PR's transport-failure fix.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: preserving primary runtime ownership when project registration requests fail.
Description check	✅ Passed	The description is detailed and covers the problem, change, scope rationale, verification evidence, residual behavior, and review decisions. It uses custom headings instead of the template headings, b…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}