Skip to content

Route suppression-check entry points through the shared launcher #891

Description

@tbhb-agent

Problem

Slice 06 of #327 connects the complete Go suppression modes to existing script entries. The original at packages/agents/source/skills/fix-prose/shared/scripts/check-suppressions.sh:56 is 263 scc code lines. State, findings and sanctions must already be complete before it changes.

Acceptance criteria

  1. AC1: Each generated suppression entry preserves the caller's arguments when invoking the supported Go modes.
  2. AC2: The switched entries reproduce the executed oracle's statuses, both streams and state effects for all five modes.
  3. AC3: An existing workflow can continue across the switch in either direction.
  4. AC4: An absent or outdated binary produces the shared launcher's one-line upgrade/install failure and nonzero status without automatic fallback.
  5. AC5: Exactly one byte-identical original remains directly usable through the documented compatibility arrangement.
  6. AC6: Non-functional: forwarding adds no shell decision logic.
  7. AC7: Non-functional: canonical generation reproduces all committed deployment changes.
  8. AC8: The shared dictionary-error case has executing replacement coverage before it stops extracting the original function.
  9. AC9: Non-functional: no owner-tier path changes occur.
  10. AC10: Missing required arguments, an unknown mode, or an invalid sanction request produces the original's diagnostic on stderr, exits 2 and leaves workflow evidence unchanged.
  11. AC11: A state outside the accepted roots, a missing or empty state target, or a mismatched supplied target produces the original's diagnostic on stderr, exits 2 and leaves workflow evidence unchanged.
  12. AC12: Given valid state with no saved original target copy, --diff exits 2 with the original's missing-copy diagnostic on stderr and no stdout.

Scope and non-goals

Use #321's permanent shared launcher for the existing suppression entry paths. This child already explicitly authorizes retaining one frozen original; this is the sole relocation exception here. Non-goal: a second resolver, capability probe, fallback, report rollout, hook changes, test retirement or original removal. No new shell logic. Canonical sources generate the packages and deployments; generated copies are never edited by hand. No edits to shared libraries, guard scripts, merge scripts, ownership rules, workflow files or harness configuration.

Out of scope: preflight's outside-repository, missing-target, lint-status and Vale coverage/probe outcomes belong to #889; writer-context's state/target refusals, command parsing, numbered document, partial lint output, Vale absence/error/control findings and replacement-count reports belong to #890. Writer-context notes precedence also belongs to #890: nonempty run notes win; when run notes are absent or empty, the shell falls back to nonempty legacy notes; when both are absent or empty it prints no notes section. #892 owns switching those report entries. This suppression-entry rollout changes none of those report behaviors and adds no guards or knobs.

Approach

Keep flags and mode parsing in Go. Implement only the constant forwarding shape specified by #321. Preserve the single original byte for byte and document direct invocation for consumers within docs/compatibility.md:3 and docs/compatibility.md:5. Keep other frozen files in place.

Use #886–#888's executed records as the baseline: state creation/duplicate refusal; target-only success; mismatched/invalid state refusal; suppression, deletion and dictionary findings; exact sanctions; diff suffix; scoped cleanup; comparison failure returning 2. Extend the executed oracle records for the argument, state and missing-copy cases below before comparing replacements; source inspection alone is not an executed oracle. Replay every record through both generated entries and compare resulting artifacts. These future switched-entry results are not claimed as already executed.

Before switching, #321 must adapt or retire tests/skills/source/lib/masked-status.bats:205 after #888 supplies executing failure coverage; a function-not-found result must never satisfy that test.

Dependencies

Blocked on: #886
Blocked on: #887
Blocked on: #888
Blocked on: #321
Blocked on: #333
Blocked on: #544
Blocked on: #753

These block this rollout's merge: complete commands, the shared launcher and adapted shared failure case, a verified release containing the commands, installed consumer binaries, and golden protection for the verifier plus command registration. #753 owns .agents/agent-skills.conf edits; none belongs here.

Keep these seven dependencies in prose; native blocked-by links wait on #655. No native links are claimed as recorded by this draft.

Before rollout, inspect #753's merged protection patterns against the actual replacement prose-verifier paths and the Go command-registration paths delivered by #886–#888. Record each path and its matching pattern in the PR's Verification section. Both must be protected at the rollout base; #753 being closed or merged alone is insufficient. Any missing protection stays with #753 and blocks this rollout; do not repair ownership rules in this child.

Size

M. Measured existing-body replacement plus the one explicitly authorized frozen copy: 263 + 263 = 526 code lines before forwarding and integration changes. Counts use scc 4.1.0; generated package/deployment output and token metadata are excluded, while source, tests, fixtures and frozen copies count. One PR with fewer than 800 changed code lines and at most 2,000 prose lines. No unbuilt Go size is claimed: measure the actual staged diff before publication and split again if the cap is exceeded.

Verify plan

The named fixtures below are required planned cases, not claims of existing tests or passing results. Run go test ./internal/agentrepo -run '^TestProseEntry' -count=1 -v for the entry fixtures. Every behavioral fixture runs the executed frozen oracle and each generated harness entry from the same initial repository/state, captures exit status, stdout and stderr separately, and compares all resulting state and repository artifacts. Normalize only temporary roots and diff timestamps. Refusal cases assert unchanged evidence, including no appended sanction, new baseline or cleanup. Include real assertions so a missing test or skipped case cannot count as success.

  • AC1: TestProseEntry/argv uses a recording supported binary behind the unchanged shared launcher. Invoke each entry with all five supported modes and vectors containing a space-bearing target/state/path, an empty argument, a leading dash, literal glob characters and a sanction line with spaces. Assert the recorded Go-mode prefix is exactly Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321's mapping and every caller argument follows byte for byte in its original position, with empty arguments retained and no shell expansion or splitting.
  • AC2: TestProseEntry/oracle-modes replays every Record, read and clean up prose workflow state in Go #886–Apply exact dictionary sanctions in Go #888 record. Baseline fixtures cover a present target (status 0, exact state line and baseline artifacts), a nonexistent target (status 0, state without an original copy) and duplicate canonical targets (status 2, cleanup guidance, first state untouched). Verify fixtures cover target-only edits and preexisting directives (silent status 0), a new suppression comment, modified/added/deleted non-target files, guarded configuration including ignored Vale styles, and dictionary changes (status 1, exact findings and TOTAL line). Sanction fixtures cover each allowed dictionary kind, exact added lines (silent status 0), an extra addition or removal (status 1 and exact dictionary finding), and comparison failure (status 2, never a clean result). Diff fixtures use identical target bytes (silent status 0), changed target bytes (status 0 and exact unified diff), and a nonempty allow file (status 0 and exact sanctioned-by-the-operator suffix); absent or empty allow files produce no suffix. Cleanup fixtures create two runs and require status 0 with only the named run removed. Compare both streams and complete artifacts for every fixture.
  • AC3: TestProseEntry/mixed-state creates a baseline with the frozen shell, continues allow/verify/diff with each generated entry, and cleans it with the shell; repeat with baseline creation through each generated entry and continuation through the shell. Run separate fixtures with worktree-relative .fix-prose/run.*, absolute worktree state and legacy Git-directory state. Matching absolute/relative target spellings must refer to the same document; omitted optional targets on verify/diff must still work. Assert successful exact sanctions, expected diff bytes and scoped cleanup, with preserved state readable at each handoff.
  • AC4: TestProseEntry/launcher-failure separately supplies no binary and a binary below Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321's supported version. For each generated entry and each mode, assert exactly the shared launcher's one-line upgrade/install diagnostic on its specified stream, empty other stream and its specified nonzero status. Put a recording frozen-shell stand-in at the compatibility path; assert it was never invoked and repository/state bytes did not change. Do not add a local version probe or fallback to obtain this result.
  • AC5: TestProseEntry/frozen-original compares the retained compatibility file byte for byte with the pinned pre-switch original using cmp; record that source commit and blob. Inventory maintained and generated/deployed paths by content and require exactly one full original copy, at the documented compatibility location; each old public entry must contain only forwarding content. Directly invoke that retained file on the baseline/verify/diff/allow/cleanup fixtures and require oracle-equivalent outputs, statuses and state. Check the documented invocation resolves to this same executable file.
  • AC6: git diff origin/main...HEAD -- packages/agents/source/skills/fix-prose/shared/scripts/check-suppressions.sh plus TestProseEntry/forwarding-source reviews each entry's source and generated content against Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321's exact constant forwarding shape. The source fixture asserts only the authorized launcher invocation/mode mapping and argument forwarding remain, with no added shell branches, parsing, resolver, probe, fallback or other decisions. Review the complete branch diff for a relocated shell decision implementation outside the one frozen original; reject one. Executing entry tests alone does not prove this criterion.
  • AC7: mise run check-generated-skills and apm audit --ci use the changed canonical forwarding sources as the fixture. After mise run generate-skills, documented deployment and token/lock refresh, require the regenerated files to equal the committed packages/deployments with no residual differences and a clean audit.
  • AC8: mise run test-hooks tests/skills/source/lib plus go test ./internal/agentrepo -run '^TestProseSanction' -count=1 -v executes a valid dictionary baseline and sanctioned path with a controlled comparison failure. Require status 2 from the actual replacement comparison, with both streams matching the oracle; a missing extracted function, unregistered command or skipped test fails the fixture. Inspect Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321's merged adaptation/retirement of tests/skills/source/lib/masked-status.bats:205 and attach that disposition and Apply exact dictionary sanctions in Go #888's executing replacement evidence before switching.
  • AC9: git diff --name-status origin/main...HEAD uses the complete rollout diff, including additions, deletions and renames, as the path fixture. Classify every old/new path against the base's .agents/agent-skills.conf owner-tier patterns and require zero matching edits. Separately match the actual verifier and command-registration paths against build(agents): protect replacement Go enforcement paths #753's merged golden protection as required above, recording each match; missing protection fails rollout readiness without adding a protected-path edit here.
  • AC10: TestProseEntry/argument-refusals has separate input fixtures for no mode, an unknown mode, --baseline with no target, and valid-state --allow with no dictionary path, no line, an empty line, a non-sanctionable path, a tab-bearing line or a newline-bearing line. Each must exit 2, have empty stdout, match the executed original's exact stderr diagnostic, and leave existing evidence unchanged. For no/unknown mode require the usage diagnostic; for missing baseline target require its target-path diagnostic; for missing allow operands, unsanctionable paths and malformed lines require their respective diagnostics. State validation precedes allow validation, so use valid state in every allow-operand fixture.
  • AC11: TestProseEntry/state-refusals invokes verify/diff/allow/cleanup separately with a missing state argument, a state outside both accepted roots, and an accepted-root state with its target file separately absent and empty. Each must exit 2 with empty stdout and the oracle's invalid-state or missing-target stderr diagnostic, with no evidence changed or removed. Additional valid-state verify/diff fixtures supply a different target; require status 2, empty stdout and the exact belongs-to diagnostic, preserving the run. Missing optional verify/diff targets are covered as successes under AC3, not refusals.
  • AC12: TestProseEntry/diff-no-original first creates valid state for a nonexistent target, then creates that target without adding a saved original copy. --diff must exit 2 with empty stdout and the exact baseline-kept-no-copy diagnostic on stderr, leaving state untouched; this is distinct from AC11's missing state-target file.

At the merged head, record every AC ID, its fixture, command and result in the PR's Verification section, with native platform execution and any skips. Required checks must pass without weakening gates. Obtain independent review of the exact head and record finding dispositions. Sign every commit, include Assisted-by trailers and apply the provenance:agent-assisted label.

Verify the compatibility documentation against the retained file, direct invocation and launcher's supported-version behavior; record the documentation check in the PR's Verification section. Complete required gitleaks and check-leaks.sh scans with the configured private-strings lists without exposing their contents, and complete the required leak review of each later post before publication. File and list follow-up issues; if none are needed, record that explicitly. Any unmet AC requires a cited follow-up and removal of the closing reference. Otherwise merge with a reference closing #891, update the board to Done and post a final Status comment naming the merge commit. These are completion requirements for later authorized work; this draft authorizes no publication.

Risks

Switching before distribution can strand checks; incompatible evidence can strand an active workflow. Undo: revert this rollout and regenerate to restore the original entry body, preserving all active evidence.

Operator authority

None

Activity

  1. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — Route suppression-check entry points through the shared launcher
    Model: gpt-6-astra

    Child 06 Route suppression-check entry points through the shared launcher

    Verdict: READY

    Checked: Both numbered round 1 gaps are closed:

    • AC10 and Dependencies require Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321’s retirement or adaptation of the dictionary-error case after 03’s replacement coverage and before this switch. Verification explicitly rejects a function-not-found false pass, addressing tests/skills/source/lib/masked-status.bats:18, tests/skills/source/lib/masked-status.bats:87 and tests/skills/source/lib/masked-status.bats:205.
    • AC4 and AC5 freeze the invocation contract: missing or outdated binaries fail with the launcher’s single guidance line; exactly one frozen copy remains available for documented direct invocation. Verification exercises that route.

    The protection gap is also closed at the requirements level: rollout requires merged protection naming both the prose verifier and command registration, with an explicit hold if coverage is absent. The 526-line relocation baseline and 720-code-line estimate remain correct.

    Gaps: None.

    Split: None required.

    Definition of done

    The definition of done for this issue (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.

    Round: 2 of 3

  2. tbhb-agent commented on Nov 1, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — Suppression-entry rollout is verifiable and scoped

    Checked

    Gaps

    None.

    Split

    None indicated.

    Definition of done

    Planning coverage only; checked boxes show the draft plans for these items, not completed work.

    • D1 Record each AC’s fixture, command, and result in the PR’s Verification section.
    • D2 Obtain independent review of the exact head and record finding dispositions.
    • D3 Pass required checks without weakening gates.
    • D4 Complete required leak scans and per-post leak reviews.
    • D5 Sign every commit.
    • D6 Verify documentation against the change.
    • D7 Include Assisted-by trailers and the provenance:agent-assisted label.
    • D8 Stay within the size limits.
    • D9 File and list follow-up issues.
    • D10 Close through a merge reference, update the board, and name the merge commit in the final status comment.

    Round: 2 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

    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