fix(cli): type unredeemable cursors instead of unavailable - #2269
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 561ac2736b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if reason == OmissionReason::Unavailable { | ||
| RetrievalPortOutcome::Unavailable(evidence) | ||
| } else { | ||
| RetrievalPortOutcome::Failed(evidence) |
There was a problem hiding this comment.
Mark failed cursor outcomes as tool errors
When an expired, foreign, or malformed cursor reaches this branch, the application returns an Ok evidence envelope whose termination is failed; however, render_result_parts only sets semantic_error for Err(ApplicationProblem) and the canonical human renderer labels every evidence envelope as successful. The resulting MCP payload therefore lacks top-level isError, so tracedecay tool exits 0 despite reporting a failed cursor redemption, silently misleading scripts and CI. Propagate OperationTermination::Failed into the tool's semantic-error classification.
AGENTS.md reference: AGENTS.md:L189-L190
Useful? React with 👍 / 👎.
Fixes #2252
Cause
The cursor was never bound to a connection or process.
ccq2cursors are stateless, HMAC-authenticated with the project's durable cursor key, and a secondtracedecay toolprocess redeems them fine. On master, starshipsrc/modules/mod.rs::handlepages all 112 callees across 12 separate CLI processes.The report's second call ran from
scratch/, not fromscratch/starship. The CLI resolves the project from the working directory, so that call routed to the enclosing tracedecay checkout. That checkout carries an identity marker from the operator's own profile, but the isolated profile never enrolled it. The daemon refused the route asproject_not_enrolled, andproject_open_problemflattened the refusal into a retryableapplication.surface.unavailable, so retrying could never help. MCP worked becausetracedecay serve -p .pinned the project.The same flattening hid every unservable cursor:
unavailable, terminationunavailable.unavailable.application.surface.unavailable.Change
project_not_enrolled, and the newproject_requiredfor a handshake with no project path, are terminalinvalid_requestproblems. They carry the route's own remedy text and thecorrect_requestlegal action.tracedecay-query).PreparedQueryErrorV1::Foreigncovers a cursor issued for another scope digest. Verification maps a key this authority does not hold toStalerather thanUnavailable.tracedecay-code-index-runtime). An authenticated cursor whose generation is gone isStale. A rejected cursor returns aFailedoutcome with the new typed omissionscursor_expired/cursor_foreign. Only an unavailable authority stays retryableUnavailable.OmissionReason::{CursorExpired, CursorForeign}, with the TypeScript SDK operations regenerated.No cursor store was added: the daemon already owns the durable key, and redemption is stateless.
Fail before / pass after
tracedecay-clicore_cli_suite::tool_cursor_test::a_cursor_presented_where_it_cannot_be_served_is_typed, run with this branch's sources reverted toorigin/masterand the tests kept:It passes on the branch. It also asserts
project_not_enrolled(kindinvalid_request,retryable: false,["correct_request"]) from a checkout enrolled by another profile, andproject_requiredfrom a directory outside any repository. Then the same cursor still returns a disjoint 10-row page where it was issued.callees_cursors_page_to_the_end_across_tool_processesasserts pages[10, 10, 5]and exactlyleaf_01..leaf_25across separate processes. It also passes on master: it is the cross-process regression guard for the journey the issue describes.tracedecay-querycanonical_execution_equivalence::unredeemable_prepared_cursors_reject_with_their_typed_stateassertsForeignfor another scope,Stalefor expiry, andStalefor an authority without the key. On master those areInvalid,Stale, andUnavailable; theForeignvariant did not exist.Journey (debug CLI from this branch, isolated profile, one daemon under
systemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G)starship@cc825b00,
handlecallees paged with successivetracedecay tool tracedecay_callees ... --jsonprocesses:The same starship cursor presented elsewhere:
A 70-minute-old fixture cursor (TTL is 15 minutes) now returns
omissions [{'reason': 'cursor_expired'}] termination failed. Earlier in the same session, master answered that cursor withunavailablewhile it was still inside its TTL but its generation was no longer held.Suites
tracedecay-query: lib 263 (1 ignored),canonical_execution_equivalence5,retrieval_contract_spine2,search_quality_suite71tracedecay-code-index-runtime: lib 516 (2 ignored)tracedecay-contracts: lib 420,contracts_suite277tracedecaylib: 756 of 758 passed in a full run. The two failures are shared-state flakes:replay::client_identity_startup_replays_retained_profile_receipts(SQLitedatabase is locked) anddashboard::tests::shutdown_deadline_aborts_joins_and_clears_dashboard_task(process-global dashboard manager not idle). Both pass in isolation, and neither touches this diff. After the rebase,daemon::tests::socket+daemon::invocation_dispatch: 20.tracedecay-clicore_cli_suitetool_: 51. After the rebase,tool_cursor_test+tool_surface_transport: 14.mcp_suite(test-transport;serve_harness|callers|callees|cursor|enroll): 24cargo clippy -p tracedecay-query -p tracedecay-contracts -p tracedecay-code-index-runtime -p tracedecay -p tracedecay-cli --all-targets -- -D warnings: clean, both without and withtracedecay/test-transport.cargo fmt --all -- --check: clean.dashboardcontracts:check: up to date.sdks/typescripttypecheck: clean.The runtime's
query_authority_lookup_preserves_real_mount_identity_isolationfailed once (repository id mismatch for a linked worktree) and passed on both reruns. It does not touch this diff.