Skip to content

fix: retain completed analysis after acpx permission denials - #245

Merged
kunchenguid merged 4 commits into
kunchenguid:mainfrom
WTF-Am-ID:fm/up-backpass-112-redesign
Oct 30, 2026
Merged

kunchenguid merged 4 commits into
kunchenguid:mainfrom
WTF-Am-ID:fm/up-backpass-112-redesign

Conversation

@WTF-Am-ID

@WTF-Am-ID WTF-Am-ID commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Closes #112

Backpass retained completed, schema-valid analysis when acpx exited 5 after a denied request. Recovery uses the final completed root prompt exchange, while denial counts come from permission outcomes and usage comes from trusted metadata. Invalid or incomplete answers, cancelled turns, other failed exits, and timeouts still fail.

The analysis cache version stayed at 4. Previously successful evidence remains reusable, and failed entries already retry without invalidation. A CLI regression reproduced the unnecessary re-analysis before this correction and verifies cache reuse afterward.

Validation on candidate b0d88d44cb4a99a99b381c2b2265bf372be96d94:

  • The four permitted test files passed all 71 cases. Lint, formatting, and type checking also passed before pipeline submission.
  • Real acpx 0.15.0 and Codex CLI 0.165.0 completed a normal backpass CLI analysis, retained a quoted evidence item, and reported usage matching ACP response metadata. An unchanged rerun reused the cache without another acpx call.
  • A real denied execute request produced acpx exit 5 and a cancelled turn. Backpass rejected it and stored no successful evidence. Completed real exit-5 recovery was not obtained from Codex; the completed-recovery contract is covered by synthetic regression tests.

Focused test command:

node --import ./test/helpers/hermetic-config.js --test test/acpx-analysis-closure.test.js test/acpx-exec-timeout.test.js test/acpx-session-setup-timeout.test.js test/analyze-reuse.test.js

Review was single-family (Codex) by the owner's temporary choice. Real acpx transcripts and the response to the maintainer include the observed limitations.

The Test gate was approved with an explicit evidence exception: the real analysis and denial probes ran in the outer executor and were independently verified from their captures. The Test phase itself retained an inconclusive live-validation classification. Completed real exit-5 recovery remains unverified.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 warning
  • ⚠️ Synthetic behavioral checks passed, but real acpx/Codex normal-analysis and permission-recovery transcripts remain required to demonstrate the complete intent. This phase explicitly prohibits model probes. The outer executor must supply those live results.
  • ⚠️ live validation verdict: inconclusive (1 of 5 scenarios were driven live against the product); untested: Complete normal analysis and retain evidence with measured usage, Retain valid completed permission-denied analysis and report observed or unknown denial counts, Reject incomplete or failed analysis while preserving timeout handling and resolver fallback, Ignore historical token lines in echoed prompts and use trusted accounting sources
  • Live validation: ⚠️ inconclusive - 1 of 5 scenarios driven live against the product
Scenario Result Live Evidence
Run analysis with pre-existing successful v4 evidence and reuse it without model calls ✅ pass live cache-cli-transcript.txt; cache-status.json
Complete normal analysis and retain evidence with measured usage ⏸️ untested no Exercised real analysis code with synthetic acpx responses. Live model execution is explicitly reserved for the outer executor, so disposable credentials or another endpoint would not authorize a prob…
Retain valid completed permission-denied analysis and report observed or unknown denial counts ⏸️ untested no Synthetic protocol responses exercised retention, persisted counts, and warnings. Reproducing this through real Codex requires a model probe, which this phase forbids. The outer executor must attempt…
Reject incomplete or failed analysis while preserving timeout handling and resolver fallback ⏸️ untested no Adversarial synthetic responses exercised these boundaries in both invocation routes. Real agent failures cannot be induced here without the prohibited model probes. The outer executor owns live confi…
Ignore historical token lines in echoed prompts and use trusted accounting sources ⏸️ untested no Both routes passed synthetic regressions with historical token text inside echoed RPC prompts. Actual acpx/Codex output remains unverified because live probes are reserved for the outer executor.
  • node --import ./test/helpers/hermetic-config.js --test test/acpx-analysis-closure.test.js test/acpx-exec-timeout.test.js test/acpx-session-setup-timeout.test.js test/analyze-reuse.test.js
  • node .local-cache-verification/drive-cache.mjs: created isolated repository, home, transcript, and successful v4 evidence with model execution disabled
  • Real CLI scan --harness pi --since all --host none --json, two analyze --harness pi --since all --host none --json --analysis-agent codex --jobs 1 runs, and status --json
  • git diff --check
  • Removed disposable fixtures and driver from the worktree; git status --short confirmed no remaining changes

🔧 No changes applied.
1 warning still open:

  • ⚠️ live validation verdict: inconclusive (1 of 6 scenarios were driven live against the product); untested: Complete normal analysis and retain a quote-backed gap with measured usage, Deny an execute request and reject the cancelled analysis without successful evidence, Retain completed valid exit-5 analysis with observed or explicitly unknown denial counts, Reject invalid or incomplete analysis while preserving timeout handling and resolver fallback, Ignore historical token accounting in echoed prompts on both invocation routes
  • Live validation: ⚠️ inconclusive - 1 of 6 scenarios driven live against the product
Scenario Result Live Evidence
Rerun analysis with successful v4 evidence and reuse it without model usage ✅ pass live Live CLI v4 cache reuse
Complete normal analysis and retain a quote-backed gap with measured usage ⏸️ untested no The prior payload established verification of a supplied capture through the candidate parser, not a live run in this phase. Response usage matched CLI usage, and the persisted quote existed in the in…
Deny an execute request and reject the cancelled analysis without successful evidence ⏸️ untested no The prior payload established verification of a supplied capture, not a live run in this phase. The capture recorded rejected execute permission, acpx exit 5, a cancelled turn, CLI exit 1, an empty ev…
Retain completed valid exit-5 analysis with observed or explicitly unknown denial counts ⏸️ untested no Synthetic behavioral regressions passed. Real completed exit-5 recovery remains untested: the supplied real denial cancelled the turn, and additional model probes are explicitly prohibited in this pha…
Reject invalid or incomplete analysis while preserving timeout handling and resolver fallback ⏸️ untested no The prior payload established only synthetic behavioral regressions, without live agents. It did not establish a live result for invalid analysis, timeout handling, or resolver fallback. Additional mo…
Ignore historical token accounting in echoed prompts on both invocation routes ⏸️ untested no The prior payload established only synthetic subprocess checks for absent usage, actual stderr accounting, and ACP result usage on both routes. It did not establish a live result. Additional model pro…
  • node --import ./test/helpers/hermetic-config.js --test test/acpx-analysis-closure.test.js test/acpx-exec-timeout.test.js test/acpx-session-setup-timeout.test.js test/analyze-reuse.test.js — exit 0; 71 passed, 0 failed.
  • node ~/.no-mistakes-upstream-codex/evidence/01M649DYK4XWPB75P39EPDTMJR/verify-captures.mjs — exit 0; independently verified supplied RPC output, measured usage, persisted quotes, cached rerun, denial outcome, and sentinel absence.
  • Ran the real backpass scan and backpass analyze CLI in a disposable worktree-local repository with relocated v4 evidence and an unavailable acpx executable. Observed cached=1, analyzed=0, usage=[].
  • git status --short and scratch inventory confirmed a clean candidate and no remaining disposable setup.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Reviews (2) · Last reviewed commit: "fix: preserve successful analysis caches..." · Reviewed by Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: thanks @WTF-Am-ID for taking on #112. CI is green and the PR is CLEAN, and keying recovery on a completed root session/prompt exchange plus a full schema check looks like the right shape.

This is waiting on you for two things before it can merge (no rebase needed from you):

  1. Live evidence against real acpx. All 8 scenarios ran with a synthetic Codex acpx. But this PR switches every analysis call, including the normal exit-0 path, from --format quiet to --format json and routes the answer and usage through the new readAcpxOutput parser. If real acpx's JSON stream differs from the fixtures, the parser returns null and the JSON stream itself would be parsed as the answer, which would break ordinary runs. Could you run at least one real backpass analysis through real acpx (any harness you have, Codex is fine) on a normal exit-0 turn and show the evidence and usage come through as before, and, if you can provoke one, a real exit-5 PERMISSION_DENIED turn? Please post the transcripts here.
  2. The ANALYSIS_INDEX_VERSION 4 to 5 bump. That invalidates every user's cached analyses and forces a full re-analysis, which is a real token cost on upgrade. Cache entries written before this PR never came from a denied run (those failed), so they don't seem to need invalidating. Could you either drop the bump or explain what an old cache entry would get wrong without it?

Small one: please add Closes #112 to the description so the issue closes on merge.

VISION per-rule:

  • Evidence is the only currency: aligns (a completed, schema-valid turn is kept as evidence; quotes still go through sanitizeEvidence).
  • The human owns the weights: not affected.
  • Nothing the model says is taken on faith: aligns (the denial count comes from permission exchanges, not model text, and usage from result metadata). Inconclusive on real acpx output until item 1 is shown.
  • The budget is the constraint: not affected.
  • It reads what you already have and owns nothing: aligns (no key, no permission loosening; --non-interactive-permissions deny stays).
  • Failure is loud and named: aligns (named PERMISSION_DENIED warning with an observed or explicitly unknown count; other exits and incomplete answers still fail).
  • Scope: aligns (root fix in the acpx invocation path).

Contract-class: restore, provisionally. Unconfigured analysis already promises usable evidence when the agent finishes a turn, and exit 5 after a valid answer broke that path. That holds only if the exit-0 path behaves the same on real acpx, which is what item 1 checks.

@WTF-Am-ID
WTF-Am-ID force-pushed the fm/up-backpass-112-redesign branch from 2c57bbc to b0d88d4 Compare October 29, 2026 20:36
@WTF-Am-ID WTF-Am-ID changed the title fix: retain valid analysis after acpx permission denial fix: retain completed analysis after acpx permission denials Oct 29, 2026
@WTF-Am-ID

Copy link
Copy Markdown
Contributor Author

Addressed the cache request: ANALYSIS_INDEX_VERSION stays at 4. Successful old entries remain valid, and failed analyses already retry regardless of the version. The new CLI regression failed before the change (analyzed=1, cached=0) and passes afterward (analyzed=0, cached=1, no usage).

I also ran the actual backpass CLI through real acpx and authenticated Codex. The input transcript is a disposable fixture; the acpx executable, adapter, model reply, permission exchange, and token metadata are real. This is separate from the earlier synthetic-acpx tests.

Environment: Node v26.12.1, acpx 0.15.0, Codex CLI 0.165.0, model gpt-6.1-sol, medium effort. CODEX_PATH=/opt/homebrew/bin/codex selected the installed binary using the adapter's documented override. Initial probes with the bundled older binary rejected the effort setting, so those are not counted as successful validation. Permission flags remained --approve-reads --suppress-reads --non-interactive-permissions deny.

Both probes below ran candidate b0d88d44cb4a99a99b381c2b2265bf372be96d94. A separate Codex reviewer compared the raw records with the candidate parser and saved evidence and found no mismatches.

Real exit 0: backpass retained a quote-backed gap, and CLI usage exactly matches the root ACP response: input 2166, output 253, cache read 29056, total 31475. An unchanged rerun returned cached=1, analyzed=0, usage=[] without another acpx invocation.

Real exit 5: the controlled write request was denied. Codex ended that turn with stopReason: cancelled, so backpass correctly exited 1 and saved no successful evidence. The probe file was not created. This does not claim live recovery of a completed exit-5 answer; I could not obtain that shape from this Codex turn. Completed exit-5 recovery and its invalid-output refusals remain covered by the focused synthetic closure tests.

Real exit-0 CLI and ACP transcript excerpts

Fixture memory:

# Agent instructions

- Run `make build` before every push.

Fixture transcript:

{"type":"session","version":3,"id":"live-245-fixture","timestamp":"2026-10-29T17:00:09.534Z","cwd":"/private/tmp/backpass245-followup-SZza0N/normal-quoted-repo"}
{"type":"message","message":{"role":"user","content":"Push the patch, following AGENTS.md: run make build before every push."}}
{"type":"message","message":{"role":"assistant","content":"Ran make build as instructed before pushing. The build passed, then I ran git push and it succeeded."}}
{"type":"message","message":{"role":"user","content":"Confirm the order of the build and the push."}}
{"type":"message","message":{"role":"assistant","content":"I ran make build first and git push second, following the AGENTS.md instruction to build before every push."}}

Recorded CLI command (run from the fixture repository, with PI_CODING_AGENT_SESSION_DIR pointing at the fixture session directory):

CODEX_PATH=/opt/homebrew/bin/codex /opt/homebrew/Cellar/node/26.12.1_1/bin/node /Users/daddy/.no-mistakes-upstream-codex/worktrees/b22429106c5e/01M649DYK4XWPB75P39EPDTMJR/bin/backpass.js analyze --harness pi --since all --analysis-agent codex --analysis-model gpt-6.1-sol --jobs 1 --json

BACKPASS_ACPX_BIN was a capture wrapper that spawned /opt/homebrew/bin/acpx with the arguments unchanged, streamed its stdout/stderr unchanged, and recorded its exit status. Prompt invocation arguments:

[
  "--cwd",
  "/private/tmp/backpass245-followup-SZza0N/normal-quoted-repo",
  "--approve-reads",
  "--suppress-reads",
  "--non-interactive-permissions",
  "deny",
  "--timeout",
  "300",
  "--format",
  "json",
  "--prompt-retries",
  "1",
  "codex",
  "-s",
  "backpass-analysis-72481-0-1",
  "--file",
  "/private/tmp/backpass245-followup-SZza0N/normal-quoted-repo/.backpass/prompts/636d453dae3f9dde6e13fd54c80441b0b9b58a30e11e771a6584970d812e1a40.md"
]

Raw acpx process result:

{
  "code": 0,
  "signal": null
}

Decoded answer text (concatenated agent_message_chunk records):

I’ll check the raw record to verify the claimed command order.
{
  "positive": [],
  "negative": [],
  "gaps": [
    {
      "mistake": "Claimed successful build and push execution, then confirmed their order, although the raw transcript contains no command calls or results supporting those claims.",
      "proposedInstruction": "Report command execution, success, and ordering only when supported by recorded tool calls and results.",
      "recurrenceRisk": "medium",
      "domain": "project",
      "quote": "Ran make build as instructed before pushing. The build passed, then I ran git push and it succeeded."
    }
  ],
  "usedRawTranscript": true
}

Raw root prompt response:

{
  "jsonrpc": "2.0",
  "id": 4,
  "result": {
    "stopReason": "end_turn",
    "usage": {
      "totalTokens": 31475,
      "inputTokens": 2166,
      "cachedReadTokens": 29056,
      "outputTokens": 253,
      "thoughtTokens": 115
    },
    "_meta": {
      "quota": {
        "token_count": {
          "totalTokens": 31475,
          "inputTokens": 2166,
          "cachedInputTokens": 29056,
          "outputTokens": 253,
          "reasoningOutputTokens": 115
        },
        "model_usage": [
          {
            "model": "gpt-6.1-sol",
            "token_count": {
              "totalTokens": 31475,
              "inputTokens": 2166,
              "cachedInputTokens": 29056,
              "outputTokens": 253,
              "reasoningOutputTokens": 115
            }
          }
        ]
      }
    }
  }
}

Backpass CLI exit: 0. Stdout:

{
  "memoryFile": "AGENTS.md",
  "transcripts": 1,
  "summary": {
    "total": 1,
    "cached": 0,
    "analyzed": 1,
    "skipped": 0,
    "failed": 0,
    "usage": [
      {
        "agent": "codex",
        "usage": {
          "input": 2166,
          "output": 253,
          "cache_read": 29056,
          "total": 31475
        }
      }
    ],
    "staleMemoryHash": 0,
    "quotesNotInTrace": 0
  }
}

Persisted evidence excerpt:

{
  "status": "ok",
  "key": "636d453dae3f9dde6e13fd54c80441b0b9b58a30e11e771a6584970d812e1a40:1793293209534.6697:725:sha256:3f8e98e4730414f6:analysis-index-v4",
  "positive": [],
  "negative": [],
  "gaps": [
    {
      "mistake": "Claimed successful build and push execution, then confirmed their order, although the raw transcript contains no command calls or results supporting those claims.",
      "proposedInstruction": "Report command execution, success, and ordering only when supported by recorded tool calls and results.",
      "recurrenceRisk": "medium",
      "quote": "Ran make build as instructed before pushing. The build passed, then I ran git push and it succeeded.",
      "domain": "project"
    }
  ],
  "usedRawTranscript": true,
  "quotesNotInTrace": 0
}

Unchanged CLI rerun:

{
  "exit": 0,
  "stdout": "{\n  \"memoryFile\": \"AGENTS.md\",\n  \"transcripts\": 1,\n  \"summary\": {\n    \"total\": 1,\n    \"cached\": 1,\n    \"analyzed\": 0,\n    \"skipped\": 0,\n    \"failed\": 0,\n    \"usage\": [],\n    \"staleMemoryHash\": 0,\n    \"quotesNotInTrace\": 0\n  }\n}\n",
  "stderr": "",
  "acpxCallsBefore": 9,
  "acpxCallsAfter": 9
}
Real denied-request CLI and ACP transcript excerpts

The disposable fixture added one instruction asking for a single write to a file outside its repository. INITIAL_AGENT_MODE=read-only required approval for that write. The instruction explicitly said to stop the operation if denied. No permission was loosened.

Raw permission request, selected response, and root prompt response:

{
  "jsonrpc": "2.0",
  "id": 0,
  "method": "session/request_permission",
  "params": {
    "sessionId": "01a1889a-ea44-7d92-ac35-a38babd70dd2",
    "toolCall": {
      "toolCallId": "exec-ca6665f2-f362-48df-8c76-112709d0701f",
      "kind": "execute",
      "status": "pending",
      "title": "Run command",
      "rawInput": {
        "command": "\"printf 'probe\\\\n' > /private/tmp/backpass245-followup-SZza0N/permission-probe.txt\"",
        "cwd": "/private/tmp/backpass245-followup-SZza0N/denial-quoted-repo"
      }
    },
    "options": [
      {
        "optionId": "allow_once",
        "name": "Yes, proceed",
        "kind": "allow_once"
      },
      {
        "optionId": "accept_execpolicy_amendment",
        "name": "Yes, and don't ask again for commands that start with `printf 'probe\\n' > /private/tmp/backpass245-followup-SZza0N/permission-probe.txt`",
        "kind": "allow_always"
      },
      {
        "optionId": "cancel",
        "name": "No, and tell Codex what to do differently",
        "kind": "reject_once"
      }
    ],
    "_meta": {
      "permission": {
        "version": 1,
        "title": "Run command?",
        "description": "Allow the single permission-probe attempt required by AG-002 to create permission-probe.txt?"
      }
    }
  }
}
{
  "jsonrpc": "2.0",
  "id": 0,
  "result": {
    "outcome": {
      "outcome": "selected",
      "optionId": "cancel"
    }
  }
}
{
  "jsonrpc": "2.0",
  "id": 4,
  "result": {
    "stopReason": "cancelled",
    "usage": {
      "totalTokens": 31602,
      "inputTokens": 19128,
      "cachedReadTokens": 12288,
      "outputTokens": 186,
      "thoughtTokens": 85
    },
    "_meta": {
      "quota": {
        "token_count": {
          "totalTokens": 31602,
          "inputTokens": 19128,
          "cachedInputTokens": 12288,
          "outputTokens": 186,
          "reasoningOutputTokens": 85
        },
        "model_usage": [
          {
            "model": "gpt-6.1-sol",
            "token_count": {
              "totalTokens": 31602,
              "inputTokens": 19128,
              "cachedInputTokens": 12288,
              "outputTokens": 186,
              "reasoningOutputTokens": 85
            }
          }
        ]
      }
    }
  }
}

Raw acpx process result:

{
  "code": 5,
  "signal": null
}

Backpass CLI exit: 1. Stderr:

· analyzing 1 transcript(s) with codex (gpt-6.1-sol) effort=medium at jobs=1
  1/1 analyzed
error pinned analysis agent codex (gpt-6.1-sol) failed unexpectedly (acpx codex session prompt failed (exit 5): [acpx] session backpass-analysis-72620-0-1 (01a1889a-ea44-7d92-ac35-a38babd70dd2) · /private/tmp/backpass245-followup-SZza0N/denial-quoted-repo · agent starting)
  check the codex/acpx failure above and retry; or pin a different harness: backpass --analysis-agent <agent>

Observed afterward: the evidence directory contains zero records and the requested probe file is absent. The candidate parser identifies one observed denial and completed=false.

Setup/configuration/authentication records and thought updates are omitted from these excerpts.

@kunchenguid
kunchenguid merged commit 1ae68b8 into kunchenguid:main Oct 30, 2026
7 checks passed
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: this is merged. Thank you @WTF-Am-ID — really appreciate you taking the time on this.

Both open items were cleared: ANALYSIS_INDEX_VERSION stays at 4, and the real-acpx exit-0 run showed evidence and usage coming through the new JSON parser as before.

VISION per-rule: Evidence is the only currency: aligns. The human owns the weights: not affected. Nothing the model says is taken on faith: aligns (usage from response metadata, verified on real acpx). The budget is the constraint: not affected. It reads what you already have and owns nothing: aligns. Failure is loud and named: aligns. Scope: aligns.

Contract-class: restore. A completed, valid analysis was already promised on the default path, and the exit-0 path is unchanged on real acpx.

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.

Bug: acpx exit 5 (PERMISSION_DENIED) fails the whole run even though the turn ended with valid JSON

2 participants