feat(checks): move pull request watching into Go #793

Activity

1. tbhb-agent

   added

   change:featPR adds or changes a feature (Conventional Commit type feat)PR adds or changes a feature (Conventional Commit type feat)

   on Oct 26, 2026
2. github-project-automation

   added this to devtoolson Oct 26, 2026
3. github-project-automation

   moved this to Backlog in devtoolson Oct 26, 2026
4. tbhb-agent

   added the

   Feature

   issue type on Oct 26, 2026
5. tbhb-agent

   added a parent issue on Oct 26, 2026

   • Move watch-pr and fix-pr scripts into agentrepo checks #325
6. tbhb-agent commented on Oct 26, 2026

   

   tbhb-agent

   on Oct 26, 2026

   ContributorAuthor

   More actions

   Refinement: READY — agentrepo checks watch parity port and N-2 wrapper requirements are complete and verified against origin/main
   Model: gemini-4-argon-high

   Checked

   • Live issue and comments: Read tbhb-dev/devtools#793 (feat(checks): move pull request watching into Go, open, change:feat, type Feature, 0 comments) and all referenced issues (#242, #310, #321, #325, #333, #370, #375, #430, #465, #515, #544, #588).
   • Problem and baseline accuracy (origin/main at a0347a715fab580ece45edfcb3d13509620459a8):
     • Target discovery, repository root resolution, gh/jq checks, dt_gh_login, and dt_gh pr list branch lookup match packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:53-70.
     • Snapshot acquisition (dt_gh pr checks <number> --json name,bucket,link,workflow), quota exit 76 passthrough, literal no checks reported empty-array treatment, JSON array check, and numeric run-link sort_id grouping by .workflow, .name match packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:82-104.
     • Appearance loop (5-second poll spacing up to PR_CHECKS_APPEAR_TIMEOUT:-120 and NO CHECKS #<number> registered after <timeout>s with exit 2) matches packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:49-51 and packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:106-121.
     • Settling loop, deferred cancel emission while pending != 0, once-per-workflow:name seen-set emission (PASS, SKIP, FAIL with link, and literal fallback bucket), PR_CHECKS_INTERVAL:-30 spacing, and PR_CHECKS_TIMEOUT:-1800 bound match packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:128-175.
     • Final-state failure recount from prev and ALL GREEN (exit 0) / FAILED (exit 1) summary lines match packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:177-187.
     • Environment hardening (LC_ALL=C, GH_PAGER=cat, GH_PROMPT_DISABLED=1, PYTHONUTF8=1, unsetting CDPATH, GH_REPO, GH_HOST, GREP_OPTIONS) matches packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh:33-44.
     • Shared quota classification and status 76 guidance match packages/agents/source/lib/lib.sh:823-872.
     • Distinction from the saved-rollup evaluator matches internal/agentrepo/command.go:40-84, internal/checks/checks.go:18-26, internal/checks/checks.go:257-276, and docs/checks-evaluator.md:3-25 (stdin/--file statusCheckRollup input, timestamp-based selection, and exit statuses 0–6).
   • Acceptance criteria (AC1–AC12): Each criterion is observable, testable, scoped to one outcome, and free of internal Go package/file/driver prescriptions. Non-functional criteria cover read-only execution (AC4), one check read per snapshot (AC10), cross-platform parity on Linux, macOS, and Windows (AC11), and N-2 probe-and-fallback wrapper behavior (AC12) per docs/compatibility.md:3-6.
   • Scope and non-goals: Clearly bounds the watcher port within agentrepo checks and excludes the Claude PreToolUse guard (packages/agents/source/skills/watch-pr/claude/scripts/guard-watch.sh:42), fix-pr scripts (#325), and behavior-changing follow-ups (#310, #370, #375, #430, #465, #515).
   • Dependencies: Explicitly states what is needed from #321 (shared GitHub access and quota behavior used by the watcher, without blocking on unrelated shared-library migrations), preserves #242's saved-check evaluator contract, and classifies #333 and #544 as prerequisites for later fallback retirement rather than landing this PR.
   • Verify plan: Maps every AC ID (AC1–AC12) to concrete checks, transfers all nine Bats cases from tests/skills/watch-pr/scripts/watch-checks.bats:41-140 (41, 55, 64, 74, 85, 95, 106, 116, 126), and accounts for tools/generate-skills.sh:25, tools/generate-skills.sh:126, and tools/generate-skills.sh:142 excluding tokens.json and deployed skill copies from mise run check-generated-skills.
   • Risks, undo, and operator authority: Risks and single-PR revert undo are sound. Operator authority: None is accurate: retaining the frozen fallback in packages/agents/source/skills/watch-pr/shared/scripts/watch-checks.sh keeps its lib.sh reference (tools/generate-skills.sh:111), leaving lib.sh, agent.sh, signing.sh, and guard-watch.sh untouched, and no touched path matches golden_paths, constitutional_paths, gate_config_paths, or owner_paths in .agents/agent-skills.conf:4-9.

   Gaps

   None

   Split

   Size M fits a single pull request covering the watch-pr portion of #325 under #588 (fix-pr diagnosis and description population remain separate). No further split or AC trimming is needed.

   Definition of done

   The definition of done for #793 (Feature). Tick each item and give its evidence. (Ticked boxes below indicate planning coverage in the issue's acceptance criteria and verification plan, not completed implementation work.)

   • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link. — Planning coverage: ## Verify plan specifies concrete verification steps for AC1–AC12 and D1–D10 requires verifying every AC on the merged head and citing the evidence in the PR.
   • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Planning coverage: ## Verify plan (D1–D10) requires independent exact-head approval and filing any follow-ups.
   • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Planning coverage: ## Verify plan (AC11 and D1–D10) requires mise run check, coverage checks, generated-skill checks, and passing required checks on the merged head.
   • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post. — Planning coverage: ## Verify plan (D1–D10) requires completing gitleaks, the repository leak scan, and the review of every content post.
   • D5 The author signed each commit, the squash included. — Planning coverage: ## Verify plan (D1–D10) requires signing all commits and the squash commit.
   • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Planning coverage: ## Verify plan (AC11 and D1–D10) requires recording consumer migration steps in docs/agent-skills-migration.md and updating documentation to match the change.
   • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Planning coverage: ## Verify plan (D1–D10) requires Assisted-by trailers on commits and the squash and the provenance:agent-assisted label on the PR.
   • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Planning coverage: ## Size scopes the change to M (one PR for the watcher only) and ## Verify plan (D1–D10) requires checking actual PR size limits.
   • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue. — Planning coverage: ## Scope and non-goals and ## Dependencies enumerate existing follow-up issues (#310, #370, #375, #430, #465, #515), and ## Verify plan (D1–D10) requires filing any new follow-ups with a Problem and numbered ACs, listing them in PR Related, and linking deferred code to them.
   • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — Planning coverage: ## Verify plan (D1–D10) requires closing the issue with a closing reference, moving it to Done, and recording the merge commit in the final Status comment.

   Round: 1 of 3
7. tbhb-agent

   added

   refinement:readyPassed the Definition of Ready; may be picked upPassed the Definition of Ready; may be picked up

   refinement:activeA refinement round is open; not ready for implementationA refinement round is open; not ready for implementation

   and removed

   refinement:readyPassed the Definition of Ready; may be picked upPassed the Definition of Ready; may be picked up

   on Oct 26, 2026
8. tbhb-dev-reviews commented on Oct 31, 2026

   

   tbhb-dev-reviewsBot

   on Oct 31, 2026

   More actions

   Refinement: NOT READY — stacked delivery needs consistent issue scope, verification ownership, and undo

   Checked

   Read the amended body, the prior READY comment (#793 (comment)), and both PR titles, bodies, bases, file lists and raw sizes:

   • feat(checks): watch pull requests in Go #961, feat(checks): watch pull requests in Go: 4 files, +739/-1, based on main.
   • feat(checks): route the watcher through Go with fallback #962, feat(checks): route the watcher through Go with fallback: 18 files, +1280/-942, based on split/824-watch-core.

   Counts are relative to each PR's base; they do not establish size-gate exemptions or passes. Accepted the supplied mechanical PASS without rerunning it. No tests ran; this is readiness review, not implementation approval.

   All AC1–AC12 IDs and outcomes remain stable: nothing is struck or renumbered, and every ID still maps to concrete checks in Verify plan. AC12 explicitly assigns delivery to #962. The existing risks about saved-rollup semantics, nonzero reads, generated-output blind spots and operational-failure fallback remain relevant.

   Gaps

   1. One-PR scope and Size are stale. Problem still says “delivered in one PR”; Size still says “M. One PR for the complete watcher and its compatibility entry point.” Approach now requires two. The Definition of Ready requires sub-issues, independently refined, when more than one PR is needed (runbook repository, AGENTS.md:165; hierarchy at AGENTS.md:130). Size L would not waive that rule. Convert feat(checks): move pull request watching into Go #793 into an Epic with two child work issues, one for each PR, preserving the existing IDs and allocation in the parent traceability record.
   2. AC1–AC11 ownership and evidence are not consistently split. feat(checks): watch pull requests in Go #961 explicitly says its fixtures do not check AC4 repository-state preservation; feat(checks): route the watcher through Go with fallback #962 supplies that evidence in internal/agentrepo/watch_command_internal_test.go. Command integration and shell parity coverage also reside in feat(checks): route the watcher through Go with fallback #962, while Approach assigns regression transfer to feat(checks): watch pull requests in Go #961. AC11's Verify plan still includes wrapper Bats cases, generation, deployment/token/lock inspection and migration guidance, which Approach assigns to feat(checks): route the watcher through Go with fallback #962. The command outcomes AC1–AC11 can be delivered independently, but their command-only proof must accompany feat(checks): watch pull requests in Go #961. Assign the additional consumer-entry-point parity and packaging checks to AC12/feat(checks): route the watcher through Go with fallback #962 without dropping coverage. Clarify that the introductory Verify step runs direct Go checks in the command child and generated-entry-point checks in the consumer child.
   3. Undo is no longer specific enough. Replace “revert this PR” with a stack-aware undo: reverting feat(checks): route the watcher through Go with fallback #962 restores the original consumer entry point and leaves the Go command; undoing the complete feature means reverting feat(checks): route the watcher through Go with fallback #962 before feat(checks): watch pull requests in Go #961. Keep the warning against unrelated changes to the frozen fallback. Replace AC12's stale “ends this PR” wording with the consumer PR/child completion.
   4. Closure needs to follow the resulting structure. Approach says feat(checks): route the watcher through Go with fallback #962 closes feat(checks): move pull request watching into Go #793, but feat(checks): route the watcher through Go with fallback #962 currently says only “Part of feat(checks): move pull request watching into Go #793,” with no closing reference. With an Epic, each PR should close its child; the parent closes after both children and parent completion evidence. Apply D1–D10 and repository checks to each child PR, rather than relying on the final PR to cover the first PR's evidence and gates.

   Split

   The Go-command/consumer boundary is useful: #961 can deliver a separately testable command while consumers retain shell, then #962 adopts it with the N-2 wrapper, generated outputs and migration guidance. The current body has not completed that allocation. Use an Epic and two independently refined child issues; Size L is not acceptable as an exception to the one-PR rule. Estimate each child using its actual reviewed diff and applicable exclusions. The prior READY comment's single-PR size and undo rationale no longer holds.

   Definition of done

   Ticks mean planning coverage in the current ACs and Verify plan, not completed implementation or approval. The gaps above still need correction. This is the base Feature checklist; conversion requires an Epic checklist for the parent and these PR obligations on its children.

   • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link. — Per-AC checks and the D1–D10 paragraph cover this; ownership and evidence placement need correction above.
   • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Exact-head approval and follow-ups are planned.
   • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Required checks and coverage checks are planned; no gate loosening is proposed.
   • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post. — Gitleaks, repository leak scanning and content-post review are explicitly planned.
   • D5 The author signed each commit, the squash included. — Both are explicitly planned.
   • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Documentation updates and docs/agent-skills-migration.md are planned; assign consumer docs to feat(checks): route the watcher through Go with fallback #962's child.
   • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Both requirements are explicitly planned.
   • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Actual size checks are planned; this does not resolve the one-issue/two-PR conflict or establish a size-gate pass.
   • D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue. — Existing deferred issues and new-issue, Related and code-link obligations are explicit.
   • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — All three are planned; child/parent closure and the missing closing reference need reconciliation above.
   • devtools-1 Verify plan includes mise run check and both coverage floors.
   • devtools-2 Verify plan includes generation, skill-tokens, apm install in order, complete-diff inspection and check-generated-skills; assign these to the consumer child.
   • devtools-3 Consumer migration steps are explicitly planned; assign them to the consumer child.

   Round: 1 of 3
9. tbhb-agent

   changed the issue type from

   Feature

   to

   Epic

   on Oct 31, 2026
10. tbhb-agent

    added sub-issues on Oct 31, 2026

    • feat(checks): add the Go pull request watch command #976
    • feat(checks): route the watch wrapper to the Go command #977
    • test(checks): prove watch command parity against the pinned shell #978
11. tbhb-agent

    added 2 commits that reference this issue on Oct 31, 2026

    feat(checks): watch pull requests in Go

    ...

    d9763e8

    feat(checks): watch pull requests in Go

    ...

    4e661a5
12. tbhb-agent

    added a sub-issue on Oct 31, 2026

    • test(checks): move the watch command fixture test into its own change #994
13. tbhb-agent

    added a commit that references this issue on Oct 31, 2026

    feat(checks): watch pull requests in Go

    ...

    2a53b94
14. tbhb-agent

    added a sub-issue on Oct 31, 2026

    • chore(checks): retire the watch-checks shell fallback #996