Repository navigation
Description
Problem
The commit, pr, merge-pr, watch-pr and analyze-session skills run their gates through a Python helper, packages/agents/source/lib/gates.py, copied beside each generated shared library, and through the shell function dt_gate that calls it. The helper decides whether a gate's earlier pass can be reused: it fingerprints the clean tree, the selected tool versions and the gate inputs, and stores passes in a cache. This is feature 4 of #588. Keeping it in Python leaves a second runtime on the commit and merge path and a fork-heavy Bats suite (tests/skills/source/lib/gates.bats). The reuse rule is safety-relevant: a stale pass reused on changed input would let unchecked code through a gate.
Acceptance criteria
- AC1: Given a clean tree and a gate that passed on identical inputs, when the gate is run again, then it is not executed and the output says it was reused.
- AC2: Given the same gate run with the force option, then it executes again whatever the cache holds.
- AC3: Given any uncommitted or untracked change, a different commit tree, a different selected tool version or a changed gate input (including an ignored config file), when the gate runs, then it executes and a stale pass is never reused.
- AC4: Given a gate that fails, then its exit status and output reach the caller unchanged and no pass is recorded.
- AC5: Given an unreadable, truncated or corrupt cache entry, or a failing Git or tool-version read, when the gate runs, then it executes and is never reported as reused.
- AC6: Existing callers keep working with their current arguments, exit statuses and reuse message, and a cache written before the change is either honoured or ignored with a fresh run, never mis-read as a pass.
- AC7: After the change no Python gate helper remains in the skill sources or the generated skill copies, and
mise run check-generated-skillspasses after regeneration. - AC8: Non-functional: a reused-pass lookup on a clean tree adds under 200 ms over running no gate.
Scope and non-goals
In scope: the pass-cache logic and its command line, the shell caller dt_gate, and the gate Bats cases.
Non-goal: changing which gates a skill runs or what they check.
Non-goal: the shell check judge (#723) and the rest of lib.sh (a later wave).
Out of scope: the Go commit-lint, signing and hook commands.
Approach
Port the fingerprint and cache logic into the existing agentrepo Go command tree as a gate subcommand, keeping the cache key inputs, so existing cache semantics carry over. Replace the dt_gate call into Python with a call to the Go command, delete gates.py and its generator copy step in the same PR, and regenerate skills from the canonical sources, never by hand. The existing Bats cases are the behavioural oracle: replay them against the Go command before deleting the Python. Order after #723, as #588 sequences the gate work.
Dependencies
Blocked by #723 (epic order: the judge migration lands first, and both edit the same generated output). Binary availability to deployed skills follows the release-asset story #333, so callers may only switch once the command ships with the skills.
Size
S. One PR.
Verify plan
- AC1: Go table test and the reuse case of
tests/skills/source/lib/gates.batsagainst the branch build. - AC2: forced-run case in the same Bats file and a Go test.
- AC3: Go table tests that change one input at a time (dirty tree, untracked file, tree hash, tool version, ignored config) and assert execution.
- AC4: Go test with a failing gate command asserting status, output and an empty cache.
- AC5: Go tests with corrupt, truncated and unreadable cache files and a stubbed failing Git read.
- AC6: the existing gate Bats cases run unchanged, plus one test that opens a cache entry written by the Python helper.
- AC7: grep shows no
gates.pyand no Python helper copy;mise run check-generated-skills. - AC8: Go benchmark on a clean fixture tree.
Risks
Reusing a pass when it should have run, which weakens every gate; AC3 and AC5 exist for this and the table tests must fail closed. A cache format drift could make old entries read wrongly; AC6 covers it. The undo is reverting the PR, which restores the Python helper and the previous generated outputs.
Operator authority
None
Activity
[-]Port the shell check judge to Go and settle one contract[/-][+]Port the gate runner and pass cache from Python to Go[/+]tbhb-agent commentedon Oct 26, 2026
Refinement amendment for the shell-to-Go epic (#588), covering compatibility and protection. The acceptance criteria above stay as written, except where noted.
gates.pyconflict with the N-2 rule in docs: state the N-2 compatibility policy #661. Consumers on older pinned binaries must keep working without configuration changes, so the Python helper and its generator copy step stay in this PR as a frozen fallback. The caller probes for the Go gate subcommand without writes. When the subcommand is absent, the caller uses the existing Python path. When the subcommand is present and fails, returns partial output or returns an invalid result, the gate refuses and never falls back to repeat the run. A separate XS issue removes the fallback after every supported release pin ships the command. AC7 then applies to that issue.tbhb-dev-reviews commentedon Oct 31, 2026
Refinement: NOT READY — gate-cache safety and rollout criteria need clarification
Checked
Read the current #713 body with gh, its native blockedBy links, parent #588, and relevant source found with git grep. Accepted the supplied mechanical PASS without rerunning it. No tests were run.
The current body is “Port the gate runner and pass cache from Python to Go.” AC3 concerns cache invalidation. It contains no review-dismissal or “superseded by a later review id” rule. Refusing to dismiss tbhb reviews and distinguishing supersession on older/current heads are consequently inapplicable to this issue.
The Approach fits the thin dt_gate caller (packages/agents/source/lib/lib.sh:909) and canonical helper-copy step (tools/generate-skills.sh:111). For the requested comparison, review edit uses operation parsing (internal/ghpost/args.go:50), shared content scanning (internal/ghpost/run.go:74), dispatch (internal/ghpost/run.go:240), and audit records (internal/ghpost/audit.go:17). Those posting semantics are separate from this gate migration.
Native blockedBy contains #723 only. #713 is feature four of #588, so first-child epic blockers do not apply; its named predecessor is represented.
Gaps
Split
S can fit one PR for this leaf migration, targeted tests and generated output once the failure contract is settled. Keep release delivery and golden-policy changes in their existing dependency issues. No further feature split is needed. Do not add dismissal/supersession behavior to #713.
Definition of done
The definition of done for #713 (Feature). Tick each item and give its evidence.
check-leaks.shwith the repository's private-strings list, and the leak review of each post.docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.provenance:agent-assistedlabel.Status:comment names the merge commit.mise run checkis green, and so are the coverage floorsmise run cover-go-checkandmise run cover-py-check, whichcheckdoesn't run.packages/agents/source/commits whatmise run generate-skills, thenmise run skill-tokens, thenapm installrewrite, in that order, andcheck-generated-skills(part oflint) passes.docs/agent-skills-migration.mdor the adoption docs, or the PR's Verification says "No consumer impact" and why.Coverage: D1 partial (AC IDs mapped, proof gaps above); D3 partial (generation check only); D8 partial (S stated); devtools-2 partial (regenerate/check stated, tokens/deployment absent); devtools-3 partial (caller compatibility stated, adoption evidence absent). Other items have no explicit AC/Verify coverage. Boxes remain unchecked because none is fully evidenced by this refinement.
Round: 1 of 3