fix(admission): make NotApplicable a terminal no-op - #1843
Conversation
Closed NotApplicable already finishes the replay record, but the shared pass classifier still backed off on it and the MCP hook boundary still published it as a JSON-RPC failure. Stop that pass without retry or a failure log, and return a successful no-op on the wire. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ee84d3bf0
ℹ️ 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 tool_name == "tracedecay_hook_runtime" | ||
| && let Some(result) = hook_runtime_not_applicable_noop(error) | ||
| { | ||
| return JsonRpcResponse::success(id, result); |
There was a problem hiding this comment.
This branch has no production path that can satisfy it: the repo-wide production constructor of HostAdmissionStatus::NotApplicable is code_index_host_outcome in crates/tracedecay/src/mcp/server.rs, whose outcome is consumed by replay and hook-event notification handling rather than converted into the status-bearing TraceDecayError required here. The only call demonstrating this response is the synthetic unit test below, so the advertised MCP behavior does not actually stop a host retry; translate the real outcome on its response path instead of adding an unreachable boundary mapping.
AGENTS.md reference: AGENTS.md:L132-L134
Useful? React with 👍 / 👎.
…le-terminal-noop-9c8a
The hook-runtime error boundary gained a branch that turned a `not_applicable` admission status into a successful JSON-RPC result, but no production path can produce that error. The only production constructor of `HostAdmissionStatus::NotApplicable` is `code_index_host_outcome`; its outcome reaches either the hook-event notification path (which returns no JSON-RPC response at all) or a replay worker pass, and neither converts an outcome into a `TraceDecayError`. The one hook-runtime call site that maps an outcome to an error, `hermes.rs`, is gated on `!commits_replay_record()`, and `NotApplicable` commits. Removing the branch and its synthetic test leaves the reachable half of the change: `classify_replay_pass` treating a closed `NotApplicable` that did not shrink the spool as `TerminalNoop`. Resolves the Codex P1 review finding on #1843 and corrects the CHANGELOG entry, which claimed a wire behaviour change that does not occur. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Cursor checked: this PR is fully folded into #1848 (batch C). Leaving the leaf alone — Claude owns the batch head. No competing rebase. |
2 participants
Summary
HostAdmissionStatus::NotApplicableas a terminal no-op in the shared replay-pass decision, so an unchanged spool stops until the next kick instead of backing off.isError: false,retryable: false) rather than an internal error.Motivation
NotApplicablealready closes the replay record, butclassify_replay_passstill counted it as replay progress and backed off while records remained. The MCP wire then published the same status throughtool_error_responseas a JSON-RPC failure, so hosts retried a demand that does not apply.Changes
crates/tracedecay-host-admission/src/replay.rs:ReplayPassDecision::TerminalNoopforNotApplicablewhen the spool does not shrink. A real shrink still continues; a drain still requeues.AcceptedForReplaystill backs off.TerminalNoopby stopping without a backoff sleep and without a failure log.crates/tracedecay-mcp/src/tool_errors.rs: hook-runtimenot_applicablereturnsJsonRpcResponse::success. Other admission statuses stay JSON-RPC errors.Test plan
cargo test -p tracedecay-host-admission --lib replay::tests::not_applicable_is_a_terminal_noop_unless_the_spool_actually_moves -- --exact(1 passed)cargo test -p tracedecay-mcp --lib not_applicable(2 passed: project replay does not back off; wire response is success and unavailable stays an error)cargo test -p tracedecay-daemon-service --lib profile_host_admission_replay::tests::not_applicable_pending_record_does_not_back_off -- --exact(1 passed)cargo clippy --workspace --all-targets --locked -- -D warningsnot run hereChecklist
CHANGELOG.mdupdated (under[Unreleased]).envfiles includedReplayPassDecisiongainsTerminalNoop; both in-repo workers match it