fix: dispatch via run-agent.sh; strip inline-code from linkage scan - #684
fix: dispatch via run-agent.sh; strip inline-code from linkage scan#684johanzander wants to merge 3 commits into
Conversation
…de --bg The legacy `claude --bg` dispatch could hang forever on a permission prompt nobody was there to answer. run-agent.sh owns the isolation: a private clone at .agent-clones/, the dev-role gh token, a restricted egress allowlist, and permissions skipped inside the disposable container — the boundary is the container, so a headless dispatch never stops to ask. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR #679 explained its own fix with the literal line `- Blocked by #100 -- part of #409`, and that backticked worked example bounced issue #409 to In Verification for work it never did. Inline-code spans are examples, not linkage declarations — strip them before scanning. The no-auto-close spellings (`Part of #N`, `tracking #N`) still drive the column for genuine intermediate PRs, so the documented behavior of test_a_merged_intermediate_pr_keeps_the_issue_in_verification holds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@claude-bot review |
There was a problem hiding this comment.
Review
Root-cause match: partial. The PR's stated root cause — "A real linkage declaration is never in code markup" — is correct for the merged-PR scan it patches (scripts/backlog-digest.sh:133, refs: [ (.body // "") | gsub("\[^\`]*`"; "") | scan(...) ]), and matches the actual offending text in merged PR #679's body (- Blocked by #100 -- part of #409inside a backtick span). Verified withgh pr view 679 --json body`.
But the identical bug is left live in the open-PR linkage path, linkage_body/pr_matches_issue (scripts/backlog-digest.sh:253-259):
def linkage_body($body):
($body // "")
| gsub("(?i)(blocked by|depends on|unblocks?(?:ing)?|related to|relationship to|follow[- ]?up to|see also|see|not part of) #[0-9]+"; "");
def pr_matches_issue($p; $n):
(linkage_body($p.body) | test("(?i)(^|[^0-9])#\($n)\\b"))No backtick-stripping is applied here, only cross-ref phrase stripping — and part of #N is deliberately not in the stripped-phrase list (it's a legitimate linkage phrase for real PRs). So a code-quoted worked example containing part of #N still links.
This isn't hypothetical — it's live on this PR right now. I ran linkage_body/pr_matches_issue verbatim against PR #684's own current body (which quotes `- Blocked by #100 -- part of #409` while explaining the fix, same as #679 did):
$ jq -n --rawfile body <(gh pr view 684 --json body --jq .body) -f test.jq
{ "matches_409": true, "matches_679": true }
While this PR is open, backlog-digest.sh's open-PR scan links it to issue #409 (and #679) and would place #409 in "In Review" — the exact incident class this PR exists to fix, reproduced by this PR's own description, in the code path the PR didn't touch.
Test coverage: test_a_backticked_worked_example_does_not_flip_an_issue (backend/tests/test_backlog_digest.py) only exercises the merged-PR scan. It would not catch a regression in pr_matches_issue/linkage_body for open PRs — no test constructs an open PR with a backtick-quoted linkage phrase.
Scope: the PR bundles two unrelated fixes — the SKILL.md run-agent.sh dispatch rewiring and the digest regex fix — described separately and each internally minimal, so not a blocker on its own, but worth calling out.
Verdict: REQUEST_CHANGES. Please apply the same code-markup stripping to linkage_body (or a shared helper both scans call) so the fix covers both linkage paths, and add a test that pins an open PR with a backtick-quoted part of #N example not linking. No hard rule violations found otherwise (no Optional, hasattr, or silent-fallback issues — this is bash/jq, not Python).
The previous commit stripped backticked spans in the merged-PR scan only. The open-PR path (`linkage_body`/`pr_matches_issue`) is a separate jq program and kept matching, so a PR that merely QUOTES `part of #N` as a worked example still linked itself to that issue and reported it In Review. `part of #N` is deliberately absent from the stripped-phrase list -- it is real linkage on a real intermediate PR -- so the phrase list could never cover this; the markup has to go first. PR #684's own body reproduced it: quoting `- Blocked by #100 -- part of #409` while explaining the merged-scan fix linked it to #409. The stripping def now lives in one shell variable, `jq_strip_code_spans`, prepended to both jq programs, so there is one definition and two callers rather than two copies to drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0189hXkvdrhzwdhXNbS47KiT
Summary
scripts/run-agent.shinstead ofclaude --bg. A headless dispatch runs in a disposable container with the dev-role token, a restricted egress allowlist, and permissions skipped inside the container — so it can never stop to ask for a permission nobody is there to answer (the old path's exact hang).`- Blocked by #100 -- part of #409`as a backticked worked example, and that bounced issue Consolidate PredictionSnapshotStore + ScheduleStore into the unified per-day DailyView format #409 to In Verification for work it never did. A real linkage declaration is never in code markup. GenuinePart of #N/tracking #Nintermediate PRs still keep their issue In Verification (existing behavior pinned bytest_a_merged_intermediate_pr_keeps_the_issue_in_verification).Test plan
test_a_backticked_worked_example_does_not_flip_an_issue— fails on the pre-fix scan, passes now.venv/bin/pytest backend/tests/test_backlog_digest.py(69) +test_backlog_rhythm.py/test_backlog_board_init.py(77) — all passtest_quality_check_mypy_gate.pyfailures are sandbox artifacts (pass unsandboxed — the ratchet'sgit archive|tarruns under/var/folders)scratch_*.pyin.claude/worktrees/bench-pwl-everywhere/(CI checks a clean checkout)