Bugfix spec: a branch name invented a work item, and the daemon obeyed it
Phase 1 of 3 for a bug (bugfix → design → tasks). This phase MUST be reviewed and approved before the design is derived from it.
Summary
issue-285 in a branch name became org/lib#285 — a work item that does not exist — and nothing between the regex and the tmux session asked whether it did. The ghost then took the two decisions that matter most: it became work_items[0], so it absorbed the operator's the-loop start, and it was the ref a full session spawned against — clone, registry entry, tmux session — while the real work item, with the whole spec chain, sat in another repository being ignored.
The daemon was told. session.announce_failed carried gh: Not Found (HTTP 404) on the work item's own ref, seconds after spawning for it, and the spawn carried on. Ticket: #269.
Steps to reproduce
Two repositories under one org: org/planning holds tickets and specs, org/lib delivers code. One poller watches both, spawnOnUnmatched: labeled, requireStartCommand: true.
- The work item is
org/planning#285, with its spec chain.org/libhas no issue #285. - A pull request
org/lib#48is opened from branchissue-285-consolidation— the-loop's own branch convention, pointing at the planning repository's issue. Its body links the ticket as a plain URL (no closing keyword), soclosingIssuesReferencesis empty. org/lib#48carriesthe-loop: auto-execute.- An authorized user comments
the-loop starton the pull request.
Expected vs actual
- Expected: the start binds to something that exists — the pull request itself, or a linked work item that was verified — and the session that spawns has a real ticket underneath it.
- Actual: the router emits
[org/lib#285, org/lib#48]; the start is recorded againstorg/lib#285; a session spawns fororg/lib#285(workspace.prepared,session.registered,session.spawned), thensession.announce_failedreportsgh: Not Found (HTTP 404)and nothing acts on it; the session runs without a spec chain (graph.skipped/no-spec-dir) because the real one is inorg/planning.
Root cause (confirmed)
Three independent layers each fail open, and the ghost walks through all three.
flowchart TB
B["branch: issue-285-consolidation"] -->|"_issue_from_branch → 285"| R
R["linked_work_items():<br/>resolved in the PR's OWN repo"] --> W["work_items[0] = org/lib#285"]
W --> C["_apply_control: start recorded<br/>against work_items[0]"]
W --> S["_on_unmatched: spawn work_items[0]"]
S --> A["announce → HTTP 404<br/><i>logged, ignored</i>"]- The weakest source is treated as the strongest.
linked_work_items(cli/the_loop/webhook/router.py) reads three linkage sources. Two of them state a repository —closingIssuesReferencescarries one, a qualified closing keyword names one — and the third, the branch convention, does not. issue-183 settled that correctly ("the branch convention stays local:issue-12on a branch says nothing about a repository"), but local is a guess, and a guess with no existence check is a fabrication. Ordering then promotes it: the loop emits linked items before the entity's own number, so the fabricated ref iswork_items[0]. work_items[0]is the target for decisions that had a better answer available._spawn_refusal,_apply_control,_on_unmatchedand_record_graph_command(cli/the_loop/webhook/dispatcher.py) all bind torouted.work_items[0]. For an event carrying a pull request, two better answers already exist in the process: the durable PR → work-item binding the registry records (issue-172), andpr_work_item(), which names the pull request itself. Neither is consulted before the list's head.- The 404 is evidence, and it was filed as decoration.
SessionAnnounceris best-effort by design (an announcement must never fail a dispatch), so it degrades every failure identically — a missinggh, a rate limit, and this work item does not exist all become one warning-levelsession.announce_failed.
Requirements
Requirement 1 — a work item invented from a branch name is verified before it is used
The branch convention is the only linkage source that supplies a repository the pull request never stated. It is therefore the only one that can name a work item nobody created, and it must earn its place in work_items before anything acts on it.
Acceptance criteria (EARS)
- WHEN a routed event yields a work-item ref derived only from the
issue-<n>branch convention AND that ref has no session record on this machine THEN the system SHALL ask the ticketing provider whether the ref exists before that ref is used as a spawn target or a control-command target. - WHEN that check answers definitively that the ref does not exist (HTTP 404) THEN the system SHALL remove the ref from the event's work items, SHALL record the removal (
routing.linkage_dropped, with the ref, the source and the reason), and SHALL continue routing the event on the refs that remain. - WHEN the check cannot be made — no
ghon PATH, a timeout, a transport error, any non-404 failure — THEN the system SHALL keep the ref and SHALL NOT drop the event: an unavailable check is not evidence of absence, and a daemon that stops routing when GitHub is unreachable is a worse failure than the one this fixes. - WHEN a ref is corroborated by
closingIssuesReferencesor by a closing keyword in the pull request body THEN the system SHALL NOT subject it to the check: those sources state their repository, and issue-183's cross-repository routing must not acquire a network dependency. - WHEN any of the event's refs is owned by a live session record on this machine THEN the system SHALL NOT consult the check at all: internal tracking has already answered which work item this event belongs to, no external answer may drop a live session's ref, and a ghost sitting beside a matched record is inert anyway (nothing spawns while an event matches, and R2.1 binds the command to the record).
- WHEN the same ref is checked again THEN the system SHALL answer from a bounded in-process cache rather than asking again, so a repeatedly-commented pull request costs one call.
- WHEN every one of an event's work items is dropped by this rule THEN the system SHALL drop the event with reason
work-item-not-foundand SHALL NOT release its delivery id: a nonexistent work item is a permanent condition, and releasing the id would have GitHub redeliver — and the poller re-forward — the same event every cycle.
Requirement 2 — what a pull request delivers is read from the-loop's own record first
(the owner's direction on the ticket: "whenever user responds to a PR, the-loop should check what work item that PR is linked to — not through GitHub, but through internal tracking mechanisms")
Acceptance criteria (EARS)
- WHEN a control command arrives on an event whose refs include one this machine holds a live session record for THEN the command SHALL act on that record's work item, whatever order the router emitted the refs in.
- WHEN no live record owns any of the event's refs THEN the command SHALL act on the first ref that survived Requirement 1 — which, for a pull request whose only other linkage was a branch-derived ghost, is the pull request itself.
- WHEN an unmatched event spawns a session THEN it SHALL spawn against the same ref the control path would have acted on, so "what was started" and "what is running" cannot name different work items.
- The
start-was-requested test (requireStartCommand) SHALL be asked about that same ref, so a start recorded on one ref is never read back from another. - Which tmux session an event is then delivered into SHALL remain the existing decision — the operator's
routing.tmux.sessionPerPras overridden by the work item's frozenphase-selectionanswer (issue-260) — unchanged by this work item. - WHEN a comment on a pull request arrives through the poll ingress THEN the system SHALL resolve that pull request the way a webhook comment does, so the binding is recorded and the R2.5 decision is actually taken. The poller reuses the pull request's own payload (key
pull_request) and renames the event toissue_comment;pr_work_itemreads onlypayload["issue"]for that event name, so on the ingress the ticket was reported from, every pull-request comment answered "this event carries no pull request" — no binding written, no endpoint chosen.
Requirement 3 — a 404 on the work item is reported as what it is
Acceptance criteria (EARS)
- WHEN the session announcement fails because the work item itself is not found THEN the system SHALL record
session.work_item_missingat error level, naming the ref and the remedy, rather than only the generic best-effortsession.announce_failed. - WHEN that happens THEN the ref SHALL be recorded as missing in the same cache Requirement 1 consults, so the next event carrying it as a branch-derived ref is dropped without a second call.
- The announcement SHALL remain best-effort: a 404 SHALL NOT fail the dispatch, and SHALL NOT kill the session that was just spawned (see
design.md§Alternatives — a private or permission-scoped repository answers 404 for items that do exist, and killing a live agent on that evidence destroys work).
Requirement 4 — a regression test per layer
- The fix SHALL include tests that fail before it and pass after it, covering: the ghost ref being dropped, the control command binding to the surviving ref, the spawn target, the unknown-answer fail-open path, the corroborated-ref exemption, the live-record exemption, and the announce-404 record.
- The reproduction in this document SHALL be covered end-to-end by an integration test carrying a Gherkin docstring (
testing.gherkinDocstrings: required).
Security considerations
The bug itself is not exploitable, and the fix opens one new surface: payload-derived coordinates reaching a gh argv.
| Boundary | Where | How it fails closed |
|---|---|---|
| Payload → command line | the new existence check builds gh api repos/<owner>/<repo>/issues/<n> from a WorkItemRef whose owner/repo/number came from a webhook payload | the same validation the_loop.comments already applies at this seam: owner and repo matched against ^[A-Za-z0-9._-]+$, the number is an int by construction, the process is spawned from an argv list with no shell, and a ref failing validation is answered "unknown" (kept, not dropped) |
| Non-GitHub / GitHub Enterprise refs | a ref on a non-default host | the check passes --hostname for a non-default host and answers "unknown" for a non-GitHub provider — never a 404 from the wrong GitHub, which would drop a real work item |
| Availability | one network call on the ingress thread | bounded timeout, bounded LRU cache, asked only for a branch-only ref with no local record; every failure mode answers "unknown" in bounded time |
| Authorization | unchanged | the check runs after the router's self-marker and authorizedUsers guards and touches neither; it can only ever remove a ref from an event, never add one, never widen which events arrive, and never arm a work item |
An attacker who can open a pull request in a watched repository could already name any branch they liked; today that fabricates a work item and spawns a session for it. After this change the fabrication is dropped, so the change narrows the reachable surface. The reverse abuse — forcing the check to fail so a ghost survives — buys exactly today's behaviour and nothing more.
Out of scope
- Pre-start comments are never replayed (the ticket's "related casualty"). A comment dropped with
dispatch.dropped reason: awaiting-startkeeps its delivery id in the dedup cache forever, so it is not delivered even after the work item is started. That is a deliberate, documented refusal in the current code with its own trade-off (releasing the id would have every unstarted labelled item re-forwarded every cycle), and changing it is a product decision about replay semantics, not part of restoring linkage correctness. The reporter flagged it as "possibly its own issue"; it is filed as #270. - Verifying the other two linkage sources. A closing keyword naming a nonexistent issue (
Closes org/typo#9) is a human typo in a source that states its repository; it is caught by Requirement 3's report rather than by a pre-emptive call on every event. - A configuration key for the check. It is a correctness fix, not a preference: it costs one cached call for the one ref shape that can be fabricated, and it degrades to a no-op where
ghis unavailable.
Open questions
None. The ticket states the expected behaviour as three alternatives ("some combination of"); this spec adopts the first two as the fix and the third as a report, with the reason in design.md §Alternatives considered.