From 7d9e906399cf6fc3806df4483de47f0ad57ec1c6 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:04:02 -0400 Subject: [PATCH 1/2] build(agents): protect replacement Go enforcement paths Enforcement ports need owner review before callers start using them. Keep the shell protection intact while giving the replacement packages and command construction the same golden-file policy. Preserve the existing waiver and exclude helpers without an enforcement role. Refs #753 Assisted-by: codex:gpt-6.1-sol Signed-off-by: Tony Burns --- .agents/agent-skills.conf | 4 +- docs/agent-skills-migration.md | 4 + .../skills/agent-merge-pr/go-protection.bats | 121 ++++++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tests/skills/agent-merge-pr/go-protection.bats diff --git a/.agents/agent-skills.conf b/.agents/agent-skills.conf index 14737e0a..5e3e3f6a 100644 --- a/.agents/agent-skills.conf +++ b/.agents/agent-skills.conf @@ -1,11 +1,11 @@ # cspell:words noqa nosec owner_review_label = merge:owner-review # Protect merge enforcement, its policy, shared libraries, and skill guards. -golden_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$)) +golden_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$))|^(internal/(agentrepo|checks|commitlint|config|review|signing|ghpost|mergepolicy|gates|confirmation|delegation)/|cmd/(agentrepo|commit-lint|ssh-sign|gh-post)/) constitutional_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/(rulesets/|CODEOWNERS$)|\.claude/settings\.json$|packages/agents/source/(lib|skills/merge-pr/(shared|claude|codex)/scripts)/(merge-policy\.py|merge-agent\.sh|signing\.sh|lib\.sh)$|(packages/agents/[^/]+/\.apm/skills|\.(claude|agents)/skills)/[^/]+/scripts/(merge-policy\.py|merge-agent\.sh|signing\.sh|lib\.sh)$) loosening_markers = noqa|nosec|nolint|shellcheck disable|vale (off|disable)|cspell:disable|rumdl-disable|type: ignore|pragma: no cover # Include task bodies as well as the files that configure their checkers. gate_config_paths = ^(mise\.toml$|\.config/mise/|\.devtools/(tasks|config)/|\.github/|\.(pre-commit-config\.yaml|pre-commit-hooks\.yaml|gitleaks\.toml|vale\.ini|cspell\.jsonc|rumdl\.toml|golangci\.yml|go-arch-lint\.yml|testcoverage\.yml|yamllint\.yaml|yamllintignore|editorconfig-checker\.json|commit-lint\.toml|gremlins\.yaml)$|\.vale/|\.ast-grep/|pyproject\.toml$|packages/devtools/pyproject\.toml$|cosmic-ray\.toml$|biome\.json$|tombi\.toml$|sgconfig\.yml$|\.leak-(allow|skip)$) -owner_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$)) +owner_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$))|^(internal/(agentrepo|checks|commitlint|config|review|signing|ghpost|mergepolicy|gates|confirmation|delegation)/|cmd/(agentrepo|commit-lint|ssh-sign|gh-post)/) # Keep settings, notes, dependency directories, and other worktrees. disposable_ignored = ^(bin/|dist/|coverage\.(out|html|txt|xml)$|coverage\.covdata[^/]*/|\.coverage(\.[^/]*)?$|htmlcov/|\.pytest_cache/|\.hypothesis/|\.ruff_cache/|\.complexipy_cache/|\.rumdl_cache/|\.cspell-cache$|([^/]+/)*__pycache__/) diff --git a/docs/agent-skills-migration.md b/docs/agent-skills-migration.md index 29a758a1..c16797e8 100644 --- a/docs/agent-skills-migration.md +++ b/docs/agent-skills-migration.md @@ -154,6 +154,10 @@ Pin the harness package containing the updated pin reader and run `apm install` Reports include the rule used: `owner approval` or `App review states Loosens: no`. They also record whether execution used an override and the approving login. The App approval, required checks, independent review, and write authority still apply. The owner-review label still holds a PR for the owner, and neither rule waives it or any other merge gate. +devtools also classifies replacement Go enforcement as golden, with the existing golden-only App waiver and no constitutional classification. The protected packages are `internal/agentrepo`, `internal/checks`, `internal/commitlint`, `internal/config`, `internal/review`, `internal/signing`, and `internal/ghpost`. Reserved enforcement packages are `internal/mergepolicy`, `internal/gates`, `internal/confirmation`, and `internal/delegation`. Command entry points under `cmd/agentrepo`, `cmd/commit-lint`, `cmd/ssh-sign`, and `cmd/gh-post` receive the same protection. This includes command construction and registration in `internal/agentrepo/command.go`. Formatting, build metadata, and hook I/O helpers stay outside these patterns. + +[Issue 753](https://github.com/tbhb-dev/devtools/issues/753) must merge before a caller switches to a Go enforcement replacement. Each port issue lists it under Dependencies, including ports filed later, and each port pull request cites its merged commit before landing. Replacement enforcement belongs in the protected packages. A different package boundary requires extending protection before caller activation. + The scan skips full comment lines. It recognizes exclusion keys instead of matching words inside values and ignores suppression text inside quoted values. Non-local APM deployments come from `apm.lock.yaml` at the pinned base commit. Local deployment overrides stay in scope. A missing lockfile means no deployment exemptions. Malformed ownership data refuses the scan. A PR can't grant itself an exemption by editing its lockfile. The scan makes a narrow exception for each of these, without configuration. A `mise.lock` diff that adds at least as many `checksum` lines as it removes doesn't count the removed checksums as removed gates. An inline `//nolint` in a `vendor/**/*.go` file doesn't count as a suppression when `vendor/modules.txt` exists at the base and changes in the same diff, and any other marker on that line still does. Consumers adopt both through a normal pin bump. The scan is a backstop and doesn't replace review. Required checks come from GitHub's applied branch rules and active ruleset JSON files in `ruleset_dir` at the pinned base. Requirements match names exactly. The merge requires a nonempty list of known required checks and a nonempty rollup. Skipped required checks, unknown states, and missing results refuse the merge. Check judgement uses `agentrepo checks evaluate` instead of another check judge. A configured `merge_ready_cmd` must be an executable path and receives ` `. Its nonzero status blocks, while empty readiness doesn't add a check. diff --git a/tests/skills/agent-merge-pr/go-protection.bats b/tests/skills/agent-merge-pr/go-protection.bats new file mode 100644 index 00000000..cb1f419c --- /dev/null +++ b/tests/skills/agent-merge-pr/go-protection.bats @@ -0,0 +1,121 @@ +#!/usr/bin/env bats +setup() { + source "${BATS_TEST_DIRNAME}/test_helper.bash" + agent_setup + # Install the repository policy on the base, so only the replacement changes. + git checkout --quiet main + cp "$ROOT/.agents/agent-skills.conf" .agents/agent-skills.conf + git add -- .agents/agent-skills.conf + git commit --quiet -m 'test: install repository policy' + BASE_OID=$(git rev-parse HEAD) + git checkout --quiet topic + git rebase --quiet main + base_at "$BASE_OID" +} + +replacement_policy() { + local key value config='{}' + for key in golden_paths constitutional_paths owner_paths loosening_markers gate_config_paths disposable_ignored; do + value=$(bash -c 'source "$1/lib.sh"; dt_repo_conf "$2"' _ "$SCRIPTS" "$key") + config=$(jq --arg key "$key" --arg value "$value" '.[$key]=$value' <<<"$config") + done + uv run --quiet --script "$POLICY" inspect "$BASE_OID" "$HEAD_OID" <<<"$config" +} + +assert_replacement() { + local path=$1 + change "$path" '// enforcement' + run replacement_policy + [ "$status" -eq 0 ] + jq -e --arg path "$path" '.owner_paths == [$path] and .findings == [[$path,"changes a golden file"]]' <<<"$output" + run bash "$AGENT/review.sh" 7 + [ "$status" -ne 0 ] + [[ "$output" == *needs-owner* ]] + [[ "$output" == *"changes a golden file"* ]] + approve_owner + run bash "$AGENT/review.sh" 7 + [ "$status" -eq 0 ] + pr '.reviews |= map(select(.author.login != "owner"))' + pr ".reviews[0] += {body:\"Reviewed head: $HEAD_OID\\n\\nLoosens: no\",lastEditedAt:null} | .reviews[0].author.__typename=\"Bot\"" + run bash "$AGENT/review.sh" 7 + [ "$status" -eq 0 ] +} + +@test "AC1 replacement command construction is golden" { assert_replacement internal/agentrepo/review.go; } +@test "AC1 replacement registration is golden" { assert_replacement internal/agentrepo/command.go; } +@test "AC1 replacement checks are golden" { assert_replacement internal/checks/checks.go; } +@test "AC1 replacement signing is golden" { assert_replacement internal/signing/signing.go; } +@test "AC1 replacement review is golden" { assert_replacement internal/review/stamp.go; } +@test "AC1 replacement configuration is golden" { assert_replacement internal/config/repository.go; } +@test "AC1 replacement commit enforcement is golden" { assert_replacement internal/commitlint/commit.go; } +@test "AC1 replacement posting enforcement is golden" { assert_replacement internal/ghpost/run.go; } +@test "AC1 replacement merge policy is golden" { assert_replacement internal/mergepolicy/policy.go; } +@test "AC1 replacement gates are golden" { assert_replacement internal/gates/gates.go; } +@test "AC1 replacement confirmation is golden" { assert_replacement internal/confirmation/confirm.go; } +@test "AC1 replacement delegation is golden" { assert_replacement internal/delegation/delegate.go; } +@test "AC1 agentrepo entry point is golden" { assert_replacement cmd/agentrepo/main.go; } +@test "AC1 signing entry point is golden" { assert_replacement cmd/ssh-sign/main.go; } +@test "AC1 commit entry point is golden" { assert_replacement cmd/commit-lint/main.go; } +@test "AC1 posting entry point is golden" { assert_replacement cmd/gh-post/main.go; } + +@test "AC2 replacement push invalidates owner approval" { + change internal/review/stamp.go '// first' + approve_owner + change internal/review/stamp.go '// second' + run bash "$AGENT/review.sh" 7 + [ "$status" -ne 0 ] + [[ "$output" == *needs-owner* ]] + approve_owner + run bash "$AGENT/review.sh" 7 + [ "$status" -eq 0 ] +} + +@test "AC2 replacement deletion and both rename names stay golden" { + change internal/review/stamp.go '// first' + BASE_OID=$HEAD_OID + git mv internal/review/stamp.go helper.go + git commit --quiet -m 'test: move replacement out' + HEAD_OID=$(git rev-parse HEAD) + run replacement_policy + [ "$status" -eq 0 ] + jq -e '.owner_paths == ["internal/review/stamp.go"] and .findings == [["internal/review/stamp.go","changes a golden file"]]' <<<"$output" + BASE_OID=$HEAD_OID + mkdir -p internal/review + git mv helper.go internal/review/stamp.go + git commit --quiet -m 'test: move replacement in' + HEAD_OID=$(git rev-parse HEAD) + run replacement_policy + [ "$status" -eq 0 ] + jq -e '.owner_paths == ["internal/review/stamp.go"] and .findings == [["internal/review/stamp.go","changes a golden file"]]' <<<"$output" + BASE_OID=$HEAD_OID + git rm --quiet internal/review/stamp.go + git commit --quiet -m 'test: delete replacement' + HEAD_OID=$(git rev-parse HEAD) + run replacement_policy + [ "$status" -eq 0 ] + jq -e '.owner_paths == ["internal/review/stamp.go"] and .findings == [["internal/review/stamp.go","changes a golden file"]]' <<<"$output" +} + +@test "AC2 existing pattern alternatives are preserved and helpers excluded" { + local key old current path + for key in golden_paths constitutional_paths owner_paths; do + old=$(git -C "$ROOT" show origin/main:.agents/agent-skills.conf | sed -n "s/^$key = //p") + current=$(sed -n "s/^$key = //p" "$ROOT/.agents/agent-skills.conf") + # The old expression is kept byte for byte as the first alternative. + [[ "$current" == "$old" || "$current" == "$old"'|^('* ]] + for path in .agents/agent-skills.conf .agents/review-instructions.md .github/rulesets/main.json \ + .github/CODEOWNERS .claude/settings.json packages/agents/source/lib/lib.sh \ + packages/agents/source/lib/agent.sh packages/agents/source/lib/signing.sh \ + packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py \ + .agents/skills/merge-pr/scripts/merge-agent.sh; do + if [[ "$path" =~ $old ]]; then + [[ "$path" =~ $current ]] + fi + done + for path in internal/markdown/format.go internal/buildmeta/version.go internal/hookio/path.go \ + internal/checks-extra/check.go internal/mergepolicy.go cmd/agentcontext/main.go \ + cmd/agentrepo-extra/main.go; do + [[ ! "$path" =~ $current ]] || return 1 + done + done +} From e4e1d6da7c352425396cb3f1cf75b6226f798703 Mon Sep 17 00:00:00 2001 From: "tbhb.dev Agent" <334376894+tbhb-agent@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:19:26 -0400 Subject: [PATCH 2/2] test(agents): pin the pre-replacement protection baseline Shallow CI checkouts lack origin/main, so the preservation check can't read its baseline from that ref. Keep the previous patterns in a fixture so default branch updates don't change the comparison baseline. Refs #753 Assisted-by: codex:gpt-6.1-sol Signed-off-by: Tony Burns --- tests/skills/agent-merge-pr/fixtures/pre-go-protection.conf | 3 +++ tests/skills/agent-merge-pr/go-protection.bats | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 tests/skills/agent-merge-pr/fixtures/pre-go-protection.conf diff --git a/tests/skills/agent-merge-pr/fixtures/pre-go-protection.conf b/tests/skills/agent-merge-pr/fixtures/pre-go-protection.conf new file mode 100644 index 00000000..fff14482 --- /dev/null +++ b/tests/skills/agent-merge-pr/fixtures/pre-go-protection.conf @@ -0,0 +1,3 @@ +golden_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$)) +constitutional_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/(rulesets/|CODEOWNERS$)|\.claude/settings\.json$|packages/agents/source/(lib|skills/merge-pr/(shared|claude|codex)/scripts)/(merge-policy\.py|merge-agent\.sh|signing\.sh|lib\.sh)$|(packages/agents/[^/]+/\.apm/skills|\.(claude|agents)/skills)/[^/]+/scripts/(merge-policy\.py|merge-agent\.sh|signing\.sh|lib\.sh)$) +owner_paths = ^(\.agents/(agent-skills\.conf|review-instructions\.md)$|\.github/rulesets/|\.claude/settings\.json$|packages/agents/source/(lib/(lib|agent)\.sh$|skills/[^/]+/(shared|claude|codex)/scripts/guard[^/]*\.sh$|skills/merge-pr/(shared|claude|codex)/scripts/(merge-agent\.sh|merge-policy\.py)$)|(packages/agents/(claude|codex)/\.apm/skills|\.(claude|agents)/skills)/([^/]+/scripts/(guard[^/]*\.sh|(lib|agent)\.sh)$|(merge-pr|agent-merge-pr)/scripts/(merge-agent\.sh|merge-policy\.py)$)) diff --git a/tests/skills/agent-merge-pr/go-protection.bats b/tests/skills/agent-merge-pr/go-protection.bats index cb1f419c..9f2bc046 100644 --- a/tests/skills/agent-merge-pr/go-protection.bats +++ b/tests/skills/agent-merge-pr/go-protection.bats @@ -99,7 +99,8 @@ assert_replacement() { @test "AC2 existing pattern alternatives are preserved and helpers excluded" { local key old current path for key in golden_paths constitutional_paths owner_paths; do - old=$(git -C "$ROOT" show origin/main:.agents/agent-skills.conf | sed -n "s/^$key = //p") + # Pin the pre-replacement policy without requiring Git history or remote refs. + old=$(sed -n "s/^$key = //p" "$BATS_TEST_DIRNAME/fixtures/pre-go-protection.conf") current=$(sed -n "s/^$key = //p" "$ROOT/.agents/agent-skills.conf") # The old expression is kept byte for byte as the first alternative. [[ "$current" == "$old" || "$current" == "$old"'|^('* ]]