Skip to content

feat(pins): move pin checking to Go #798

Open

Description

@tbhb-agent

Problem

Part of #588. Split the read-only pin checker out of #330 as one reviewable PR. Consumers need the existing pin-consistency gate to work when its logic moves from shell to Go.

Evidence on origin/main at a0347a715fab580ece45edfcb3d13509620459a8: discovery and reporting are in .devtools/tasks/pins.toml:228; required tool checks and the optional agentrepo recommendation start at .devtools/tasks/pins.toml:312; agreement checks start at .devtools/tasks/pins.toml:339; tracked working-tree prose checks start at .devtools/tasks/pins.toml:374. Behavioral fixtures are in tests/tasks/pins.bats:53 and the failed-producer regression is in tests/tools/masked-status.bats:146.

Acceptance criteria

  1. AC1: The checker reports the version, commit where present, and repository-relative line location of every currently supported pin site.
  2. AC2: The checker exits 1 when discovered versions or commits disagree, or a discovered version is unreadable, identifying the conflicting values and their locations.
  3. AC3: A missing required guard-markdown, commit-lint, or gh-post tool pin causes the existing actionable refusal.
  4. AC4: An absent agentrepo tool pin remains a recommendation rather than a failure.
  5. AC5: Every stale tracked working-tree prose mention is reported with its location, mentioned version, pinned version, and manual-edit guidance.
  6. AC6: Non-functional: checking pins makes no network requests or repository changes.
  7. AC7: A failing discovery or comparison producer cannot turn partial output into a successful check.
  8. AC8: Non-functional: supported inputs retain the frozen shell's exit status, stdout, and stderr contract.
  9. AC9: Every existing consumer-facing checker entry point ends as a thin shell wrapper that probes the Go subcommand and falls back to the frozen shell when that capability is unavailable throughout N-2 compatibility.
  10. AC10: Non-functional: the replacement's behavior tests pass on Linux, macOS, and Windows.

Scope and non-goals

Cover root and common/claude/codex/agy APM pins, including quoted forms; guard-markdown, commit-lint, gh-post, and optional agentrepo tool pins; devtools vendir refs; pre-commit SHA and frozen-version comments; all matching reusable-workflow callers in both YAML extensions; and every named devtools Renovate preset. Preserve unpinned preset reporting, missing version comments, multiple pin sites, optional-file absence, and agreement without any SHA-bearing sites. The discovery contract is evidenced by .devtools/tasks/pins.toml:237 and tests/tasks/pins.bats:64.

Preserve the no-sites refusal in .devtools/tasks/pins.toml:333. Prose checking covers tracked working-tree text, multiple mentions on one line, no-match success, and exclusion of untracked prose (tests/tasks/pins.bats:211). A matching tombi header is prose evidence, not an additional structured pin site.

Non-goals: resolving remote tags, rewriting pins or prose, adding absent tool declarations, expanding accepted pin syntax, or changing preset ownership. Those are not part of this read-only gate; update behavior remains separately deliverable under #330.

Approach

Port the checker behavior to Go while preserving the existing consumer interface. Establish failing replacement tests before changing dispatch. Compare frozen shell and replacement on identical fixtures before removing migrated shell logic from the active path. Keep only the frozen compatibility implementation behind the required probe-and-fallback wrapper during N-2; capability detection must respect the consumer's selected tool version and must not retry a failed Go check through shell.

Dependencies

Size

S. One PR for pin checking, its behavior coverage, and compatible dispatch.

Verify plan

Write and observe the named replacement tests fail before changing dispatch or removing shell behavior. Use frozen-shell comparisons as the parity oracle; no live protected action is needed.

  • AC1, AC2, AC3, AC4: Failing tests first for each supported pin family, unsupported APM subpackages, missing required tools, optional agentrepo presence/absence, mismatched versions and commits, unknown versions, unpinned presets, absent optional files, and no sites. Port all checker assertions from tests/tasks/pins.bats:53 onward and add coverage for shell branches without an existing assertion.
  • AC5: Failing tests first for stale and matching prose, multiple mentions per line, tracked but modified text, untracked text, and no matches; compare all diagnostic locations with the frozen checker.
  • AC6, AC7: Failing tests first with network/mutation sentinels and producers that print plausible data before failing; include the regression from tests/tools/masked-status.bats:146 and genuine Git-search errors distinct from no matches.
  • AC8: Run old/new fixture comparisons for status and separate output streams; retain the no-sites and missing-comment refusals and update-task guidance.
  • AC9: Failing wrapper tests first for a capable binary, a missing binary, a missing subcommand, an unsupported executable, and a consumer-selected older binary beside a newer ambient one. Assert unchanged arguments, environment, streams, and statuses, and no fallback after an operational Go failure.
  • AC10: Run focused Go tests on Linux, macOS, and Windows, plus applicable shell wiring tests on Linux and macOS; then run mise run lint, mise run test, and relevant payload checks. Remove Bats assertions only once their behavior has replacement coverage.

Definition of done: D1–D10 apply.

Risks

A parser that accepts new syntax or omits unknown sites can turn a red consistency gate green. Prose scanning must read tracked working-tree text rather than staged contents. Binary rollout must preserve docs/compatibility.md:3 without making agentrepo a newly required pin.

Operator authority

None.

Activity

  1. added
    change:featPR adds or changes a feature (Conventional Commit type feat)
    on Oct 26, 2026
  2. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — #798 clearly specifies the read-only pin-checker port to Go with verified baseline evidence, N-2 fallback, and falsifiable AC1–AC10 test coverage
    Model: gemini-4-argon-high

    Checked

    • Live issue #798 body and comments (0 comments) and named issues #330, #588, and #333 against origin/main at a0347a715fab580ece45edfcb3d13509620459a8.
    • Problem and baseline evidence:
      • .devtools/tasks/pins.toml:228-310 defines devtools:check-pins and read_sites(), covering mise.toml, apm.yml, vendir.yml, .pre-commit-config.yaml, .github/renovate.json5, and .github/workflows/*.y*ml (.devtools/tasks/pins.toml:237-309).
      • .devtools/tasks/pins.toml:312-331 runs discovery, emits the non-fatal agentrepo recommendation on stderr (.devtools/tasks/pins.toml:313-315), and enforces required guard-markdown, commit-lint, and gh-post tool pins (.devtools/tasks/pins.toml:317-331).
      • .devtools/tasks/pins.toml:333-337 refuses with exit 1 when no pin sites are found.
      • .devtools/tasks/pins.toml:339-372 formats the discovered site table on stdout and reports version (? included) and commit SHA disagreements on stderr with remediation guidance.
      • .devtools/tasks/pins.toml:374-396 scans tracked working-tree prose via git grep -nI -o -E, propagates non-1 git grep errors, reports stale mentions on stderr, and prints final agreement on stdout.
      • Existing behavioral coverage in tests/tasks/pins.bats:53-270, synchronization coverage in tests/skills/agent-merge-pr/pins.bats:355-412, and failed-producer regression coverage in tests/tools/masked-status.bats:146-153 match the issue's citations.
    • Acceptance criteria (AC1–AC10): Each AC is observable, testable, scoped to a single outcome, and states behavioral and non-functional requirements (offline/read-only execution in AC6, stream/exit-status parity in AC8, N-2 probe-and-fallback wrapper in AC9 per docs/compatibility.md:3, and cross-platform test execution on Linux, macOS, and Windows in AC10) without prescribing internal Go package layout or parser implementation details.
    • Scope, non-goals, and dependencies: Cleanly separates the read-only checker from devtools:update-pins (.devtools/tasks/pins.toml:57-207), which remains under #330. Dependencies #330, #588, and #333 are real and accurately classified.
    • Verify plan: Every AC (AC1–AC10) is mapped to concrete failing-first tests, frozen-shell parity comparisons, failed-producer/network/mutation sentinels, N-2 wrapper fallback cases, and repository gates (mise run lint, mise run test).
    • Risks and operator authority: Risks cover syntax over-acceptance, working-tree vs. index prose scanning, and N-2 rollout compatibility (docs/compatibility.md:3). .devtools/tasks/pins.toml, cmd/agentrepo/main.go:1-15, internal/, and tests/ do not match golden_paths, owner_paths, or constitutional_paths in .agents/agent-skills.conf:4-9, so Operator authority: None. is accurate (note that touching .devtools/tasks/pins.toml requires a Compatibility: line in the PR description per docs/compatibility.md:11-20 and packages/agents/source/skills/pr/shared/scripts/validate-description.sh:491-492).

    Gaps

    None

    Split

    Size S fits one pull request (devtools:check-pins is 100 lines in .devtools/tasks/pins.toml:228-396, already split from devtools:update-pins in #330). No further split is needed.

    Definition of done

    The definition of done for #798 (Feature). Tick each item and give its evidence. (Ticked boxes indicate planning coverage in the issue and repository gates, not completed 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. — Covered for planning by the ## Verify plan mapping for AC1–AC10 and Definition of done: D1–D10 apply. in #798, enforced on PR descriptions by packages/agents/source/skills/pr/shared/scripts/validate-description.sh:1-506.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. — Covered for planning by Definition of done: D1–D10 apply. in #798 and enforced by the merge workflow in packages/agents/source/skills/merge-pr/shared/scripts/preflight.sh:1-224 and packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:1-376.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. — Covered for planning by AC10 and ## Verify plan (mise run lint, mise run test) in #798 and enforced by .agents/agent-skills.conf:6-8 and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:430-538.
    • 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. — Covered for planning by Definition of done: D1–D10 apply. in #798 and enforced by .devtools/scripts/check-leaks.sh:1-573 and cmd/gh-post/main.go:1-15.
    • D5 The author signed each commit, the squash included. — Covered for planning by Definition of done: D1–D10 apply. in #798 and enforced by packages/agents/source/lib/signing.sh:1-91 and packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:1-376.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Covered for planning by Definition of done: D1–D10 apply. in #798 and .github/pull_request_template.md:1-60.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. — Covered for planning by Definition of done: D1–D10 apply. in #798 and enforced by .commit-lint.toml:1-40 and packages/agents/source/skills/pr/shared/scripts/validate-description.sh:1-506.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. — Covered for planning by ## Size (S, one PR scoped to the read-only pin checker) and Definition of done: D1–D10 apply. in #798.
    • 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. — Covered for planning by Definition of done: D1–D10 apply. in #798 and .github/pull_request_template.md:1-60.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. — Covered for planning by Definition of done: D1–D10 apply. in #798 and the merge workflow in packages/agents/source/skills/merge-pr/shared/scripts/squash-message.sh:1-278.

    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

    change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions