GitHub API fallback eligibility may not be derived from the issue-comment watch store

[khaliqgant]

Follow-up from #225. Filed so this does not live only in chat. Not dispatched deliberately — no readiness label, no title prefix.

Summary

#deriveGithubApiFallbackEligibility resolves whether an issue may be re-read through the direct GitHub API fallback by scanning durable decision sources. As of 91add5f it covers three:

• the in-flight batch (batch.inFlight)
• waiting clarifications (listWaitingClarifications)
• non-terminal dispatch lifecycles (listDispatchLifecycles)

There appears to be a fourth durable store holding a TriageDecision that it does not consult.

Evidence

• escalationWatchRecord (src/orchestrator/factory.ts:15796) constructs an InFlightIssue locally from a bare TriageDecision, rather than one restored into the batch — the same shape as the clarification path that caused the round-6 defect in fix(factory): fall back to direct GitHub API when the Relayfile issue projection is stale #225.
• It is called at src/orchestrator/factory.ts:8709 with pending.decision, feeding #handleTriageEscalationGithubAnswer, which reads the issue via #readIssue with no decisionHint.
• That decision originates from GithubIssueCommentWatchPending.decision, durable via #state.setGithubIssueCommentWatch / getGithubIssueCommentWatch.

This is a distinct lifecycle stage from the three covered sources: a triage-time escalation, resolved by a GitHub comment reply before the issue is ever dispatched.

Status — structurally confirmed, not demonstrated

The code path is confirmed by inspection. What is not established is whether a pre-dispatch escalation decision can actually carry issueResolution.source === 'github-api-fallback'. It is plausible — a CLI-targeted triage of an issue whose projection is stale — but no live failure has been reproduced, and this should not be repeated as a confirmed defect until someone does.

If it can occur, the failure shape matches the one fixed in #225 round 6: after a restart the eligibility cache is empty, derivation misses the decision, and the read fails on an issue that is actually fine.

Why this is filed rather than patched

Adding a fourth source at this call site would repeat the method that produced the problem. Rounds 4 and 6 of #225 were the same root failure — the enumeration of "where can a decision live" was built reactively around whichever case a reviewer named, instead of surveyed up front.

The right fix is a complete audit of every local InFlightIssue / decision: construction site in src/orchestrator/factory.ts, producing the enumeration mechanically before any code changes, so the set is proven rather than asserted. This one was found by grepping for that shape, not by a review round.

Suggested acceptance criteria

• A mechanically-produced list of every construction site that wraps a TriageDecision outside the in-flight batch, in the issue or the PR description.
• Eligibility derivation covers every durable source on that list.
• A guard test per source: a decision reachable from only that source resolves eligible.
• A determination, either way, on whether a pre-dispatch escalation decision can carry github-api-fallback — with the reasoning recorded even if the answer is "it cannot".

[kjgbot]

Mechanical survey: every site holding or constructing a TriageDecision outside batch.inFlight

Method: enumerated every method on StateStore (src/ports/state.ts) and checked each returned/stored type for a .decision/TriageDecision field, then checked BatchSnapshot's full shape (not just .inFlight), then traced every local InFlightIssue-shaped or escalationWatchRecord-style construction found by grep for the decision: field shape. Grepping for the shape, not for remembered call sites, is what surfaced everything below the first row.

#	Holder	Durable or ephemeral	Feeds a read at	decisionHint passed?	Covered by 91add5f derivation?
1	batch.inFlight	Restored from durable dispatch lifecycles at startup (durable fleets); live for the process otherwise	~20 call sites	2 (#dispatchUnlocked, #resumeDurableDispatch)	Yes
2	#state.listWaitingClarifications	Durable	#clarificationIssueStillActive (via #resumeWaitingClarification)	Yes	Yes
3	#state.listDispatchLifecycles, non-terminal	Durable	every #driveDispatchLifecycle phase handler	No (relies on the scan)	Yes
4	#state.listGithubIssueCommentWatches → pending[].decision (kind:'triage')	Durable	#handleTriageEscalationGithubAnswer (factory.ts:8737, via #routeGithubAnswerToImplementers)	No	No
5	batch.queued (BatchTracker in-memory map, distinct from .inFlight)	Ephemeral. For durable fleets, restart-restored via row 3's 'queued' lifecycle phase, which replays into batch.queue() at startup. For non-durable fleets, no restart-recovery concept applies to it at all.	issues move out of this map into .inFlight before any read happens on them	n/a	Not scanned directly, but see caveat below
6	batch.parked (BatchTracker in-memory map, dependency-park — distinct from clarification-park)	Same characteristics as row 5	#reportDependencyPark	not fully traced	Same caveat as row 5
7	#postAndWatchSlackEscalationThread (factory.ts:12033)	n/a — reads via #readIssue(decision.issue.path) with decision already in local scope	—	No, but see note below	Not currently exploitable (see note)

Row 5/6 caveat

batch.queued/batch.parked are not durable on their own. The only way they matter to eligibility is a same-process scenario: an issue is dequeued and re-read later in the same run. The very first read of that issue's decision (in #dispatchUnlocked, which always runs before an issue can be queued or parked) already goes through the decisionHint-covered path and warms the bounded cache (#githubApiFallbackIssues, capacity 2,000). A later same-process read of the same identity hits that warm cache — unless the entry was evicted first. That is a narrow, low-probability residual risk tied to cache churn under sustained high fallback volume, not a guaranteed-reproducible defect like rows 4 and (previously) 2/3. I have not attempted to reproduce it.

Row 7 — traced, not exploitable today

#postAndWatchSlackEscalationThread is only reached through #escalateTriage, which is only reached from:

• #dispatchUnlocked (factory.ts:~2527), after its own decisionHint-covered scope-check read has already succeeded and warmed the cache for the same identity in the same call, or
• the discovery-scan path (factory.ts:~4373), whose decision comes from the orchestrator's own internal triageIssue() and therefore never carries issueResolution at all (only cli/fleet.ts's CLI-targeted resolution sets that field).

So this specific missing hint is real but currently dead code from the fallback-eligibility angle — noting it because the next person tracing this class of bug should not have to re-derive that.

Answer 1: is the set closed?

No — not proven closed, and I found a concrete gap while trying. Confirmed 4 durable/semi-durable sources; only 3 are covered by 91add5f. I'm confident the pass over StateStore's methods is complete (it's a fixed, enumerable interface — every method and every type it touches was checked for a .decision field). I'm not confident I've individually verified hint-status for all ~24 #readIssue/#readGithubIssue call sites — I found row 4 and row 7 by depth-first tracing outward from the known gap, not by a systematic per-call-site pass. That systematic pass is the piece still missing before "closed" can be said with full confidence.

Answer 2: can a pre-dispatch escalation decision carry github-api-fallback?

Yes — traced as reachable, not just plausible.

1. A CLI-invoked factory dispatch <owner>/<repo>#<number> targeting an issue the Relayfile projection can't answer resolves via the API fallback in cli/fleet.ts, setting issueResolution.source === 'github-api-fallback' on the decision.
2. If that same decision's repo-label routing is also ambiguous (labelDerivedDispatchDecision disagreement, tripping triageEscalationReason), #dispatchUnlocked routes it to #escalateTriage → #escalateTriageToGithub (factory.ts:~11798).
3. #escalateTriageToGithub persists that fallback-sourced decision into #state.setGithubIssueCommentWatch (kind: 'triage') — durable, and it survives a restart.
4. A human's later reply on that GitHub thread is handled by #routeGithubAnswerToImplementers → #handleTriageEscalationGithubAnswer, which reads with no hint and is not covered by the current derivation.

This is the same failure shape fixed for clarifications in round 6 of #225: after a restart, the eligibility cache is empty, derivation misses the decision, the read fails on an issue that's actually fine, and the human's escalation reply is silently discarded instead of resuming triage. I have not reproduced this live (no running instance to trigger it against) — this is a traced code path, not an observed failure — but every step is a real call chain in the committed code, not a hypothetical.

What I'd do next, if asked to fix rather than survey

Route 4 through the same widening pattern as rows 2/3: add githubApiFallbackCandidatesFromGithubIssueCommentWatches (filtering to kind === 'triage' && decision present), scanned the same way. Before writing that, do the missing systematic per-call-site pass (all ~24 #readIssue/#readGithubIssue sites) so "closed" is a checked fact rather than an inference from the sources found so far.

[kjgbot]

Systematic pass, all #readIssue/#readGithubIssue call sites

Every call site of this.#readIssue( and this.#readGithubIssue( in src/orchestrator/factory.ts was located by grep (the entry-point pattern is fixed and exhaustively matchable), mapped to its enclosing method, then traced to where the record/decision/path argument in scope originates. 33 call sites, not ~24. Traced each to one of five outcomes.

A — hint-covered (3 sites)

#dispatchUnlocked, #resumeDurableDispatch, #clarificationIssueStillActive. Already covered before this pass.

B — sourced from an already-covered holder (19 sites)

14 sites read record.issue.path where record: InFlightIssue traces back to batch.inFlight / batch.getIssue / batch.getIssueByAgent (agent-exit handling, publish, completion, babysitter, agent-question delivery — all downstream of an in-flight-tracked agent). 4 sites (#driveDispatchLifecycle, #adoptInFlightAgents) read directly against a DispatchLifecycle or a batch.restore()-inserted record. 1 site (#performClarificationQuestionDelivery) reads against a WaitingClarification returned from #state.claimClarificationQuestionDelivery. None pass a hint; none need to — the scan already covers all three sources.

C — general discovery/webhook scan, no decision in local scope (9 sites)

Ready-issue-list reads, GitHub/Linear change-event handlers, dependency-blocker-issue loading, PR-merge-advance candidate scanning, Linear mirror-candidate loading. These read an arbitrary path with nothing decision-specific to hint. By design they rely entirely on the scan — not a new gap category, but worth naming precisely: if the only durable source covering a given identity is one of the two gaps below (D or the confirmed gap), a discovery-scan read of that same identity fails too. Fixing the confirmed gap fixes every read of that identity, not just the one call site that surfaced it.

D — batch.parked / batch.queued (1 concrete site found + the two collections generally)

#markDependencyTerminalAndReconcile (factory.ts:5150) reads parked.issue.path where parked = batch.getParked(...) — a direct read against the batch.parked map reported previously as a structural, untraced caveat. This is the concrete site. Confirmed the mitigation holds here too: #dispatchUnlocked's hint-covered read always runs before an issue can reach batch.queue()/#park(), so the cache is warm by the time this executes — unless the entry was evicted first (the LRU-cache caveat already reported, still not reproduced).

E — the confirmed gap (1 site)

#handleTriageEscalationGithubAnswer (factory.ts:8737) — already reported. No new sites feed the same durable holder (listGithubIssueCommentWatches) that aren't already accounted for below.

F — traced, currently unreachable independently, same root as E (4 sites)

• #escalateTriage (11751) and #escalateTriageToGithub (11799) — both always run downstream of the same hint-covered read that already warmed the cache for that identity (#dispatchUnlocked), or from discovery-scan (never carries issueResolution at all).
• #postAndWatchSlackEscalationThread (12033) — reported previously, same reasoning.
• New in this pass: #handleTriageEscalationSlackAnswer (12965), reached via #routeSlackAnswerToImplementers. Its record comes from a Slack-watcher closure built by escalationWatchRecord(decision) — same construction as the confirmed gap — but tracing the watcher-rearm path (#rearmSlackReplyWatchers) shows it only re-arms watchers sourced from batch.inFlight and listWaitingClarifications. It does not iterate listGithubIssueCommentWatches. So a triage-escalation Slack watcher is never rearmed after a restart at all — this code path can't currently run post-restart, for a reason unrelated to eligibility derivation. This looks like a separate, adjacent defect (an escalation reply arriving on Slack after a restart is presumably dropped for want of a listener, independent of whether the issue is fallback-backed) — flagging it because it surfaced during this pass, not fixing it, and not folding it into this issue's scope.

Answer 1 (revised): is the set closed?

Every durable/semi-durable holder is now individually traced to its call sites — I can defend a closed claim modulo two named, bounded items, not an open-ended "might be more":

1. The confirmed gap (E) — needs a fix.
2. The batch.parked/batch.queued LRU-eviction edge case (D) — theoretical, not reproduced, bounded to sustained cache churn under high fallback volume between an issue being blocked and later dequeued.

No sixth durable holder turned up across all 33 call sites. I'm treating "closed modulo those two" as the honest claim, not "closed" outright, because item 2 is a real (if narrow) code path I have not exercised, and because a systematic pass over one file's call sites can't rule out something entirely outside factory.ts (e.g., if another orchestrator file someday grew its own local TriageDecision holder) — that's a narrower disclaimer than last time's "I might have missed a whole source," which is the actual improvement this pass bought.

Answer 2: unchanged

Still traced-not-reproduced, as reported previously — the chain through the confirmed gap stands.

[kjgbot]

Determination on the batch.parked/batch.queued LRU-eviction edge — by reasoning, not reproduction

Correction to my own prior comment first: I previously wrote that batch.parked has "the same characteristics" as batch.queued — restart-restored via a durable 'queued'-phase dispatch lifecycle. That's wrong for batch.parked, and tracing why answers the reachability question, so folding the correction in here rather than as a separate note.

batch.queued (capacity-blocked) — the durable backing is real

Tracing #dispatchUnlocked: when #claimDispatchLifecycle is reached and the durable claim itself comes back at phase 'queued' (capacity, not dependencies), that phase is written to #state before the issue is ever added to the in-memory batch.queued map. #driveDispatchLifecycle's 'queued'-phase handling reads that durable record back and re-populates the in-memory queue from it. So for durable fleets, cache eviction doesn't matter here — the decision is independently recoverable from listDispatchLifecycles, which my derivation already scans. This one was correctly characterized as low-risk.

batch.parked (dependency-blocked) — no durable backing exists, for any fleet type

Re-traced #dispatchUnlocked end to end. The dependency-admission check (#dependencyAdmission) and its blocked branch — batch.queue(...) routing internally to #park(...), then an early return — happen before #claimDispatchLifecycle is ever called. A dependency-blocked issue never reaches the code that would write a durable lifecycle record. DispatchLifecyclePhase itself has no dependency-park-specific value — 'parking'/'waiting-for-human' are the clarification-park phases, a different mechanism entirely. So batch.parked is in-memory-only, full stop, regardless of fleet type. The cache (#githubApiFallbackIssues) is not a second line of defense here — it is the only defense.

Is eviction reachable before the read at factory.ts:5150?

#markDependencyTerminalAndReconcile — the function containing that read — is not periodic. It runs only when the specific blocking issue's own state changes to terminal (via the change-event/webhook handlers). Grepped every use of batch.parked in the file: there is no sweep that re-examines parked issues on a timer. So between the moment an issue is parked (its decision's cache entry warmed by #dispatchUnlocked's hint-covered read) and the moment its blocker resolves, nothing else reads or refreshes that specific identity.

Eviction requires 2,000 other, distinct fallback-backed identities to be newly registered in that window, since re-registration (including a cache-hit re-confirmation) refreshes recency and would keep this one alive. Fallback registration is deliberately narrow by design — confirmed earlier in this audit — it only happens for a CLI-targeted resolution of a specifically stale-projection issue; ordinary discovery-driven triage/dispatch never sets issueResolution and never touches this cache at all. Reaching 2,000 distinct such CLI-targeted dispatches inside one daemon process's lifetime, while one particular dependency chain stays unresolved, is not something ordinary operation produces — but it is not excluded either. A scripted bulk-remediation tool invoking the CLI in a loop across a large stale-projection backlog would do it.

Determination: reachable, with a named precondition — not struck

Not "unreachable." The honest claim is: reachable only under an operationally unusual precondition (≥2,000 distinct CLI-targeted fallback dispatches within one process's lifetime, none of them re-touching the parked identity, while its specific blocker stays unresolved for that whole window) — not something ordinary discovery-driven operation produces, but achievable by a scripted bulk CLI loop.

The consequence is also worse than previously stated

If it does happen, reconciliation does not retry — the same event (this specific blocker resolving) is the only trigger, and it has already fired. The park is stuck until either (a) the blocker sends another terminal-state event (not expected — it is already terminal), or (b) the process restarts, at which point ordinary discovery re-triage would re-evaluate dependency admission fresh and dispatch it normally (dependency-blocking, unlike the cache, is re-derived from live state on every triage pass, not read from a decision that needs eligibility at all — the read at 5150 that needs eligibility only exists on the reconciliation path, not on fresh triage). So a restart recovers it; nothing short of a restart does.

Revised statement for the artefact

#226's "closed modulo two" should not become "closed modulo one." It should be corrected to:

1. Confirmed gap (#handleTriageEscalationGithubAnswer) — needs the fix Chief is deciding on.
2. batch.parked cache-eviction gap — reachable under a named, unusual precondition (not ordinary operation); when it triggers, the affected dependency-park is stuck until the next daemon restart, not permanently. batch.queued (capacity-only) is separately and correctly covered via durable 'queued'-phase lifecycles and is not part of this risk.

This is a correction to, not merely a completion of, the prior "closed modulo two" statement — item 2 was previously mischaracterized as symmetric with batch.queued's durable backing, and it is not.

[kjgbot]

Correction to the previous determination: batch.parked has two populations, not one

The previous comment claimed batch.parked has no durable backing "for any fleet type, full stop." That's an overgeneralization from tracing a single path. There are two distinct ways an issue ends up in batch.parked, and only one of them is exposed.

Population A — blocked on the first dispatch pass. In #dispatchUnlocked, #dependencyAdmission (~2531) and the resulting batch.queue(...) → #park(...) (~2533) run before #claimDispatchLifecycle (~2569) is ever reached. An issue blocked here never writes a durable lifecycle record at all. This population has no durable backing, for any fleet type — the eviction gap and its precondition, as previously described, stand for this population specifically.

Population B — re-parked from an already-durable 'queued'-phase lifecycle. #driveDispatchLifecycle (3851-3860), when re-driving a lifecycle that is already durably at phase 'queued', re-runs #dependencyAdmission and, if still blocked, calls batch.queue(lifecycle.decision, ...) again — landing the issue in the in-memory batch.parked map exactly as population A does. But this issue's durable lifecycle record already exists at phase 'queued' (non-terminal), untouched by this in-memory move. listDispatchLifecycles — which the current derivation already scans — covers it regardless of cache state.

So the read at factory.ts:5150 is only at risk for population A. An issue in batch.parked because of a capacity-then-dependency sequence (population B) is safe even if its cache entry was evicted.

Revised statement for the artefact

Two items, unchanged in count, with item 2 narrowed to its accurate scope:

1. Confirmed gap (#handleTriageEscalationGithubAnswer) — needs the fix Chief is deciding on.
2. batch.parked cache-eviction gap — population A only. Issues parked on the first dispatch pass, before any durable lifecycle claim, have no durable backing; the cache is their only protection. Reachable under the previously-described precondition (≥2,000 distinct fallback-backed identities registered elsewhere while this one's blocker stays unresolved, with no intervening read of the same identity), and stuck-until-restart if it fires. Issues parked via a later redrive of an already-durable 'queued'-phase lifecycle (population B) are not exposed — listDispatchLifecycles already covers them.

batch.queued (capacity-only) remains fully covered as previously stated and is not part of this item at all.