feat(flare): page grant_needed, collapse the repeated question, stop claiming health

[itsHabib]

Stacked on #251 → #247. Review the top commit; it retargets automatically as the stack lands.

Three ways the phone rung misinformed the operator, each verified against the live gate ledger.

1. grant_needed was dropped on the floor

gate records a grant_needed artifact every time a run is refused for want of authority — grant_absent, grant_expired, and (since #242) the pre-flight grant_cycle_exceeded. 18 of them in the live ledger. flare read them and threw them away.

It is the ONE alert an agent cannot act on for itself: it can re-run, re-review and re-judge, but it cannot mint — and the operator cannot mint from a phone either. So it must arrive early, while they are still near a keyboard, rather than being discovered later as stalled work.

Now its own card class, carrying the paste-ready remedy:

🔑 No live grant — roxiq is stopped
gate found no live grant for this repo, so it refused before gathering any evidence.
gate grant -repo itsHabib/roxiq -max-tier T2 -max-cycles 3 -ttl 24h -state /Users/mh/dev/gate/state

The ceilings come from what the repo already held — flare proposes what the operator has previously judged appropriate, never widens on its own, and never mints. A spent cycle budget proposes exactly one more cycle than was consumed.

flare digest answers the same question standing rather than one refusal at a time: per repo, what is parked, whether a grant stands, how soon it lapses. Only two situations qualify — parked work with no live grant (a hard stop) and a grant lapsing under parked work — because a digest that lists everything is another wall of text to skim. No pressure produces no card, and an unchanged picture never re-pages (its dedupe id is a hash of its own content). Parked counting mirrors gate's own inbox reduction: a PR is parked when its latest terminal artifact is an escalation.

2. The operator is asked the identical question over and over

318 of 355 parks in the live ledger lead with the identical readiness sentence. Attention to a repeated warning is spent by the second one, so the third onward buys nothing by restating it — while what is DIFFERENT about this park is never surfaced at all.

The fingerprint is the leading reason clause, and that choice is load-bearing. gate joins a park's reasons with "; " and the first is the primary one, so fingerprinting the whole line almost never matches even though the operator is reading the same opening sentence every time:

window	byte-identical whole line	leading clause
24h	26 / 357 (7%)	210 / 357 (58%)
7d (chosen)	52 / 357 (14%)	276 / 357 (77%)
all time	56 / 357 (15%)	277 / 357 (77%)

Past two deliveries for a repo, the card collapses:

Same opening reason as the last 3 parks in ivy: readiness: no review decision reported by GitHub — cannot verify readiness
New here: ivy#23 · tier T1 · head b33c512 · cycle 2 of 3
Also: review-panel-completeness: review panel state unknown: declaration

Nothing is suppressed. The collapse changes the card, never whether it is sent, and the clauses after the repeated opener — the part that is actually new — are shown rather than hidden. A different reason, or the same sentence about a different repo, never collapses (both pinned by tests).

3. flare status claimed health while nothing was getting through

Verified real, in code and by test. pollSource returned nil on a delivery failure, so cycle never marked the source failed: sweep exited 0 ("swept clean") and status reported healthy: true — while the ordered cursor sat behind an undeliverable event and every event after it was blocked too.

A stall is now recorded in cursors.json (when it began, the event it is stuck on, attempt count), status reports healthy: false and names it, sweep exits non-zero, and a source that polls cleanly clears its own stall. The retry semantics are unchanged — the cursor still holds, deliberately; only the silence is fixed.

This flips one assertion added in #251 (TestFailedCorrectionRetries asserted the cycle returned no error) — that test now asserts the honest contract, plus that the stall is recorded and cleared.

Shape

Raised on #251 and addressed here rather than deferred: the journal is replayed once per cycle into all three facts it holds (settled events, live cards, reason counts) instead of three separate full scans of a file that only grows. The cycle's fixed context (runner: config, journal, router, courier) and its mutable working set (cycleState) each travel as one value, which is what kept dispatch from growing to seven parameters.

Coordination note (concurrent gate work)

No file under cmd/gate/** and no launchd script is touched. grant_needed is consumed defensively: its two persisted body shapes decode into one struct with absent fields left zero, and a body flare cannot read still pages with less detail. KindGrantNeeded is named as the string gate persists, not imported.

The additive gate field that would help remains the one recorded in docs/FOLLOWUPS.md: the park's ceilings on the escalation body.

Checks

gofmt clean · go vet ./... · golangci-lint run ./... (0 issues) · go test ./...

New tests pin: all three refusal reasons paging with a correct mint, the collapse threshold and its negative cases, the digest's content/silence/dedupe-id, and flare status exiting 1 while stalled and 0 once delivery recovers.

🤖 Generated with Claude Code

[itsHabib]

@claude review

[claude]

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

---

Code Review

• Load tools and post initial comment
• Read changed files and diff
• Review: signal_test.go + lifecycle_test.go (new test coverage)
• Review: source/gatelog.go + source/ledger.go (grant_needed + digest)
• Review: notify/notify.go (collapse logic)
• Review: journal/journal.go + journal/cursors.go (stall tracking, replay)
• Review: main.go (dispatch, cycle state, new commands)
• Review: preflight/preflight.go + digest.go
• Review: docs and guide files
• Post final review

---

Three things this PR fixes that the data backs up (18 dropped grant_needed, 77% of parks with the same opener, healthy-reporting-while-stalled are all verified against the live ledger). The journal replay consolidation is a clean bonus. Nothing structurally wrong; findings below are one P2 in test code, one P2 value-receiver nuance, one P2 proposal asymmetry, and a few P3 nits.

---

Finding 1 — P2: itoa in signal_test.go silently corrupts test JSON for single-digit numbers

signal_test.go:95:

func itoa(n int) string { return string(rune('0'+n/10)) + string(rune('0'+n%10)) }

This zero-pads to two digits: itoa(5) → "05", which is valid JSON only as a string, not a number — the embedding "number":05 would be invalid JSON that the test helpers would happily write without warning. The current test values (30–40) are safe, but any future parkWithReason call with a single-digit number would silently produce a malformed artifact line and cause confusing test failures far from the source.

strconv.Itoa already lives in the module's dep tree; a one-import fix removes the fragility entirely. Fix this →

---

Finding 2 — P2: Replay.card mutates maps via a value receiver — misleads future readers

journal/journal.go:162:

func (r Replay) card(key string, e Entry) {
    if e.Kind == CardFinal {
        delete(r.Cards, key)   // side-effects the underlying map
        return
    }
    if e.Kind == Delivered && e.Card != nil {
        r.Cards[key] = *e.Card  // ditto
    }
}

This works because Cards and Seen are maps (reference types); the value-receiver copy shares the underlying table. But a value receiver signals "this method doesn't mutate the receiver", which is the opposite of what happens. A maintainer reading the call site r.card(key, e) inside a closure that already captures r by reference will reasonably expect no mutation — and will be surprised when r.Cards is modified.

A pointer receiver (r *Replay) makes the mutation contract explicit. Fix this →

---

Finding 3 — P2: digestLine proposes T2 for repos with no prior grant, while grantNeededMint uses lastCeilingsFor

In digest.go:147:

mint := "```" + preflight.Mint(r.Repo, r.state, r.Grant.MaxTier, r.Grant.MaxCycles) + "```"
if !r.Live {
    return fmt.Sprintf("*%s* — %s parked, *no live grant*\n%s", ...), true, true
}

When !r.Live, r.Grant is zero-value → MaxTier="" → preflight.Mint falls back to T2. The PR description says "flare proposes what the operator has previously judged appropriate; it never widens on its own". The per-refusal path does exactly this via grantNeededMint → lastCeilingsFor(repo). The digest, however, proposes the generic T2 fallback even for repos that historically ran at T3 — the two surfaces give the operator different guidance about the same repo.

This isn't a correctness bug (T2 is conservative), but it contradicts the stated invariant for the digest surface. Noted as a follow-up candidate. The fix would require threading the ledger's lastCeilingsFor result through Authorities or authorityRow, adding a field to Authority.

---

Finding 4 — P3: markRepeat mutates ev.Fields through the map reference implicitly

main.go:440:

func markRepeat(ev event.Event, reasons map[string]int) string {
    ...
    if n := reasons[key]; n >= repeatAfter {
        ev.Fields["repeat"] = strconv.Itoa(n)  // mutates the caller's map
    }
    return key
}

ev is a value parameter, but ev.Fields is a map[string]string, so the write to "repeat" is visible to all callers who share the underlying map. The mutation is intentional (downstream Route and send need to see it), but nothing in the signature communicates that. Worth a brief comment at the call site: // markRepeat writes ev.Fields["repeat"] through the shared map.

---

Finding 5 — P3: digestSource is non-deterministic with multiple gate log sources

digest.go:179:

func digestSource(cfg config.Config) string {
    for _, src := range cfg.Sources {
        if src.Kind == config.SourceGateLog {
            return src.Name
        }
    }
    return "flare"
}

This returns the first gate source in config order. With multiple gate log sources, the digest's SeenKey changes if sources are reordered, turning an unchanged digest into a "new" one and re-paging with identical content. authorityRows already aggregates across all sources; a stable name like "authority-digest" (or the constant "flare") would be safer, since the ID is already content-hashed and uniqueness comes from the hash, not the source name.

---

The three features — design and correctness observations

grant_needed paging: The tolerant decode (_ = json.Unmarshal) for the refusal body is the right call — an unparseable body still pages with less detail. The grantNeededMint arithmetic is clean: a cycle-exhausted budget proposes cycles_used + 1, a lapsed/absent grant proposes the widest ceiling the repo held. The KindGrantNeeded locality (string constant, not import) correctly respects the boundary law and is documented with why.

Reason collapse: The fingerprinting choice — leading clause of the "; "-joined question rather than the whole line — is exactly right, and the live-ledger data in the PR description makes the threshold choice legible. The repeatAfter = 2 constant means the third delivery triggers collapse (the first two land in full); the comment explains it. Nothing suppressed, just reordered.

Honest stall detection: The fix is minimal and correct. pollSource now returns a typed stalledError instead of nil on delivery failure; cycle records the stall in cursors.json via Cursors.Stall() and clears it via Cursors.Clear() on recovery. status adds len(cur.Stalled) == 0 to the health gate. The test (TestStalledSourceIsUnhealthy) cycles through failure, then recovery, asserting status code at each step — exactly the right boundary to pin.

Journal consolidation: Replacing j.Seen() + j.LiveCards() with a single j.Load(since) that gathers all three replay facts in one pass is a clean improvement. The Replay struct and the since time parameter (for windowed reason counts) compose without awkwardness. The old Seen() and LiveCards() methods are removed cleanly.

flare digest: The silence heuristic — two situations qualify, no-live-grant and expiring-under-parked-work; a running-fine repo is excluded — is the right cut. Dedup by content hash means an unchanged picture never re-pages. The 12h default for -within makes sense as "actionable before the expiry".

---

Summary

No blocking correctness bugs found. The logic across all three features is sound, the invariants match the stated contracts, and the tests pin the real behaviors (including the previously wrong one). P2 findings: the itoa helper fragility in test code is the most likely to bite (any PR adding a single-digit park number in signal_test.go would silently generate invalid JSON); the Replay.card value-receiver mutation is a future-maintainer trap. Finding 3 is a documented asymmetry to track. P3 items are minor.

[github-actions]

Reviewer: claude
Reviewed commit: 72ef4230d4c041c39157afd469b4fb6008a32c65

[itsHabib]

@claude review

[claude]

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

---

Code Review

• Load tools and post initial comment
• Read changed files and diff
• Review: signal_test.go + lifecycle_test.go (new test coverage)
• Review: source/gatelog.go + source/ledger.go (grant_needed + digest)
• Review: notify/notify.go (collapse logic)
• Review: journal/journal.go + journal/cursors.go (stall tracking, replay)
• Review: main.go (dispatch, cycle state, new commands)
• Review: digest.go + preflight/preflight.go
• Post final review

---

Reviewed commit: 5cbe316 (top of claude/flare-phone-signal).

Resolutions from the prior round

All four named findings from the round-1 review are addressed:

Finding	Status
F1 — itoa in signal_test.go silently corrupts JSON for single-digit numbers	Fixed — signal_test.go:96 now delegates to strconv.Itoa(n)
F2 — Replay.card value receiver misleads maintainers	Fixed — journal.go:165 uses (r *Replay), comment added
F3 — digestLine proposes T2 for repos with no prior grant while grantNeededMint uses lastCeilingsFor	Fixed — Authority now carries ProposedTier/ProposedCycles populated by lastCeilingsFor in authority(), both surfaces now give the same advice
F5 — digestSource non-deterministic with multiple gate log sources	Fixed — digest.go:41 is a constant "flare"
F4 — markRepeat mutation comment at call site	Addressed at the docstring level, which is better placement

New findings — this round

P3: tsFor in lifecycle_test.go:61 carries the same fragility as the old itoa

func tsFor(n int) string { return "170000000." + string(rune('0'+n)) }

rune('0'+n) wraps to ':' at n=10, ';' at n=11, etc. Current tests stay safely below 10 per courier (each &fakeCourier{} resets ts to zero, and no single test posts that many events), but a future test with many parks would silently produce non-digit characters in the TS. The practical risk is lower than the original itoa — a weird-character TS still compares equal to itself in assertions, so it won't cause a wrong-ref failure — but the fragility is the same pattern. strconv.Itoa(n) fixes it the same way.

Fix this →

P3: digest() calls time.Now() three times for one logical "now"

digest.go:54,59,64 — authorityRows(cfg, time.Now()), then digestEvent(rows, time.Now(), within), then j.Load(time.Now()). None of the three are the same instant. For the liveness window and dedup check the skew is negligible, but a single now := time.Now() at the top of digest() and threading it through would make the intent explicit and remove the question of which "now" each call sees.

Feature correctness

grant_needed paging (source/gatelog.go:504–573, signal_test.go:40–86): tolerant decode is the right call — an unparseable body still pages with a generic message. The lastCeilings logic (ledger.go:553–559) correctly prefers the refused grant's own ceilings when the record names one, else the widest the repo held. TestGrantNeededMintProposesWhatWorkedBefore pins both paths cleanly. KindGrantNeeded as a local string constant (not an import) is the correct boundary-law choice.

Reason collapse (main.go:442–452, notify/notify.go:616–651, signal_test.go:103–147): fingerprinting the leading clause of the "; "-joined reason (via FNV hash) rather than the whole line is exactly the right cut — the live-ledger data in the PR description makes the threshold choice legible. repeatBlock inverts the card order (repetition named once, then what's new) without suppressing anything. TestDifferentReasonsDoNotCollapse keeps the collapse bounded to the right scope.

Stall detection (main.go:357–365, cursors.go:46–59, signal_test.go:153–185, lifecycle_test.go:308–342): pollSource returning a typed stalledError instead of nil on delivery failure, cycle recording the stall via cur.Stall() and clearing it via cur.Clear(), status adding len(cur.Stalled) == 0 to the health gate — each is minimal and correct. TestFailedCorrectionRetries now asserts the honest contract (err != nil on a stalled cycle, stall recorded in cursors, cleared after recovery).

Journal single-pass replay (journal.go:142–173): replacing three separate journal walks with one Load(since) that populates Seen, Cards, and Reasons simultaneously is a clean improvement. The pointer receiver on card makes the map mutation contract explicit. The since parameter scoping only Reasons (not Seen or Cards) is clearly documented and correct.

flare digest (digest.go, ledger.go:347–493): the silence heuristic (only hard-stop and expiring-under-parked-work rows; running-fine repos excluded) is the right cut. Dedup by content hash means an unchanged picture never re-pages. digestSource = "flare" as a constant removes the config-order dependency that was F5.

Summary

All P2 findings from round 1 are resolved. Logic across all three features is sound and the tests pin the real contracts, including the previously incorrect one (TestFailedCorrectionRetries). Two P3 items noted: tsFor in lifecycle_test.go is the same pattern as the old itoa (lower practical risk since it only affects string equality in assertions, not JSON validity); the three time.Now() calls in digest() are a cleanliness nit. No blocking issues.

[github-actions]

Reviewer: claude
Reviewed commit: 5cbe3161d25fc056c4d00febf0c2fd195ce19b25

[itsHabib]

@claude review

[claude]

Claude finished @itsHabib's task in 2m 31s —— View job

---

Code Review

• Load tools and post initial comment
• Check resolution of round-2 P3 findings
• Review top commit diff for new issues
• Review signal_test.go, lifecycle_test.go
• Review digest.go, main.go
• Post final review

---

Reviewed commit: 16b1120 (top of claude/flare-phone-signal).

Resolution of round-2 P3 findings

Both items are addressed in the top commit:

Finding	Status
P3 — tsFor in lifecycle_test.go carries the same fragility as old itoa	Fixed — lifecycle_test.go:61 now delegates to strconv.Itoa(n)
P3 — digest() calls time.Now() three times for one logical "now"	Fixed — digest.go:57 captures now := time.Now() once; all three calls (authorityRows, digestEvent, j.Load) receive it; the comment explains exactly why the same instant matters

The comment added with the fix makes the invariant explicit: liveness, the expiry window, and the dedupe read must answer the same moment or a row can be live for one question and lapsed for the next. That is the right comment to pin here — it captures the non-obvious constraint that would otherwise invite the skew back in.

Completeness check — all prior findings

Round	Finding	Status in HEAD
R1-F1	itoa in signal_test.go silently corrupts JSON for single-digit numbers	Fixed (now strconv.Itoa)
R1-F2	Replay.card value receiver misleads maintainers	Fixed (pointer receiver confirmed at journal.go)
R1-F3	digestLine proposes T2 for repos with no prior grant while grantNeededMint uses lastCeilingsFor	Fixed (Authority carries ProposedTier/ProposedCycles, both surfaces consistent)
R1-F4	markRepeat mutation comment at call site	Addressed
R1-F5	digestSource non-deterministic with multiple gate log sources	Fixed (constant "flare")
R2-P3a	tsFor fragility	Fixed (this commit)
R2-P3b	Three time.Now() in digest()	Fixed (this commit)

No new findings

The top commit is exactly its stated scope — two surgical fixes, no surrounding churn. The itoa wrapper in signal_test.go (now a one-liner delegating to strconv.Itoa) is a trace of the fix rather than an abstraction worth keeping, but inlining it is a cosmetic concern below P3 and not worth another round.

The time.Now() calls remaining in cycle() (cur.Stall, cur.LastPoll, loadState's repeatWindow anchor) are correctly independent — each is a distinct timestamp for a distinct purpose, not the same logical moment spread across three calls the way the digest was. No concern there.

Summary

All P2 and P3 findings from both prior rounds are resolved. The top commit is minimal, correct, and its intent is clearly documented. No new findings. The branch is clean.
· Branch

[github-actions]

Reviewer: claude
Reviewed commit: 16b1120a03fd00d61ce37373aeb621c853ec74cb

[itsHabib]

@codex Review this PR, pinned to 16b1120a.

Sweep of stale open PRs: this one has been open without a complete review panel. Triggering the repository roster on the exact head above so it can be gated.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-02T12:36:06.812430Z	16b1120	Manual request

ℹ️ 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" or "@codex security review".

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]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16b1120a03

ℹ️ 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".