Skip to content

Repository navigation

refactor(leaks): port staged comparison to Go #816

New issue
New issue
Open
Feature
Open
refactor(leaks): port staged comparison to Go#816
Feature
Labels
change:featPR adds or changes a feature (Conventional Commit type feat)refinement:readyPassed the Definition of Ready; may be picked up

Description

@tbhb-agent
tbhb-agent
opened on Oct 26, 2026

Problem

Part of #588. This is the staged-comparison slice of #329.

At origin/main (a0347a715fab580ece45edfcb3d13509620459a8), staged scans compare index blobs with a base and allow inherited literal counts (.devtools/scripts/check-leaks.sh:189, .devtools/scripts/check-leaks.sh:306). Pattern configuration comes from the base (.devtools/scripts/check-leaks.sh:779). A port that scans checkout bytes or treats this as a full-index gitleaks scan would change the gate.

Acceptance criteria

  1. AC1: --staged checks the changed index entries and their indexed contents against the selected base, with the same results as the frozen shell regardless of unstaged edits or submodule diff settings.
  2. AC2: Inherited private-literal occurrence counts do not block an unrelated edit or line shift, while added occurrences are findings at their index locations, with the same per-path and per-entry accounting as the frozen shell.
  3. AC3: Renames, copies, type and mode changes, empty files, binary content, symlink contents and gitlink names retain the staged scanner's current findings, including names newly made private and renames away from private names.
  4. AC4: Staged pattern checks and path allowances use the selected base's configuration; changing checkout or index configuration cannot weaken that scan, and working-tree skips have no effect.
  5. AC5: The selected base remains the explicit DEVTOOLS_LEAK_BASE tree when supplied, otherwise HEAD's tree, or the empty tree only for a genuinely unborn branch.
  6. AC6: A corrupt or unmerged index, invalid base, failed private-list discovery, unreadable blob or unreadable configuration returns status 2 instead of a clean result.
  7. AC7: Results retain statuses 0/1/2, the current streams, entry and rule labels, index line or binary locations and opaque sensitive names, without printing private values or sensitive error arguments.
  8. AC8: Non-functional: scanning leaves the caller's index, refs and working files unchanged.
  9. AC9: Non-functional: consumer entry points end as thin shell wrappers that probe Go subcommand support without writes or consuming input, use the unchanged frozen shell for absent binary or capability, and run supported execution once with preserved streams and status and no fallback after failure.
  10. AC10: Non-functional: native Go behavior works on Linux, macOS and Windows, and existing N-2 consumers require no configuration or binary-floor change.
  11. AC11: Task mode selection and published hook contracts retain their current behavior.
  12. AC12: Non-functional: no temporary matched content remains after the scan finishes.

Scope and non-goals

In scope: the staged comparison and configuration boundary and its existing callers. Source evidence: .devtools/scripts/check-leaks.sh:189, .devtools/scripts/check-leaks.sh:306, .devtools/scripts/check-leaks.sh:779, .devtools/tasks/leaks.toml:6 and .pre-commit-hooks.yaml:128.

Coverage includes .devtools/scripts/check-leaks.bats:226, .devtools/scripts/check-leaks.bats:279, .devtools/scripts/check-leaks.bats:358, .devtools/scripts/check-leaks.bats:404, .devtools/scripts/check-leaks.bats:516, .devtools/scripts/check-leaks.bats:535, .devtools/scripts/check-leaks.bats:627 and every staged regression from .devtools/scripts/check-leaks.bats:1578 through .devtools/scripts/check-leaks.bats:1744.

Overlap: AC2 and AC6 absorb #501's retained private-list consumption and incomplete-input refusal for this mode; new source lookup, source precedence and notices, and provisioning remain #501. AC1 preserves indexed-byte isolation also required by #551 AC4, but it does not absorb that issue's full-index gitleaks coverage. #551's local/ci modes, history/base resolver, caller changes and owner-approved coverage reduction stay out. Neither issue is closed by this port.

Non-goals: rescanning unchanged indexed files as a new policy, changing inherited-literal allowances, source provisioning, publication or whole-object scan modes, and changing hook IDs or stages. The frozen shell's staged pattern behavior is preserved even where it differs from literal occurrence accounting. Fallback retirement is a separate N-2 follow-up under D9.

Approach

Move the scoped behavior to tested Go, with failing cases first. The mechanism is the implementer's choice within these outcomes. Keep the compatibility implementation frozen.

Dependencies

Blocked by #753 before replacement enforcement is activated; cite its merged protection evidence. #329 tracks the port. #501 and #551 retain the excluded behavior changes. #661 supplies the compatibility policy; #333 is distribution work, not a higher binary floor for this port.

Size

M. One pull request for the staged comparison contract and caller compatibility.

Verify plan

First write failing Go cases for an index-only finding, an extra inherited occurrence and an unreadable index blob. Port all scoped Bats cases before reducing shell coverage.

  • AC1: clean index versus leaky checkout and the reverse, partial staging in both directions, unchanged and deleted entries, and submodule entries with ignore settings; compare shell and Go selection and results.
  • AC2, AC3: per-path and per-entry budgets, same-line additions, line shifts, renames with and without new occurrences, rename away from and into a private name, copies, mode and regular-file/symlink changes, empty and binary files, gitlink names and unusual path bytes. Compare all finding locations and counts with the frozen shell.
  • AC4: pin a base rule and allowance, then weaken each in the checkout and index independently; the original finding remains. An invalid working-tree skip file must not affect this mode. Include inherited patterns as well as literals to prove their distinct behavior.
  • AC5: explicit amend parent, default HEAD, genuine unborn branch, invalid explicit base and a broken non-unborn HEAD; assert selected behavior and absence of a guessed fallback.
  • AC6, AC7: inject index, blob, base-configuration and private-list failures, including output before failure; compare status and streams and assert no partial clean result or disclosed value, path or expression.
  • AC8: compare index entries, refs and working-file bytes before and after success and failure.
  • AC12: inspect the fixture temporary area for retained matched content after success and every failure class.
  • AC9: exercise the real entry point with absent binary, absent capability, supported clean/finding/error results and output-then-error; assert preserved arguments and streams, single execution and zero fallback after supported failure. Probe errors refuse without consuming stdin.
  • AC10: native platform matrix and unchanged N-2 consumer fixtures; shell parity where Bash exists. Record the same 100-changed-file staged fixture over 20 warm process runs for shell and Go, with build time excluded, and investigate regressions.
  • AC11: run the published staged hook and task with explicit arguments, with no-argument DEVTOOLS_LEAK_MODE=staged, and with an explicit range overriding task default selection; compare IDs, stages and task argument boundaries. Check the adoption text against these results.
  • Run the branch binary through mise run test-leaks and affected commit-hook regressions, then mise run check, fresh coverage and both coverage-floor tasks. Confirm replacement protection and registration coverage from build(agents): protect replacement Go enforcement paths #753. If skill sources change, regenerate, measure tokens and deploy in the documented order, then run mise run check-generated-skills.
  • Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches, or the description says No doc impact with the reason. D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label. D8 the change stays within the pull request size limits. D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work links its issue. D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a final Status: comment names the merge commit.

Risks

Confusing indexed bytes with checkout bytes can miss a staged literal; treating pattern scans like literal budgets can weaken the gate. Inherited values are intentionally tolerated only as the current comparison defines. Undo: revert this PR to restore the staged shell path and callers.

Operator authority

standing rule names the operator: activate replacement enforcement only after #753 and satisfy its exact-head owner-review requirements; this port grants no gate-loosening authority.

Activity

  1. tbhb-agent
    added
    change:featPR 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

  6. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    tbhb-agent
    on Oct 26, 2026
    ContributorAuthor

    Refinement: READY — staged leak comparison contract, non-goal boundaries, #753 prerequisite, and verification coverage are complete
    Model: gemini-4-argon-high

    Checked

    • Verified in an independent clone that origin identifies tbhb-dev/devtools and origin/main resolves to a0347a715fab580ece45edfcb3d13509620459a8. Read live issue #816 (open, Feature, label change:feat, zero comments) and the issues it names (#588, #329, #501, #551, #753, #661, #333), plus #573 and #277 referenced by the parent/slice context.
    • Problem: Clear and accurate against origin/main (a0347a715fab580ece45edfcb3d13509620459a8). Staged literal budget accounting lives at .devtools/scripts/check-leaks.sh:189 (.devtools/scripts/check-leaks.sh:195-202), staged index-versus-base blob emission lives at .devtools/scripts/check-leaks.sh:306 (.devtools/scripts/check-leaks.sh:310-338), and base-tree configuration loading for .leak-allow and .leak-patterns lives at .devtools/scripts/check-leaks.sh:779 (.devtools/scripts/check-leaks.sh:779-812).
    • Acceptance criteria (AC1–AC12): Each AC is observable, testable, scoped to one outcome, and free of prescribed package layout, file structure, or library choices.
      • AC1–AC3 (staged selection, literal budgets, and entry modes): Match .devtools/scripts/check-leaks.sh:189-203, .devtools/scripts/check-leaks.sh:269-287, and .devtools/scripts/check-leaks.sh:306-338, including --ignore-submodules=none, --diff-filter=ACMRTU, rename/copy base content attribution to destination labels (.devtools/scripts/check-leaks.sh:324-329), symlink blob contents, binary NUL detection (.devtools/scripts/check-leaks.sh:260-273), and 160000 gitlink path checks (.devtools/scripts/check-leaks.sh:324,332).
      • AC4 (base configuration isolation and skip exclusion): Matches .devtools/scripts/check-leaks.sh:204-212, .devtools/scripts/check-leaks.sh:764 (leak-skip.sh loaded only when mode == tree), and .devtools/scripts/check-leaks.sh:796-812 (.leak-allow and .leak-patterns read from $base_tree blobs).
      • AC5 (base tree selection): Matches .devtools/scripts/check-leaks.sh:784-795 (DEVTOOLS_LEAK_BASE^{tree} when set, otherwise HEAD^{tree}, or the empty tree only when HEAD is a symbolic ref whose target ref does not yet exist).
      • AC6–AC7 (fail-closed status 2 and output/redaction contract): Match .devtools/scripts/check-leaks.sh:75-92, .devtools/scripts/check-leaks.sh:146,152,171-180,215-221,242-245, .devtools/scripts/check-leaks.sh:314, .devtools/scripts/check-leaks.sh:801, and .devtools/scripts/check-leaks.sh:830-840,873-876.
      • AC8–AC12 (non-functional and caller contracts): Cover index/ref/working-tree preservation (AC8), thin-wrapper probing without writes or stdin consumption and N-2 fallback only on absent binary/capability (AC9, matching the pattern at packages/agents/source/skills/rebase/shared/scripts/marker-scan.sh:6-12 and docs/compatibility.md:1-35), Linux/macOS/Windows support and N-2 consumer compatibility (AC10), task mode dispatch in .devtools/tasks/leaks.toml:6-13 and the leaks-staged hook in .pre-commit-hooks.yaml:126-133 (AC11), and temporary file cleanup after completion or failure (AC12, .devtools/scripts/check-leaks.sh:81-93).
    • Scope, non-goals, and #501 / #551 (#573) boundaries: All cited Bats line references in .devtools/scripts/check-leaks.bats:226, .devtools/scripts/check-leaks.bats:279, .devtools/scripts/check-leaks.bats:358, .devtools/scripts/check-leaks.bats:404, .devtools/scripts/check-leaks.bats:516, .devtools/scripts/check-leaks.bats:535, .devtools/scripts/check-leaks.bats:627, and .devtools/scripts/check-leaks.bats:1578-1744 match origin/main exactly. #501's new worktree/machine private-string lookup, precedence, source notices, and provisioning remain non-goals owned by #501. #551's (and open PR #573's) devtools:gitleaks --mode local/--mode ci split, full-index gitleaks snapshot scan, strict origin base resolver, caller rewiring, and owner-approved local history coverage reduction remain non-goals owned by #551. Publication (--publish) and whole-object (--range, --pre-push, --pre-push-hook) modes stay out of scope.
    • Dependencies and #277 check:
      • #753 (open, refinement:ready) is properly listed as a blocker before replacement enforcement is activated, with merged protection evidence required at merge, satisfying #753 AC3.
      • #588 (open) and #329 (open) track the overall shell-to-Go migration and leak/pre-push port.
      • #661 (closed) supplies the N-2 compatibility policy (docs/compatibility.md:1-35), and #333 (open, refinement:ready) is accurately classified as binary release distribution rather than a binary floor increase for this slice.
      • Checked tbhb-dev/devtools#277 (referenced by #329 for .devtools/scripts/install-pre-push.sh): #277 is open and not yet Ready (no refinement:ready label; its Round 1 review returned NOT READY). Because #277 concerns shared-worktree pre-push wrapper installation in .devtools/scripts/install-pre-push.sh:1-119 and #816 is strictly the --staged comparison slice of .devtools/scripts/check-leaks.sh, #277 does not block #816.
    • Verify plan: Maps every acceptance criterion (AC1 through AC12) to concrete behavioral checks that prove the criterion, includes the warm 20-run 100-changed-file staged benchmark for AC10, names existing repository tasks (mise run test-leaks at mise.toml:584, mise run check at mise.toml:792, mise run cover-go-check at mise.toml:685, mise run cover-py-check at mise.toml:707, and conditional skill generation culminating in mise run check-generated-skills at mise.toml:462-480), verifies #753 protection and dispatcher registration coverage (internal/agentrepo/command.go:85), and states D1–D10 completion obligations.
    • Risks and Operator authority: Risks identify index-versus-checkout confusion and pattern-versus-literal budget differences, with a clear revert undo path. Operator authority accurately states that replacement enforcement activates only after #753 and must satisfy #753's exact-head owner-review requirements (covering .agents/agent-skills.conf:4,8,9, internal/agentrepo/command.go:85, and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:528-534,576-590), while granting no gate-loosening authority.

    Gaps

    None.

    Split

    Size M as a single pull request for the --staged comparison slice and its thin-wrapper/caller compatibility is well-bounded. Publication (--publish), whole-object/range/pre-push modes, and install-pre-push.sh remain separate slices under #329 and #588. No further split is needed.

    Definition of done

    The definition of done for #816 (Feature). Tick each item and give its evidence.

    A ticked box means planning coverage in the issue's acceptance criteria and verification plan, 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.
      Evidence: ## Verify plan maps AC1–AC12 to concrete checks and explicitly requires under Definition of done that D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
      Evidence: ## Verify plan requires D2 an independent review approves the exact merged head, and every finding has a recorded disposition, paired with D9 for filing deferred review findings as follow-up issues.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
      Evidence: ## Verify plan requires D3 required checks pass with no gate loosened, and ## Operator authority states this port grants no gate-loosening authority and requires satisfying #753's exact-head owner-review requirements.
    • 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.
      Evidence: ## Verify plan requires D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted, which exercises devtools:lint-leaks and devtools:gitleaks (packages/agents/source/skills/pr/shared/scripts/create-pr.sh:189-208) with exported *_PRIVATE_STRINGS (.devtools/scripts/check-leaks.sh:114-127).
    • D5 The author signed each commit, the squash included.
      Evidence: ## Verify plan requires D5 every commit is signed.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.
      Evidence: ## Verify plan requires checking adoption text under AC11 and states D6 the documentation matches, or the description says No doc impact with the reason.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
      Evidence: ## Verify plan requires D7 every commit carries an Assisted-by trailer and the pull request carries 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.
      Evidence: ## Size scopes the work to M. One pull request, and ## Verify plan requires D8 the change stays within the pull request 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.
      Evidence: ## Scope and non-goals notes Fallback retirement is a separate N-2 follow-up under D9, and ## Verify plan requires D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work 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.
      Evidence: ## Verify plan requires D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a final Status: comment names the merge commit.

    Round: 1 of 3

  7. tbhb-agent
    added
    refinement:readyPassed the Definition of Ready; may be picked up
    on Oct 26, 2026
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

    Type

    Feature

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • devtools
      Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions