orchestrate.py status: build_status mis-attributes markers (ignores marker.number), so quoted grammar examples poison the board

[TBarregren]

Summary

The orchestrate.py status board (added in #50) attributes every orchestrate: marker it finds in an issue's comments to that issue, without checking marker.number. Because the milestone grammar deliberately carries the issue number inside each marker (orchestrate: started #47, …) precisely so attribution is verifiable, any comment that merely quotes a marker — most notably #48's own Agent Brief, which quotes the grammar in code fences — poisons the board. Demonstrated live: feeding the documented status one-liner's real payload renders a bogus #48 working started scoped to the placeholder run <runId>. A related lower-severity gap lets an unsubstituted run <runId> template parse as a real run id and pool with those quoted examples.

Severity: MEDIUM (+ a folded LOW). The feature is an auxiliary read-only board, but it silently shows the wrong state, which is worse than showing nothing for a tool whose whole job is trustworthy at-a-glance status.

Evidence (verified)

• scripts/orchestrate.py:1708 load_status_universe reads each issue's comments through parse_landed_marker and appends every parsed marker to that issue's StatusIssue, with no marker.number == entry["number"] comparison:

  marker = parse_landed_marker(str(body or ""))
  if marker is None:
      continue
  ...
  issue_markers.append(StatusMarker(marker, created_at, sequence))

• parse_landed_marker is deliberately prose-tolerant (a orchestrate: interrupt-safe restart — detect a dead cross-session resume, never re-implement landed work #49 property that was safe when only preflight consumed it — preflight also demands a hex SHA that is an ancestor of the default tip, a second factor status has no equivalent of).
• Live demonstration (integration review): issue orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48's Agent Brief quotes orchestrate: started #47, run <runId> inside a code fence. Running the documented one-liner over real GitHub data renders #48 working started under bogus run <runId>.
• Amplifier (terminal fallback): the read path's any-run landed/opened-pr → done fallback deliberately ignores run scoping, and opened-pr needs only digits (unlike landed, whose [0-9a-fA-F]+ SHA rejects placeholder text). So a comment anywhere quoting orchestrate: opened PR #12, run x renders that issue done in every run scope.
• Folded LOW (finding 4): the JS milestone templates hand the reporter literal run <runId> to substitute (RUN_ID_HINT). If a reporter posts it unsubstituted, the \S+ run_id capture accepts <runId> as well-formed; since orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48's brief examples also carry run <runId>, all such markers pool into one phantom run that _resolve_run may pick as the default scope. No write-side guard, no read-side placeholder rejection.

Root-cause analysis (cause → effect)

1. Design seam. orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48 put the issue number inside the marker so a reader can verify which issue a marker is about. orchestrate: deterministic status command + push notifications (deferred R1a/R1c from #48) #50's consumer never used that field.
2. Prose tolerance meets a weaker consumer. parse_landed_marker was intentionally lenient because its only prior consumer (preflight) had a strong second factor (ancestor-SHA check). status reuses the parser but has no second factor, so leniency that was safe becomes a liability: any marker-shaped text is trusted.
3. Effect. Documentation, briefs, and even chat that quote the grammar become "markers" attributed to whatever issue hosts the comment → wrong rows. The terminal-verb fallback and the <runId> placeholder widen the blast radius from "wrong phase" to "falsely done" and "phantom default run".

Impact / blast radius

• The board — the entire deliverable of orchestrate: deterministic status command + push notifications (deferred R1a/R1c from #48) #50 — can show issues as working/done that are nothing of the sort, purely from quoted examples. orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48's brief guarantees at least one live false row today.
• Erodes trust in the exact tool meant to let a maintainer walk away and glance at status.

Proposed solution ideas

1. Attribute by number (primary fix). In load_status_universe (or build_status), drop any marker whose number is set and differs from the hosting issue's number. Milestone verbs always carry the number, so this is a clean, cheap filter and the correct second factor status was missing.
2. Reject placeholder run ids. Treat a run_id of <runId> — or any <…>-bracketed / non-substituted token the sanitiser-free \S+ capture admits — as malformed on the read path (parse to None / exclude from _resolve_run). Optionally tighten the run_id capture to exclude angle brackets. Add a parser test that an unsubstituted-template comment parses to None.
3. Constrain the terminal fallback. For the number-less terminal path, either require milestone/terminal corroboration, or restrict the any-run landed/opened-pr → done fallback to markers whose run_id is not a placeholder, so a quoted opened PR can't mark an issue done everywhere.
4. Red-first test (required by house TDD). An issue whose comments quote another issue's markers (and a prose-quoted opened-pr marker, and an unsubstituted <runId> template) must render queued. Demonstrate the test red against current build_status, then green after the filter.

Acceptance criteria (sketch)

• A marker whose number differs from the hosting issue is ignored by status (verified by a red-first test using orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48's own quoted-example payload → the issue renders queued).
• An unsubstituted run <runId> (or any bracketed placeholder run id) does not parse as a valid run and cannot become the default run scope.
• The terminal landed/opened-pr → done fallback cannot mark an issue done from a quoted marker on an unrelated issue.
• parse_landed_marker remains the single source of truth (no ad-hoc parsing); the fix lives in the consumer/attribution layer.
• Coverage added to tests/test_orchestrate_status.py; existing status tests still green.

Provenance

Discovered by the mandatory integration review of the orchestrate run wf_2d1a01f8-b49 (#48 + #50, merge mode) — a genuine cross-issue defect no per-issue verifier could see (seeing it meant reading the brief of issue 48 against the consumer added by issue 50). The run's fix-round cap was exhausted before this could be auto-remediated, so it is reported for a human. Findings 3 (MEDIUM) and 4 (LOW) folded here as one coherent fix.

Blocked by

Blocked by #52 (land the git-environment scrub first: until it lands, every hook-verified commit made while implementing this issue risks corrupting the real repository through the leaked GIT_DIR/GIT_INDEX_FILE).

[TBarregren]

Related — same orchestrate run (wf_2d1a01f8-b49)

Independent of the repo-corruption pair, but found in the same run:

• test fixtures leak GIT_DIR/GIT_INDEX_FILE and corrupt the real repo (index + core.bare) under the pre-commit hook #52 — root-cause test-env leak that corrupted the repo.
• orchestrate integrate can strand the working tree (ref-only advance); add structural bare-repo refuse + post-land tree check #53 — integrate structural hardening.

This ticket (#54) can be fixed on its own, in any order.

[TBarregren]

This was generated by AI during triage.

Agent Brief

Category: bug (MEDIUM + folded LOW — the status board silently shows wrong state)
Summary: Make status attribution trustworthy: drop any marker whose embedded issue number differs from the hosting issue (the second factor the consumer never used), and reject unsubstituted <runId>-style placeholder run ids at the parser so quoted grammar examples can neither create rows nor become the default run scope. Python-only; no engine change.

The issue body's evidence is verified against current code: load_status_universe (scripts/orchestrate.py:1740-1746) appends every parsed marker with no marker.number comparison, and the terminal fallback in build_status (lines 1838-1846) deliberately ignores run scoping. This brief settles the fix design.

Settled decisions

D1 — Attribute by number (primary fix). In load_status_universe, drop a parsed marker when marker.number is not None and marker.number != int(entry["number"]). Every milestone verb (started / implementation green / verification cleared / fix round k / parked) carries the number, so this kills the whole quoted-milestone class (#48's own brief guarantees a live false row today). Note the grammar asymmetry: the terminal landed <sha> on <branch> and opened PR #<pr> markers carry no issue number (marker.number is None — #<pr> is the PR, not the issue), so D1 intentionally lets them through; D2 handles their quoted forms.

D2 — Reject placeholder run ids at the parser. Tighten parse_landed_marker's run_id capture (currently \S+) to reject angle-bracketed tokens: an unsubstituted run <runId> — the form every quoted example and unfilled template actually carries — parses to None. This simultaneously (a) stops quoted terminal markers (opened PR #12, run <runId>) from marking their hosting issue done, and (b) prevents the phantom <runId> run from ever winning _resolve_run's default-scope election. Real run ids (wf_…, the documented unknown fallback) contain no angle brackets, and the preflight consumer (#49) only ever sees genuinely posted markers, so this tightening is safe for it — but add an explicit regression test that the preflight-relevant landed/opened-pr parses are unchanged for well-formed input.

D3 — Accepted residual. A comment quoting a fully substituted terminal marker (real-looking run id) on an unrelated issue can still fool the number-less terminal path. No cheap second factor exists for it without touching #49's grammar (out of scope). Record the limitation in load_status_universe's docstring; do not over-engineer beyond D1+D2 — the observed real-world poison (docs and briefs always quote with <runId>) is fully covered.

D4 — Red-first tests (house TDD). In tests/test_orchestrate_status.py: an issue whose comments quote another issue's milestone markers and a run <runId> terminal example renders queued — a fixture mirroring #48's real Agent Brief payload — demonstrated red against current build_status, then green. In tests/test_orchestrate.py (parser tests): an unsubstituted run <runId> marker parses to None; well-formed markers still round-trip; format_* outputs still parse.

Key interfaces

• scripts/orchestrate.py — load_status_universe (D1), the run_id pattern inside parse_landed_marker's grammar (D2), docstrings (D3). The fix lives in the parser + attribution layer; build_status's state mapping and terminal fallback logic need no structural change.
• tests/test_orchestrate_status.py, tests/test_orchestrate.py — D4.

Acceptance criteria

• A marker whose embedded number differs from its hosting issue is ignored by status; the orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48-brief-shaped fixture renders queued (red-first, then green).
• An unsubstituted run <runId> (any angle-bracketed run token) parses to None, creates no row, and can never become the default run scope.
• A quoted opened PR #<n>, run <runId> on an unrelated issue cannot render that issue done in any run scope.
• parse_landed_marker remains the single source of truth (no ad-hoc parsing anywhere in the status path); preflight-relevant parses of well-formed markers are regression-tested unchanged.
• All existing status/parser tests stay green; ruff, mypy, and scripts/audit.py pass.

Out of scope

• Any engine (orchestrate.workflow.js) change, including a reporter write-side placeholder guard — the read-side rejection (D2) already neutralises an unsubstituted post, and the reporter prompt instructs substitution.
• Any change to orchestrate: continuous out-of-band progress reporting for multi-hour runs (status command + durable milestone markers) #48/orchestrate: interrupt-safe restart — detect a dead cross-session resume, never re-implement landed work #49's marker grammar or verbs beyond the run_id token tightening.
• The fully-substituted-quote residual (D3) — documented, not solved here.

[TBarregren]

Fixed in 2f8cd16 (on main). _marker_attributable restores the missing second factor in the status consumer/attribution layer: a milestone marker whose embedded number differs from the hosting issue is dropped, and a marker whose run_id still wears its <placeholder> angle brackets is dropped — closing the quoted-grammar poisoning (finding 1), the phantom <runId> default run (findings 2 & 4), and the number-less opened PR terminal→done fallback (finding 3). Red-first coverage added to tests/test_orchestrate_status.py; full suite (602 tests) green. parse_landed_marker left untouched as the single source of truth.