gate: bind decisions to a decider, record what landed, and measure the bypass

[itsHabib]

Four audit gaps in gate's decision record, each closed by a separate commit. Every claim below was verified against the live ledger (~/dev/gate/state/log.jsonl, 4,900+ artifacts), not inferred.

1. who binding — a judgment named nobody (25d6df0)

All 237 judgment artifacts in the ledger carry the identical body key set: confidence, decision, producer, source, subject, tier, why. No who, anywhere. So a human approval and an agent-composed one are indistinguishable in the record, and the operator's 129 producer.impl=operator approvals name nobody. That is the separation-of-duties gap.

Verdict gains an optional decider {who, method, at}. method is the channel the identity was established through — cli-operator, slack-interactive, auto-<provider> — and names the channel, never an authenticated claim; gate authenticates none of them. The same operator string can be typed by an agent at a shell or produced by a signature-verified Slack callback, and only the channel separates those. at is the decider's own clock, deliberately distinct from the envelope's append time: a phone tap and its later append are two different, both-true facts.

Enforced on the write path only (RequireDecider, ahead of the append). Every reader stays tolerant, because the 237 judgments recorded before the field existed must still explain and re-reduce — a reducer that rejected them would trade one unanswerable question for a log that cannot be read at all. explain renders absence as the literal unattributed rather than omitting the line; a blank tells a reader nothing about whether the question was even asked.

-auto derives its own decider (resolved provider wrapper + model) and refuses a claimed one. The submitted-artifact path records the submitter, since the model did not choose to submit it. Schema v0.3.0 → v0.4.0, additive and omitempty.

2. receipt — gate could not say what landed (223b887)

All 163 action artifacts carry dry_run: true / would_merge. State can prove a merge was allowed and cannot say whether it happened, when, as which commit, or by whom.

gate receipt -run <id> discharges one authorization. Nothing about the landing comes from the caller — merge commit, actor, and timestamp are read back from the GitHub API, giving the record an independent clock and an independent actor rather than an executor's account of its own behavior, which is exactly the claim a receipt exists to check. Classification is head-to-head, not "did it merge": a PR merged at a head the action never saw is superseded, never a clean discharge, because joining on the PR number alone would rebuild — one layer up, in the reporting — the laundering --match-head-commit prevents. One receipt per action is structural (the store's absent-parent guard).

Gate still performs no merges. Gate authorizes, an executor acts, gate receipt records. Collapsing those would put the decision and the effect in one process.

3. reconcile — gate could not prove the negative (223b887)

Every other surface reads gate's own decisions back and can only report what gate did. gate reconcile -repo R [-since] reads the platform first and asks what gate can account for, so an absence of artifacts becomes visible as an absence rather than as silence. It writes a coverage artifact: authorized-and-landed, authorized-never-landed, landed-without-authorization.

Pre-adoption merges are classified separately and never counted as bypasses — reporting history alongside a real bypass is how a real bypass gets ignored. The boundary defaults to a fact state already holds: the first artifact naming the repo, i.e. the control took effect when the control first ran. No config file, no custody surface, nothing to remember, and it cannot go stale. -effective-from overrides.

The basis is stated on the artifact (merged-pull-requests): a direct push to the protected branch has no PR and no head to join by, so it is outside this claim and belongs to branch protection. A report that quietly omitted it while implying it covered everything would be worse than no report.

gate audit reports both anomalies as first-class findings, and keeps integrity and accountability apart: a tampered chain still fails; an incomplete record prints and exits 0. Before any reconcile it says UNMEASURED, not zero.

Live output today:

chain intact
authorization without receipt (163)
unattributed judgments (238)
note: no repo has been reconciled, so merge-without-authorization is UNMEASURED, not zero.

4. A killed resolve stranded an authorization (d56fdb5)

itsHabib/ivy#22, run_fe7ac73ddb7c59a7: judgment jdg_b65ffd12563e8d85 written at 02:01:14Z, then nothing. No verdict, no action, no resolution. The PR stayed open, gate next still called the run parked, and the Slack card said gate had authorized the merge.

serve.process gave one 25s budget to the grant lookup (gate next -json, a full-log projection over a ledger that only grows) and the decision (gate resolve, which appends judgment → verdict → action separately), and exec.CommandContext SIGKILLs on expiry. A deadline landing mid-sequence does not cancel a decision, it strands one.

Why raising resolveTimeout would have been the wrong fix: it widens the window rather than closing it. The window is only harmless once (a) the decision cannot lose a race to the network and (b) a strand is visible and finishable. So:

• Separate budgets. The lookup is a read that can be killed for free. The decision writes durable state and gets a hang guard, not a latency budget — the two outcomes are not symmetric: too short destroys an authorization and needs a human to notice; too long delays a card that was already acked. What actually bounds latency lives inside gate, where the phases are distinguishable.
• Ordering is now a precondition of the payload. stamp.Post requires the action artifact's chain hash, which exists only once that artifact is durable — so no GitHub call can precede the authorization it decorates. Not a convention about call order a later edit could invert. Pinned by test.
• The stamp's outcome is reported, on the result JSON, not only logged. "Best-effort" was half true while a failed post went to a stderr nobody read and the card reported unqualified success.
• Stamp budget is now one small total for the whole attempt instead of one per gh call (two sequential calls could spend double), and gh is resolved before invocation so an unresolvable binary reports as itself.
• gate next names the state: judged but not authorized, with the command that finishes it. Verified against the live ledger — it finds exactly ivy#22 across 4,900+ artifacts.

Is ivy#22's one-shot judgment spent?

No. Reasoned from the code, not by running gate judge. applyJudgment checks artifactForParent(arts, KindJudgment, escalationID) first and, when a judgment exists, takes resumeJudgment — which loads the persisted judgment, re-checks its grant lineage, refuses only a retry whose -decision contradicts the record, then re-reduces and acts. judgment_duplicate fires only from a fresh append racing an existing one, or from finishJudgment when an outcome already exists. Run run_fe7ac73ddb7c59a7 has a judgment and no reduced verdict and no action, so it takes the resume path and completes. Not a P1; the gap was that nothing said so, which commit d56fdb5 fixes.

A judgment recorded before the decider binding stays completable for the same reason: resume never rebuilds the judgment, so the new write-path requirement cannot strand a decision already in the log. Pinned by a test built on ivy#22's exact shape.

Tests

• verify: decider vocabulary (empty who, unknown method, bare auto- prefix, non-RFC3339 clock all rejected); RequireDecider bounds judgments only; Reduce stays tolerant of unattributed legacy judgments.
• gate: a judgment artifact carries its decider on the wire; an unattributed one is refused before the one-shot is spent and the escalation stays judgeable; the CLI flag matrix.
• ledger: head-vs-merge classification (merged / superseded / abandoned / failed / not-landed); the platform clock stays distinct from gate's; the three reconcile classes; the effective-from boundary at ±1s and exactly on it; findings never include pre-adoption.
• gate: a receipt discharges exactly one action (second refused, no state mutation); audit findings appear and clear.
• observe: stranded-run detection, plus the negatives — any completed outcome, and a re-park, are not stranded.
• escalate: the binary-resolution self-check fires and names gh, gate/authorized, and PATH; an absolute -gate is taken literally.

gofmt, go vet, golangci-lint (0 issues), go test, and go test -race all clean.

Notes for review

• Scope split. A concurrent agent owns cmd/flare (including the launchd plist templates in cmd/flare/scripts/) and the escalate card rendering. This PR therefore does not fix the plist PATH template — the live plist is hand-patched and flare-launchd.sh install will still regress it. cmd/escalate/internal/serve/serve.go is touched here for the budget seam only; expect a rebase.
• Contract placement. receipt / coverage bodies stay gate-internal: gate is the only writer and reader today. Promotion to contracts/ with an embedded schema is the repo's lazy-migration trigger — the first time a second tool touches one.
• Deliberately not done. gh call sites still resolve via PATH rather than an absolute path pinned at startup. That is the repo's own test seam (docs/features/tier-aware-panel/driver.md: a fake gh on PATH observes the argv with zero production change). The failure is instead made loud — resolved-and-named at the call site, and refused at ingress startup.

🤖 Generated with Claude Code

[itsHabib]

@claude review

[claude]

Claude finished @itsHabib's task in 4m 23s —— View job

---

PR #249 Review

• Load tools
• Read key changed files
• Analyze contracts/verdict changes (decider binding)
• Analyze receipt & ledger logic
• Analyze reconcile/coverage logic
• Analyze escalate serve timeout fix
• Analyze observe/stranded detection
• Post comprehensive review

---

Summary

A well-designed, production-quality PR. Each of the four audit gaps is closed with a clear mechanical argument (write-path refusal vs. read-path tolerance, independent clock, structural one-receipt-per-action, separate budgets). The code comments are among the most thorough in the repo. Two concrete findings and a few lower-priority notes follow.

---

P1 — lookupMergedPRs records the PR author, not the merger

File: cmd/gate/receipt.go:212-213

// reconcile sweep path
Actor: r.Author.Login, // PR creator, not the person who merged

// single-PR receipt path
l.Actor = view.MergedBy.Login // ✓ the actual merger

cmdReceipt's lookupLanding correctly captures merged_by.login — the human who clicked Merge. lookupMergedPRs (the reconcile sweep) requests author from gh pr list --json, which is the PR creator. In renderCoverage, the bypass line prints by=<actor> — so a genuine bypass attributes "who merged without authorization" to the PR author rather than the person who performed the unauthorized merge. On any team where authors don't self-merge, this is wrong.

gh pr list --json accepts mergedBy as a valid GraphQL field. The fix adds it to the struct and the --json field list:

type mergedPR struct {
    // ...existing fields...
    MergedBy struct {
        Login string `json:"login"`
    } `json:"mergedBy"`
}
// and in lookupMergedPRs:
Actor: r.MergedBy.Login,

Fix this →

---

P2 — reconcileLimit = 500 is a silent ceiling with no artifact disclosure

File: cmd/gate/receipt.go:229, cmd/gate/receipt.go:430-432

const reconcileLimit = 500

The coverage artifact records BasisMergedPullRequests ("what this claim is about") but not "up to N of them." On a repo where more than 500 PRs merged in the reconcile window, the oldest merges in the window are silently absent. An operator reconciling after a busy sprint could get a "0 bypasses" reading that omits the first half of the window.

The PR description's own rule (from workbench-101): "No silent caps: if a workflow bounds coverage, log() what was dropped." The render path (renderCoverage) and the Coverage struct should at minimum state the limit so a reader knows the claim's actual scope.

Minimal fix: record limit on the artifact and print it in renderCoverage:

// in Coverage struct
Limit int `json:"limit,omitempty"` // 0 means unlimited / unknown

// in renderCoverage
if c.Limit > 0 {
    fmt.Fprintf(w, "note: sweep covered the newest %d merged PRs; older merges in window are not included\n", c.Limit)
}

---

P3 — AuthorizedNeverLanded is silently filtered by authorization time

File: cmd/gate/internal/ledger/coverage.go:150-159

for _, a := range auths {
    if !Authorizes(a.Outcome) || matched[a.Action] {
        continue
    }
    if a.At.Before(w.Since) || a.At.After(w.Until) {
        continue // authorization issued outside the window → silently absent
    }

An authorization issued 60 days ago whose PR is still open does not appear in a 30-day window sweep's authorized_never_landed — the one class that would flag "something was authorized and never closed." The operator sees no outstanding entry, not that it was excluded. The effective-from separation for pre-adoption merges is well-documented; this asymmetric exclusion is not. Worth a prose comment next to the filter, or extending CoverageRow.Why to surface these as an out-of-window class.

---

Observations (no action required)

strandedFrom resume command template with -who NAME (cmd/gate/internal/observe/stranded.go:133-136): The -who NAME placeholder is correct for the "on resume gate checks -decision but re-uses the persisted judgment's -who" semantics. The actual -who the operator passes doesn't matter to state; it just needs to not be empty (the flag parser requires it). The template makes this slightly ambiguous — a reader might wonder whether NAME matters. A short comment (// gate loads the persisted decider; this is checked only for non-empty) would close that.

sweepReceipts mutates c.AuthorizedAndLanded before e.st.Append (cmd/gate/receipt.go:324-358): If e.st.Append fails after receipts were swept, the coverage artifact is lost but the receipt artifacts are durable (correct). A re-run picks them up. The code is correct; the comment could note this invariant explicitly.

unattributedJudgments in audit vs. StrandedRun.Decider in next (cmd/gate/internal/observe/accountability.go:133-151, cmd/gate/internal/observe/stranded.go:145-149): Both render absence as "unattributed" and the logic is deliberately duplicated (the comment explains why). Good.

escLocks entries are never reclaimed (cmd/escalate/internal/serve/serve.go:223): The comment bounds this correctly ("bounded by the escalations a run ever parks"). Acknowledged in the code. Fine.

decideTimeout = 5 * time.Minute (cmd/escalate/internal/serve/serve.go:98): The reasoning is sound — it's a hang guard, not a latency budget, and the decision is upstream of any network call now. The one-sentence argument ("a deadline that is too SHORT destroys an authorization; too LONG delays a card that was already acked") is the clearest statement of the asymmetry I've seen in any codebase. Worth keeping verbatim.

---

What's verified correct

• Decider binding (write-path only, tolerant reader): RequireDecider bounds judgments; Reduce ignores the field; Attribution renders absence as "unattributed". Conformance test pins schema←→Go alignment. TestDeciderIsOptionalAndAdditive pins the additive contract. All correct.
• Receipt one-per-action: AppendIfAbsentParentKinds keyed on the action artifact makes "second receipt refused" structural, not a check. The error message (receipt_duplicate) is observable. Correct.
• Head-to-head classification: classify joins on landing.HeadSHA == auth.Head, not on PR number alone. OutcomeSuperseded is the right name for "merged but not at the authorized head." TestReceiptClassifiesOnTheHeadNotOnTheMerge covers all four outcomes. Correct.
• Stamp ordering: validate() requiring Hash != "" (the action's chain hash, which only exists after the artifact is durable) structurally prevents any GitHub call from preceding the authorization it decorates. The comment is precise about the mechanism. Correct.
• Separate budgets in serve.resolve: grantTimeout (20s, kill-safe) vs. decideTimeout (5m, hang guard). The ivy#22 post-mortem in the constant's doc is the best in-repo example of "here is the exact failure mode this constant prevents." Correct.
• strandedRuns detection: The newest map (action or escalation), judgments map, and parent check hasParent(judgment, terminal.ID) together correctly identify "judgment recorded, no outcome followed." The sort on JudgedAt then Run makes the output deterministic. Correct.
• Pre-adoption boundary derivation: DerivedEffectiveFrom uses the earliest artifact timestamp for the repo, not a config file. Zero-config and cannot drift. Correct.
• SSRF guard on response_url: checkSlackURL requires https and a slack.com host before any POST. Correct defense-in-depth given the payload is already signature-verified.

---

Test coverage assessment

The tests cover what they need to: the write-path refusal (TestRequireDeciderBoundsTheWritePathOnly), the tolerant read (TestReduceStaysTolerantOfUnattributedJudgments), the absence rendering (TestAttributionNamesTheAbsence), all four receipt outcomes (TestReceiptClassifiesOnTheHeadNotOnTheMerge), the one-receipt structural constraint, the stranded-run detection, and the effective-from boundaries. The conformance test binding schema to Go types is particularly good (it checks omitempty alignment, not just field presence). No test gaps relative to the stated claims — though the actor-attribution bug (P1) would be caught by a test that verified reconcile's Actor field against a fixture using mergedBy.

---

| Branch

[itsHabib]

@codex review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[github-actions]

Reviewer: claude
Reviewed commit: 817dd3baac6444f3b43c97eecabcb27880f62ff8

[itsHabib]

@claude review

Round 1 fixes pushed at 65ee29f2c6f3fb6889157a7d5c772d3cd3c27d0b. All three findings addressed:

• P1 cmd/gate/receipt.go — mergedPR now requests and reads mergedBy, so a bypass row names the merger rather than the PR author. Verified mergedBy is a valid gh pr list --json field against this repo.
• P2 — Coverage gains limit + truncated; renderCoverage prints the ceiling, and prints INCOMPLETE with a narrow--since instruction when the sweep reaches it.
• P3 — Coverage.OutstandingOutsideWindow counts authorizations issued before the window that nothing discharged, rendered as a line telling the reader to widen -since. The list stays window-scoped; only the silence is gone. Pinned by TestReconcileCountsOutOfWindowOutstandingAuthorizations.

Both observations are now comments in the code (the -who NAME placeholder's semantics, and the receipts-before-coverage append ordering).

[claude]

Claude finished @itsHabib's task in 3m 37s —— View job

---

PR #249 — Round 2 Review

• Load tools
• Read Round 1 fix changes (65ee29f)
• Verify P1 fix: mergedBy field in reconcile sweep
• Verify P2 fix: limit/truncated disclosure
• Verify P3 fix: OutstandingOutsideWindow
• Verify both observations are now commented
• Check for new issues introduced by fixes

---

All three P1/P2/P3 fixes are correct, and both observations now carry exactly the comments that were requested. Two minor notes on implementation semantics below — neither is blocking.

---

P1 — Verified correct

mergedPR.MergedBy with json:"mergedBy" (GraphQL camelCase) is the right field for gh pr list --json. The REST single-PR path already used merged_by (snake_case) from the REST API, so the two paths now consistently capture the merger rather than the author. The comment explains why the field matters on a bypass row. Clean.

---

P2 — Verified correct; one implementation note

Truncated = len(landings) >= reconcileLimit is checked on the raw count from lookupMergedPRs, before inWindow() filters to the window. This is the right join: the question is whether the API call was truncated, not whether the window was truncated. The inWindow filter is irrelevant to that question.

One conservatism: on a repo with 500+ total merged PRs, the raw count hits the ceiling even if every window PR is present in the newest 500. The INCOMPLETE message says "narrow -since and re-run" — correct advice for real truncation, but a false positive here does nothing worse than prompt an unnecessary re-run with a shorter window (which self-clears by showing a complete scan). Acceptable.

---

P3 — Verified correct; one implementation note

OutstandingOutsideWindow counts unmatched outside-window authorizations without checking receipts[a.Action]. An authorization issued and discharged before the window — PR merged, receipt written, landing filtered out by inWindow — would be counted. This over-counts: the authorization is closed, but the count says it isn't.

The operator's natural response (widen -since to include the merge) resolves the false positive correctly: once the landing is inside the window, the authorization moves to AuthorizedAndLanded and exits the count. The render text already prescribes that action. Conservative and self-correcting; not a bug. A comment noting the receipt gap (analogous to the one added to sweepReceipts) could pre-empt a future reader wondering why receipts aren't checked, but it is not required.

---

Both observations — done

The ResumeCommand comment in stranded.go:51-53 now closes the "does NAME matter?" ambiguity precisely: the flag parser requires non-empty, the original decider is already in the log. The sweepReceipts ordering note in receipt.go:329-336 states the invariant and its consequence (a failed coverage append leaves durable receipts, re-runnable without loss). Both are exactly what was asked for.

---

What's verified correct (round 2 additions)

• mergedPR field rename: The --json field list is updated in sync with the struct; r.MergedBy.Login is used at the only call site that assigns Actor. No other path left using Author.
• Coverage.Limit assignment: coverage.Limit = reconcileLimit is set in cmdReconcile after Reconcile() returns, before Append. The field is omitempty so a zero value (impossible here) would be invisible; 500 is always written.
• coverage.Truncated vs coverage.Limit: The JSON/render path correctly handles Truncated=false, Limit>0 (prints ceiling) vs Truncated=true (prints INCOMPLETE). The omitempty on Truncated bool suppresses it from JSON when false — correct.
• TestReconcileCountsOutOfWindowOutstandingAuthorizations: Tests the exact P3 scenario — out-of-window authorization excluded from the list, counted in OutstandingOutsideWindow. Covers both the list and the count, not just one.

---

Round 2 is clean. Residual observations are pre-existing design tradeoffs, not defects introduced by the fixes.

---

| Branch

[itsHabib]

@codex review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[itsHabib]

@claude review

Round 2 fix pushed at 3c63b7ddf52d4786cd79b41f25ef6223c28b36d1 — the single residual worth taking.

OutstandingOutsideWindow now consults the receipt index. You flagged that an authorization issued and discharged before the window (merge landed, receipt written, landing filtered out by inWindow) was still counted as outstanding. Benign and self-correcting as you said, but it inflates the one number whose entire meaning is "authorized and never closed" — and this PR's whole subject is that these numbers be honest, so it earned the three lines. Pinned by TestOutOfWindowCountExcludesDischargedAuthorizations.

Deferred, with reasons:

• P2 truncation conservatism (raw count vs window count) — accepted as-is, for the reason you gave: the question Truncated answers is whether the API call was truncated, and a false positive on a repo with 500+ lifetime merges costs one unnecessary re-run that self-clears. Making it precise would require paginating to find out whether the window was actually complete, which is the cost the ceiling exists to avoid.
• gh call sites still resolve via PATH rather than an absolute path pinned at startup. That is the repo's own test seam (docs/features/tier-aware-panel/driver.md: a fake gh on PATH observes the argv with zero production change). The failure is made loud instead — resolved-and-named at the stamp call site, and refused at ingress startup by serve.Preflight.
• The launchd plist PATH template (cmd/flare/scripts/) is out of scope here by coordination: a concurrent agent owns cmd/flare. The live plist is hand-patched and flare-launchd.sh install will still regress it — that fix lands in their PR.

This is fix-round 2 of 2 under the repo's review-cycle discipline; anything further goes to the judge as recorded residual.

[claude]

Claude finished @itsHabib's task in 1m 30s —— View job

---

PR #249 — Round 3 Review

• Load tools
• Read round 2 fix (3c63b7d) — OutstandingOutsideWindow receipt-index consultation
• Verify fix correctness and test coverage
• Check for new issues introduced
• Post review

---

Fix verified — OutstandingOutsideWindow now consults the receipt index

cmd/gate/internal/ledger/coverage.go:173-178

if a.At.Before(w.Since) || a.At.After(w.Until) {
    if receipts[a.Action] == "" {
        c.OutstandingOutsideWindow++
    }
    continue
}

The three-line change is in exactly the right place. The comment above it is precise about the asymmetry: the receipt is the discharge, and a closed authorization has no business inflating the one number that means "never closed." The nil-map case (receipts passed as nil in TestReconcileCountsOutOfWindowOutstandingAuthorizations) is safe in Go — a nil map read returns "", so the nil branch still increments correctly. No new paths were opened.

TestOutOfWindowCountExcludesDischargedAuthorizations — well-constructed. Two out-of-window authorizations, one receipted (act_closed) and one not (act_open); the expected count is 1. Covers both the positive (counted) and the negative (excluded) in one fixture. The test name states the contract rather than the scenario, which is the right choice for a test that pins a definition.

---

Deferred items — all sound

• P2 truncation conservatism (raw count vs. window count): The Truncated field answers whether the API call was truncated, not whether the window was covered. Making it precise would require paginating to exhaustion — exactly what the ceiling exists to avoid. A false positive costs one unnecessary re-run that self-clears. Accepted.
• gh PATH resolution: The repo's own test seam (docs/features/tier-aware-panel/driver.md) depends on a fake gh on PATH. Pinning at startup would require a bespoke injection mechanism for that seam. The failure is made loud at the stamp call site and refused at ingress by serve.Preflight. Accepted.
• launchd plist PATH template: Correctly coordinated to the concurrent agent owning cmd/flare. Out of scope here.

---

What's correct (round 3 additions)

• The receipt check placement is in the out-of-window branch only. The in-window AuthorizedNeverLanded path already records receipts[auth.Action] on the row — no asymmetry introduced.
• The matched[a.Action] guard correctly precludes double-counting an out-of-window authorization that was matched to a landing (goes to AuthorizedAndLanded in the first loop); those never reach the out-of-window counter. The fix does not disturb this invariant.
• TestReconcileCountsOutOfWindowOutstandingAuthorizations (the existing test, with receipts: nil) still passes because a nil-map read in Go returns "", which takes the increment branch. The existing test was not broken by the new receipt check.

---

Round 3 is clean. This is fix-round 2 of 2 under the repo's review-cycle discipline; the record is complete for the judge.
| Branch