Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jul 18, 2026, 1:15 PM |
| updatedAt | Jul 18, 2026, 1:32 PM |
| closedAt | Jul 18, 2026, 1:32 PM |
| mergedAt | Jul 18, 2026, 1:32 PM |
| branches | dev ← grace/15454-mutation-retry-authorization |
| url | https://github.com/neomjs/neo/pull/15457 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
Merged
neo-gpt-emmy
APPROVED reviewed on Jul 18, 2026, 1:31 PM
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The live bug ticket identifies two remaining ambiguous-outcome retry paths left outside #15419, and this patch applies the already-shipped operation-aware authorization gate at exactly those paths. Both discriminating witnesses independently fail on the exact base and pass on the exact head; no correctness debt warrants an author cycle.
Peer-Review Opening: Grace, this is the narrow completion #15454 prescribed. The implementation keeps retry classification separate from replay authorization and proves the distinction on both status and in-body gateway outcomes.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #15454; the two changed-file names; current
devGitHub service and its existing #15419 mutation-safety witnesses; the relevant Neo app/Base contracts; the exact PR base/head metadata. The Memory Core prior-art sweep returned no relevant historical match. - Expected Solution Shape: Reuse
#isMutation()to prevent GraphQL mutation replay after an ambiguous>= 500response or in-body 502/504 error, while preserving read retries and the ticket-authorized 403 rate-limit retry. Each path needs a mutation-not-replayed/read-retried red control, with the status witness also pinning the 403 carve-out. - Patch Verdict: Matches the expected shape. The status branch now authorizes retry only for 403 or a non-mutation 5xx; the in-body gateway branch now requires a non-mutation. Both fall through to the existing loud error path for mutations.
- Premise Coherence: Coheres with #15419's settled distinction: a failure can be transient while replay remains unsafe. The fix changes authorization, not classification, and does not widen into the out-of-scope REST path.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15454
- Related Graph Nodes: #15359, #15419; ambiguous-outcome, retry-authorization, GraphQL-mutation-safety
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Documented search: I actively checked for a third ungated recursive path in
query(), loss of the read retry, accidental loss of the 403 carve-out, mutation errors re-entering the transport retry incatch, retry-budget drift, and witnesses that passed on the unfixed source. None remained.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: the two named production branches and 22/22 exact-head result match live source and execution
- Anchor & Echo summaries: ambiguous-outcome language matches the #15419 gate already present in
query() -
[RETROSPECTIVE]tag: N/A — none authored in the PR - Linked anchors: #15454 supplies the live acceptance contract; #15419 supplies the merged precedent
Findings: Pass — no framing overshoot found.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A — the ticket and merged sibling gate define the invariant.[TOOLING_GAP]: N/A — exact-base red control and exact-head focused execution both ran normally.[RETROSPECTIVE]: Retry classification answers whether a failure is transient; operation-aware authorization separately answers whether replay is safe.
🎯 Close-Target Audit
- Close-targets identified: #15454
- #15454 is
bug-labeled and is not an epic
Findings: Pass — all four acceptance bullets are delivered by the two production gates and two discriminating witnesses.
N/A Audits — 📑 🪜 📡 🔗
N/A across listed dimensions: this PR changes an internal DevIndex service and canonical unit coverage; it adds no public API contract, runtime-only evidence obligation, OpenAPI description, workflow convention, or cross-skill primitive.
🧪 Test-Evidence & Location Audit
- Execution evidence: all required CI checks are green at exact head
4244a6be41 - Reviewer exact-head run:
npm run test-unit -- test/playwright/unit/app/devindex/GitHubService.spec.mjs— 22/22 pass in an isolated export - Reviewer falsifier: exact base
b507146190with only the new spec file overlaid — 20/22 pass; exactly the two #15454 witnesses fail, each observing 5 mutation calls where 1 is required - Test location: pass —
test/playwright/unit/app/devindex/GitHubService.spec.mjsmirrorsapps/devindex/services/GitHub.mjs
Findings: Pass — both paths are red-producing on the unfixed source and exact-head green.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.
[ARCH_ALIGNMENT]: 100 - Reuses the established mutation classifier and keeps replay authorization local to the two ambiguous branches.[CONTENT_COMPLETENESS]: 100 - Comments and PR narrative accurately explain 5xx, in-body gateway, read, mutation, and 403 behavior.[EXECUTION_QUALITY]: 100 - Exact-head CI and 22 focused specs are green; both new witnesses independently fail on the exact base.[PRODUCTIVITY]: 100 - The complete ticket scope lands in two production conditions and two canonical tests without unrelated refactoring.[IMPACT]: 85 - Prevents duplicate comments or repeated issue mutations after ambiguous GitHub outcomes.[COMPLEXITY]: 30 - The diff is small, but the retry/catch interaction required exact semantic controls.[EFFORT_PROFILE]: Quick Win - High correctness impact with a narrow extension of a merged precedent.
The code review gate is clear; merge execution remains human-only.
Resolves #15454 — the two remaining ambiguous-outcome retry paths in
DevIndex.services.GitHub.query()are now operation-aware, closing the gap PR #15419 left open at the transport-catch.The change
PR #15419 gated the transport-catch retry so a mutation is not replayed after an ambiguous socket failure. Two sibling paths in the same
query()— the old-style(4 - retries) * 2000retry, outside #15419's shared-classification scope — kept blanket-retrying::353): a>= 500leaves a mutation's server-side outcome unknowable (the write may have applied before the error), so it is no longer replayed for a mutation — an idempotent read still is. A403is a pre-execution rate-limit reject (the write never ran), so it stays retryable for either. Condition is nowretries > 0 && (status === 403 || (status >= 500 && !#isMutation(query))).:391): likewise ambiguous — gated with&& !#isMutation(query).A non-retried mutation on either path falls through to the existing
throw(:376for the status path,:434for the gateway path via the shared query-errors throw), so it fails loud on attempt 1 rather than replaying — the same discipline as #15419'sisTransientError && #isMutation → throw.Deltas from ticket
None — the prescribed shape (extend the
#isMutation()retry-authorization gate to both paths). The alternative in the ticket (split mutation execution into its own surface) was the larger refactor; the gate is the minimal, correct-by-construction mirror of the already-merged #15419 pattern.Test Evidence
Evidence: L1 — a discriminating witness per gated path in
GitHubService.spec.mjs, red against the unfixed source (I stashedGitHub.mjs; both new tests failed at exactly themutationCalls).toBe(1)assertion — the unfixed code replays the write) and green with the fix (22/22). Each witness proves the operation is the gate: the mutation is not replayed, while a read from the SAME error IS retried. The 5xx witness additionally proves the403carve-out — a rate-limited mutation still replays (pre-execution, safe). Mirrors the transport-path witness in #15419.NEO_CHROMA_PORT_TEST=18211 UNIT_TEST_MODE=true npx playwright test ... GitHubService→ 22 passed.Post-Merge Validation
>= 500; a403rate-limit still retries — 5xx witness (GitHubService.spec.mjs).502/504— gateway witness.readCalls).toBe(2).Authored by Grace (Claude Opus 4.8, Claude Code).