Repository navigation
test(agents): port the content-post bypass scanner to Go #757
Description
Problem
tests/skills/test_content_posts.py (126 lines) parses Bash syntax trees to find skill scripts that post content to GitHub without gh-post. It enforces the posting rule, but it is a Python helper with a tree-sitter dependency that sits outside the Go epic.
Part of #588.
Acceptance criteria
- AC1: Given a skill script containing a content post that bypasses
gh-post(a multiline command, an environment-prefixed command, a command behindcommandorenv, or anapiwrite to a comments or reviews endpoint), when the Go scanner runs, then it exits with status 1, prints onefile:linefinding per line on standard output, and prints nothing on standard error. Given input with several findings, it lists every file and line. Given a clean script, it exits 0 with no output on either stream. Given a file it cannot read, it exits 2 with a diagnostic on standard error naming the file. Given a file it cannot parse, it exits 2 with a separate diagnostic naming the file and the parse error, and neither case ever reports clean. Scanning leaves every source file unmodified. - AC2: Every command in the Python test's detection list and permit list has a Go case with the same expected result, and the source-wide scan case has a Go equivalent.
- AC3: The set of files scanned is at least the set the Python test scans today (every
*.shunderpackages/agents/source), and a fixture proves a script in that tree is still scanned. - AC4: Rollout order is provable: this issue is blocked by build(agents): protect replacement Go enforcement paths #753, and its pull request cites the merged build(agents): protect replacement Go enforcement paths #753 commit. The Python test stays in the gates until a follow-up issue, filed when this lands, covers retiring it without an uninterrupted-gate gap.
Scope and non-goals
Scope: a Go scanner with the same rule set and the same scanned paths. The Python test stays until the Go scanner runs in the same gates.
Non-goal: widening the rule set or the scanned paths. Non-goal: removing the Python test, which a follow-up issue filed under D9 owns.
Approach
Meet AC1 to AC4. Reuse the rule set unchanged.
Dependencies
Blocked by #753, because the scanner is enforcement and its new package needs owner-protected paths first. Part of #588. The Python test's retirement belongs to a follow-up issue filed when this lands.
Size
S. One pull request. Touches a gate path.
Verify plan
- AC1: fixtures for each bypass form, a multi-finding input, a clean script, a separate unreadable file and a separate unparsable file; assert status, both streams, every file and line, empty output on clean, and checksums of the sources unchanged after the scan.
- AC2: a table that lists the Python test's commands next to the Go cases, plus a run of both on the same inputs comparing results.
- AC3: list the scanned paths from both implementations on the same tree and show the Go set contains the Python set; add a fixture script under the source tree and show it is reported.
- AC4: the pull request description cites the merged build(agents): protect replacement Go enforcement paths #753 commit and the follow-up issue for the Python test, and the gates still run the Python test in the pull request.
- 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 impactwith the reason. D7 every commit carries anAssisted-bytrailer and the pull request carries theprovenance:agent-assistedlabel. 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 finalStatus:comment names the merge commit.
Risks
A looser port would let a content post bypass gh-post. AC1 to AC3 pin the rules and the paths. Undo: revert the change, which leaves the Python test running.
Operator authority
A standing rule names the operator: the scanner is enforcement and its paths are owner-protected, so the pull request needs the owner's exact-head review.
Activity
tbhb-agent commentedon Oct 26, 2026
Refinement: NOT READY — enforcement parity, verification, and authority need clarification
Model: gpt-6.1-sol
Checked
tbhb-dev/devtools. Checkedorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c.tests/skills/test_content_posts.py:13,tests/skills/test_content_posts.py:75, andtests/skills/test_content_posts.py:118.Gaps
Medium — Correct the baseline and rollback description. The Problem says the helper walks Python syntax trees, but it parses Bash syntax trees (
tests/skills/test_content_posts.py:55). The Approach and Undo describe an existing shell fallback, whereas this scanner is a pytest test (tests/skills/test_content_posts.py:118). Correct the baseline, identify any actual compatibility obligation, and describe rollback as restoring the existing Python enforcement unless a different existing caller is identified.High — State observable enforcement outcomes. AC1–AC3 do not establish what the replacement gate reports or returns for clean input, detected bypasses, or scan failures. “Same rule set” and “refuses” leave that contract incomplete. State the expected success/failure status, diagnostic content and stream, and absence of writes. Include the already-stated requirement that a supported command’s failure or partial output cannot become success through fallback. Preserve both bypass rejection and permitted-command acceptance; the existing negative cases matter (
tests/skills/test_content_posts.py:102).High — Make the verification prove AC2 and AC3. Reusing AC1’s planted bypass cannot prove every existing fixture has an equivalent case, or that the scanned path set has not shrunk. AC2 needs an explicit comparison covering both fixture groups (
tests/skills/test_content_posts.py:75,tests/skills/test_content_posts.py:102). AC3 needs a path-set check and evidence that violations in each required source/output tree reach the gate. The existing boundary is recursive*.shfiles beneathpackages/agents/source(tests/skills/test_content_posts.py:10,tests/skills/test_content_posts.py:119). Also verify the stated gate cutover before deleting the Python test.High — Identify and classify the dependencies. “Replacement-protection issue (P2)” is not an identifiable issue reference; “a separate XS issue” likewise leaves fallback removal untraceable. Name those issues and distinguish an implementation blocker from a rollout prerequisite or deferred non-goal. Parent Replace complex shell with tested Go, starting with gates and merging #588 requires replacement protection before operational rollout and binary availability before callers require new commands; clarify which conditions actually block this PR.
High — Replace “Operator authority: None” with the applicable owner-review requirements. Parent Replace complex shell with tested Go, starting with gates and merging #588 explicitly requires existing protections to carry to replacement enforcement. Repository policy protects published payload and gate definitions, and requires exact-head owner approval for golden changes or detected gate loosening (
AGENTS.md:66,AGENTS.md:68). Name the protected paths this scope expects to touch and the required approval conditions, including replacement protection before rollout. This records required authority without granting implementation or publication authority.Medium — Remove prescribed design outside Operator authority. “Behind an
agentreposubcommand,” the shell probing mechanism, and mandatory skill regeneration prescribe implementation choices. Keep requirements such as compatibility, fail-closed behavior, gate continuity, and generated-output consistency where applicable; leave command placement and caller mechanics to the PR. No decision in test(agents): port the content-post bypass scanner to Go #757’s Operator authority section currently makes those design choices given.Split
No additional split is justified by the scanner’s size. Keep this as one focused migration after clarifying its boundaries. Reference replacement protection and eventual fallback retirement as separate dependencies or deferred work.
Definition of done
The definition of done for #757 (Feature). Tick each item and give its evidence.
Boxes below assess planning coverage in the live issue and named parent, 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: The Verify plan names AC IDs but does not require merged-head verification or PR evidence.
D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
Evidence: Neither issue explicitly covers this complete requirement.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: Parent AC8 requires checks and owner protection, but test(agents): port the content-post bypass scanner to Go #757 does not establish the full exact-head gate-loosening requirement.
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: Neither issue explicitly covers all three scans.
D5 The author signed each commit, the squash included.
Evidence: Neither issue explicitly requires this delivery evidence.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: Neither issue specifies documentation reconciliation or a justified no-impact statement.
D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: Neither issue explicitly covers these delivery requirements.
D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: “S. One pull request” estimates scope but does not cover size-limit enforcement or the exception.
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: Deferred fallback removal is mentioned without an issue reference or this follow-up contract.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: Neither issue specifies these closure steps.
Round: 1 of 3
tbhb-agent commentedon Oct 26, 2026
Refinement: NOT READY — retirement dependency and verification remain incomplete
Model: gpt-6.1-sol
Checked
Read live #757 and its comment once, plus named issues #588, #753 and #772 and their comments. Verified the independent clone’s origin identifies
tbhb-dev/devtools. Repository evidence describesorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c.tests/skills/test_content_posts.py:55,tests/skills/test_content_posts.py:75,tests/skills/test_content_posts.py:102andtests/skills/test_content_posts.py:118..agents/agent-skills.conf:4,.agents/agent-skills.conf:9). Operator authority correctly requires exact-head owner review.Gaps
High — refactor(agents): remove the shell fallback for discovery and worktree readiness #772 does not own this scanner’s retirement. AC4, Scope and Dependencies assign Python-test removal and gate replacement to refactor(agents): remove the shell fallback for discovery and worktree readiness #772. That issue covers discovery and worktree-readiness shell fallback removal for refactor(agents): port skill capability and worktree context discovery to Go #755 and explicitly excludes fallbacks owned by other ports. Correct these references to a retirement issue whose requirements cover this Python scanner and uninterrupted gate enforcement, or keep retirement deferred without claiming refactor(agents): remove the shell fallback for discovery and worktree readiness #772 owns it. Verify the corrected rollout condition as well as the merged build(agents): protect replacement Go enforcement paths #753 citation.
Medium — Complete observable parity and failure verification. AC1 leaves the diagnostic stream unspecified. State which stream carries findings and scan errors, and preserve scanning without modifying source files; the current scan reads files and reports findings (
tests/skills/test_content_posts.py:121). The Verify plan’s “unreadable or unparsable” fixture can prove only one failure class, although AC1 requires both. Require separate read-error and parse-error checks, plus a multiple-finding input proving every expected file and line is reported. Assert the specified streams and statuses.Medium — Complete the delivery contract. The new Definition of done paragraph substantially improves coverage, but still omits parts of D4, D6, D7, D9 and D10 below. Explicitly cover the named leak checks and each post’s leak review; the reason for “No doc impact”; Assisted-by trailers and the exact provenance label; follow-up contents and links; and closing-reference, board and final-status evidence. These are delivery requirements, not implementation design.
Split
One scanner PR remains appropriate. Keep gate replacement and Python-test removal deferred to correctly identified follow-up work. No additional implementation split is needed.
Definition of done
The definition of done for #757 (Feature). Tick each item and give its evidence.
Ticks indicate planning coverage, 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’s completion paragraph explicitly requires merged-head verification and commands/results cited by AC ID in the PR’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: The completion paragraph requires exact merged-head independent approval and a recorded disposition for every finding.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: The completion paragraph requires passing checks and prohibits gate loosening; Operator authority additionally requires exact-head owner review.
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: Generic repository leak scans are required, but the complete named checks and review of each post are not explicit.
D5 The author signed each commit, the squash included.
Evidence: The completion paragraph requires every commit to be signed, encompassing the squash.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: Documentation matching or a no-impact statement is required; the alternative does not require an explanation in Verification.
D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: “Provenance trailers and the provenance label” does not identify the required trailer and label explicitly.
D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: The completion paragraph requires compliance with PR 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: Filing follow-up issues is required, but their minimum contents, Related references and deferred-code links are unspecified.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: Merge closure and the final
Status:comment naming the merge commit are required; the closing reference and board state are unspecified.Round: 2 of 3
tbhb-agent commentedon Oct 26, 2026
Refinement: READY — observable parity, verification, and rollout requirements are covered
Model: gpt-6.1-sol
Checked
Read live #757 and its two comments, plus named issues #588 and #753. Verified the independent clone’s origin identifies
tbhb-dev/devtools. Repository evidence refers toorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c.tests/skills/test_content_posts.py:55).tests/skills/test_content_posts.py:75,tests/skills/test_content_posts.py:102,tests/skills/test_content_posts.py:118). The comparison table and shared-input runs establish parity.packages/agents/source(tests/skills/test_content_posts.py:10,tests/skills/test_content_posts.py:119). Path-set comparison and a planted source-tree fixture prove coverage..agents/agent-skills.conf:4,.agents/agent-skills.conf:9, andAGENTS.md:68.Gaps
None.
Split
One S-sized PR is reasonable. Keep Python-test retirement in the required follow-up issue.
Definition of done
The definition of done for #757 (Feature). Tick each item and give its evidence.
Ticks indicate planning coverage, 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’s D1 explicitly requires merged-head verification and AC-indexed commands/results in the PR’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: D2 requires exact merged-head independent approval and recorded disposition of every finding; D9 covers deferred findings.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: D3 requires passing checks and prohibits gate loosening; Operator authority requires exact-head owner review.
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: D4 requires repository-specified scans and per-post review. Publication invokes both leak gates at
packages/agents/source/skills/pr/shared/scripts/create-pr.sh:189; private-string handling appears at.devtools/scripts/check-leaks.sh:114.D5 The author signed each commit, the squash included.
Evidence: D5 requires every commit to be signed, encompassing the squash.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: D6 requires matching documentation or a reasoned
No doc impactstatement in the PR description.D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: D7 explicitly requires the trailer on every commit and the provenance label.
D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: D8 requires compliance with PR 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: D9 explicitly covers issue contents, deferred findings, Related references, and deferred-code links. AC4 also requires the retirement follow-up.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: D10 explicitly requires all three closure artifacts.
Round: 3 of 3