Skip to content

Port the gate runner and pass cache from Python to Go #713

Description

@tbhb-agent

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

  1. 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.
  2. AC2: Given the same gate run with the force option, then it executes again whatever the cache holds.
  3. 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.
  4. AC4: Given a gate that fails, then its exit status and output reach the caller unchanged and no pass is recorded.
  5. 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.
  6. 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.
  7. AC7: After the change no Python gate helper remains in the skill sources or the generated skill copies, and mise run check-generated-skills passes after regeneration.
  8. 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.bats against 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.py and 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

  1. changed the title [-]Port the shell check judge to Go and settle one contract[/-] [+]Port the gate runner and pass cache from Python to Go[/+] on Oct 25, 2026
  2. updated Priority Standard and Work type Planned on Oct 25, 2026
  3. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement amendment for the shell-to-Go epic (#588), covering compatibility and protection. The acceptance criteria above stay as written, except where noted.

    1. AC7 and the Approach step that deletes gates.py conflict 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.
    2. Add an acceptance criterion: the replacement Go package is covered by the owner-approval protection patterns before the caller switches to it (the protection issue for replacement Go enforcement paths in the Wave 1 list).
    3. Dependencies: the block on Replace the shell check judge in the merge scripts with the Go evaluator #723 is a scheduling preference, not a contract this PR consumes. The dependency that matters is the protection issue above.
    4. The cache format stays readable by the older helper, so a gate cache written by either side is honoured or ignored with a fresh run (AC6).
  4. tbhb-dev-reviews commented on Oct 31, 2026

    @tbhb-dev-reviews

    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

    1. The body requires binary delivery through open Ship the Go binaries as release assets installed through mise's github backend #333 before callers switch, but that dependency is prose-only. Add the native blocker or evidence of a satisfied distribution prerequisite. Parent Replace complex shell with tested Go, starting with gates and merging #588 also requires build(agents): protect replacement Go enforcement paths #753 to protect replacement enforcement paths before operational rollout; carry that prerequisite explicitly and add a native blocker if it must precede this PR.
    2. Add an observable acceptance condition that a successful gate whose inputs change during execution cannot leave a reusable pass. The source compares before/after fingerprints at packages/agents/source/lib/gates.py:108; AC3 only describes when execution is required. Explicitly preserve gate-name/exact-command isolation, draft-specific invalidation, and removal of an earlier pass before a forced failed retry (tests/skills/source/lib/gates.bats:53, :86, :93, :124). These existing tests support, but do not substitute for, the acceptance contract.
    3. Clarify AC5 versus AC4/AC6: which failed Git reads still permit execution, what happens when root/cache/log resolution fails, and which status wins when a failed gate is followed by a failed fingerprint read? Current root/state reads are outside fallback handling (packages/agents/source/lib/gates.py:74). This requires an explicit behavior/compatibility decision rather than an assumed literal port.
    4. Complete Verify coverage: AC5 omits failed tool-version reads. AC4 should seed an earlier pass before a forced failure and prove its removal. AC6 needs unchanged shell-argument, output/status and generated-caller integration evidence against the branch binary; the Bats helper sources a generated library (tests/skills/source/lib/gates.bats:7). Preserve the per-gate command log and streaming output (packages/agents/source/lib/gates.py:97), explicitly required by the parent; this is the relevant audit/log criterion here.
    5. AC8 needs a named measurement environment, baseline, warm/cold conditions and statistic. An unspecified Go benchmark does not establish end-to-end lookup overhead including Git/mise subprocesses. AC7's deletion/generation instructions are testable delivery checks; put those mechanics in Approach and express the runtime-removal acceptance outcome without prescribing files.
    6. Complete delivery verification with the token/deployment generation sequence, consumer migration documentation, required checks and coverage floors. These are not all present in the current Verify plan.

    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.

    • 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.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact 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.
    • D5 The author signed each commit, the squash included.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
    • 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.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.
    • devtools-1 mise run check is green, and so are the coverage floors mise run cover-go-check and mise run cover-py-check, which check doesn't run.
    • devtools-2 A change to skill sources under packages/agents/source/ commits what mise run generate-skills, then mise run skill-tokens, then apm install rewrite, in that order, and check-generated-skills (part of lint) passes.
    • devtools-3 A consumer-visible change (the vendored payload, a skill, a reusable workflow, a task or a pin) has its consumer steps in docs/agent-skills-migration.md or 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    Priority

    Standard

    Work type

    Planned

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions