AM-1430 - Let a repo name its own standards and reviewer skills by bretttully · Pull Request #32 · nearmap/shipyard

bretttully · 2026-08-30T04:13:29Z

Two config keys — skills.standards and skills.reviewer — each dispatched through its own new agent (repo-standards, repo-review). Both ship null, so every existing path is byte-identical until a repo sets one.

The reviewer is dispatched by sy:gate itself against gate's already-pinned scope, so it never re-pins and never becomes a second verdict.
Its floor is spec-gate's, not gate's: it is additive, and sy:gate stays the frontier-floored backstop.
A reviewer finding the GATE worker declines returns as a reason: reviewer_findings needs-decision (batched per round) rather than being dropped — a repo's reviewer encodes team taste, so that call is the ticket owner's. Mirrors max_gate_rounds; touches neither gate_rounds_* field, and accepting the rejections always converges the loop.
repo-review vets before it returns: sy:hunt refute mode per contested or HIGH finding, sy:seam only for a genuine boundary finding. sy:debate is untouched — its charter is the core decision behind a roadmap, plan, or spike verdict, and a review finding is not one. A DIES verdict comes back as a refuted finding, not a dropped one; dropping it would be dispositioning, which this agent never does.
A plan may carry one optional reviewer orientation sentence, collected by /sy:spec only when skills.reviewer is non-null, threaded through the plan file to repo-review as orientation that overrides nothing.
When the reviewer runs, its report is posted to this PR as a standalone human-readable comment, once per reviewed SHA: repo-review writes it into the repo scratch root, sy:gate carries the path, and the GATE worker posts it through /sy:pr. /sy:pr posts because it owns the PR surface — not because the reviewer is blocked from posting.
Pre-existing guard gap, fixed here rather than introduced here. review_guard modelled local mutation only: MUTATING_COMMANDS is all filesystem verbs and MUTATING_GIT all checkout verbs, so gh and curl fell through _segment_reason to None. Every review mode — sy:gate and sy:hunt included, on main today — could run gh pr merge --squash, gh pr close, gh api -X POST or curl -X POST against the PR it was reviewing. A remote deny-list closes that for all four modes. It stays a natural-spelling deny-list, so an unrecognised read fails open and adversarially quoted or bundled shell is out of scope — an explicit, documented limit of a backstop, not a gap. The gh api leg denies both a named mutating method and a field flag (-f/-F/--raw-field/--field/--input) with no explicit method, which gh documents as an implicit POST; gh api graphql is exempt from the second by command shape, not by inspecting the query, because the plugin's own PR flows run through exactly that shape for both a read (enumerating review threads) and a write (skills/pr/SKILL.md's requestReviews mutation, which requests the automated reviewer); a mutation hidden in an uninspected query body is therefore an accepted, documented gap of this backstop, not pending work. Issue-level subcommands are deliberately absent: CONTRIBUTING.md bans gh issue by name in core, and the gh api leg covers the same writes over REST.
review_guard's SANDBOX_WRITE_MODES splits "guarded" from "may write the sandbox": repo-review writes only under repo_scratch_dir, repo-standards writes nowhere.

Net-new agent-facing text — justification, one line per addition

agents/repo-standards.md — the standards pass is a delegate in three call sites already; without an agent file it has no floor, no tools allowlist, and no return contract, so it inherits every tool and returns whatever it likes.
agents/repo-review.md — the reviewer needs a brief that pins the {"repo": true} scratch root and the five blocked returns; a bare Skill invocation from gate would silently pass on an uninvokable skill.
agents/gate.md § Review, dispatch clause — gate is the only dispatcher, so a rule it does not carry never runs.
agents/gate.md § Review, attribution clause — the GATE worker triages reviewer findings under a rule of its own and cannot apply it to a finding it cannot tell apart from gate's.
immutable-gate.md § Fix cycle, Reviewer findings — states where a declined reviewer finding goes; without it the default is the silent drop the carve-out exists to stop.
skills/ship/SKILL.md § Worker contract, reviewer-findings paragraph — the parent resolves a needs-decision itself by default, so the skip-and-ask has to be stated on the parent side too.
resolve.md tier 1 / review.md opening — both said "a dedicated repository standards skill" with nothing naming what resolves it; the key is now named and cited.
start-resume.md step 4, skills/spec/SKILL.md §3 — an unnamed delegate cannot be dispatched at a resolved model.
agents/repo-review.md § Vet before returning — without it the agent is a relay; the depth bound and the primitive choice are exactly what a model would otherwise improvise per run, against a cap it shares with sy:gate.
agents/repo-review.md, degrade-to-unvetted sentence — nested dispatch is not perfectly reliable under load, and the default failure mode of a vetting step is to swallow what it could not check.
agents/repo-review.md, appended-orientation ceiling — a plan-authored sentence appended to an agent brief is an override of that brief unless something says it is not.
skills/spec/SKILL.md §7, reviewer orientation field and its ask condition — a field no plan half declares is one no plan can carry, and an unconditional question would appear for every repo that names no reviewer.
immutable-gate.md § Pin scope, one clause — gate composes its seed from the plan file; a field it does not read stops threading one hop short of its consumer.
agents/gate.md § Review, append clause — gate is the only site that dispatches the reviewer.
agents/repo-review.md § Write the report the caller posts — findings that live only in a return reach nobody but gate; the report's audience, contents and the do-not-post rule all have to be stated where it is written.
agents/gate.md return block, REPO_REVIEW_REPORT — the caller cannot post a path gate never reports.
immutable-gate.md § Fix cycle, Posting the reviewer's report — states the poster, the once-per-scope key, and what a failed post does; the default for all three is a silent skip, which defeats the step.

Seam narrowing — justification

sy_tools/tests/test_tracker_seam.py banned the bare token gh everywhere under sy_tools/ outside the tracker zone. That is stricter than the repo's own two statements of the same rule: CONTRIBUTING.md bans "jira, acli, gh issue, gh-project, ADF, or any other tracker-native term", and scripts/validate.py's check_seam implements exactly that (gh issue, gh project, gh gist). Both ban GitHub-as-tracker vocabulary, not the CLI's name — which is why skills/pr/SKILL.md may already say gh pr comment.

The extra strictness had never been load-bearing because no Python in the package had a reason to say gh. A matcher does: review_guard denies a remote write by recognising the literal words an agent would type, and a tracker-neutral stand-in cannot deny gh pr merge. That is a different case from the guard's self-test corpus, which the whole-file rule protects and which still uses neutral names.

So the bare gh pattern alone now carries zones where it does not apply — sy_tools/guards/ and sy_tools/tests/guards/. Scoped per-token, never via _exempt(), which is whole-file and token-blind and would have admitted every tracker name into those directories at once. Two tests pin the scoping: a guard file naming jira/acli/atlassian/github/gist/subtask still violates, and a non-guard sy_tools/ file naming bare gh still violates. check_seam is untouched. Narrowing only, so no previously-passing file can become a violation — confirmed by running the seam test, not assumed.

Contract compatibility — all three callers of `resolve.md`'s return block still read true

skills/spec/SKILL.md:50 — "Resolve standards in the repo-standards delegate (running /sy:standards resolve <scope> at the model agent_model {"name": "repo-standards"} reports)". Route: via repo-standards.
skills/ship/references/start-resume.md:8 — "Resolve standards in the repo-standards delegate (running /sy:standards resolve <task scope> at the model agent_model {"name": "repo-standards"} reports, added to agents_used)". Route: via repo-standards.
agents/gate.md § Review — "Invoke /sy:standards review <scope> as a separate conformance pass". Route: inline, unchanged. The new bullet only adds that one dispatch discharges this pass when both keys name the same skill.

Not discharged by this PR — manual, post-install

The running MCP server reads the installed plugin cache, not this branch, so no live agent_model row for the two new agents can exist during the ship. With this branch installed and a fresh session: agent_model resolves repo-standards and repo-review; with skills.reviewer set to a real skill, a repo-review dispatch returns findings carrying file:line and a head SHA matching the pinned one; set to a name resolving to no skill, it returns blocked. A failure after merge is a follow-up bug, not a revert — both keys ship null, so the path is inert until a repo sets one.

Every net-new validator pin was checked for vacuity by mutating its carrier in place and re-running scripts/validate.py: 25/25 fire, plus 2/2 on the seam scoping. The probe caught two real defects — a dead repo-review containment leg that no text satisfying the attribution pin could fail, and the ## Fix cycle-is-last-heading case where _bounded_section refuses by design.

sy_tools/tests/test_config.py::test_all_worktrees_resolves_a_relative_gitdir_record fails on the build box only: local git is 2.43.0 and git worktree add --relative-paths landed in 2.48. It fails identically on clean main at 0fa09a2.

Two config keys, `skills.standards` and `skills.reviewer`, each dispatched through its own new plugin agent (`repo-standards`, `repo-review`). Both ship null, so every existing path is byte-identical until a repo opts in. The reviewer is dispatched by `sy:gate` itself against gate's already-pinned scope, and its findings are candidates gate judges rather than a second verdict. A reviewer finding the GATE worker declines returns as a `reason: reviewer_findings` needs-decision, batched per round, so a taste call the repo's own reviewer made is put to the ticket owner instead of dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tence `repo-review` now dispatches depth agents to keep or refute a finding before returning it, so what reaches gate is vetted rather than raw. `sy:hunt` refute mode is the primary primitive and `sy:seam` covers a genuine boundary finding; `sy:debate` is untouched, since its charter is the core decision behind a roadmap, plan, or spike verdict and a review finding is not one. Vetting is bounded against the phase-wide depth cap, and a depth agent that fails to dispatch leaves the finding marked `unvetted` rather than dropped. A plan may also carry one optional `reviewer orientation` sentence, collected by `/sy:spec` only when a reviewer skill is configured, threaded through the plan file to `repo-review` as orientation that overrides nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

When `repo-review` runs, its findings now reach the pull request instead of living only inside gate's return. `repo-review` writes a standalone human-readable report into the repo scratch root, `sy:gate` carries the path through its return block without pasting the body, and the GATE worker posts the file through a `/sy:pr` delegate before triaging. One comment per review scope, keyed to the reviewed SHA. Posting is not dispositioning: the comment records what the repository's own reviewer said and what vetting concluded, never what the worker decided to act on. A post that fails twice returns `blocked` naming the report's on-disk path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`review_guard` modelled local mutation only: `MUTATING_COMMANDS` is all filesystem verbs and `MUTATING_GIT` all checkout verbs, so `gh` and `curl` fell through `_segment_reason` and every review mode could merge, close or comment on the pull request it was reviewing. A remote deny-list closes that for all four modes, keyed on the method a command names rather than the verb underneath, so `gh api graphql` stays a read. The seam test banned the bare token `gh` under all of `sy_tools/`, stricter than `CONTRIBUTING.md` and `check_seam`, which both ban GitHub-as-tracker vocabulary instead. A matcher has to spell the command an agent would type, so that one token is now scoped to the guard directories; every other token still fires there, and two tests pin the scoping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…lose two guard gaps The mandated report body routinely contains `>` (quoted evidence, refutations quoting code), and the guard's redirect scan runs over the whole Bash command including heredoc text, so the shell-redirect instruction was realistically denied with no loud failure. repo-review now writes the report with the `Write` tool, which the guard already permits under the resolved scratch root, and a report that cannot be written is a `blocked` return. `gh api` with a field flag and no explicit method is a POST per gh's own documentation, so the guard now denies it outside `gh api graphql`; `gh pr create|lock|unlock` join the pr deny-list; and the `MUTATING_GH` comment no longer claims an enforcement the seam check stopped performing inside `sy_tools/guards/`.

…d missing two `gh pr` writes `_gh_words` stepped over only `-R`/`--repo`/`--hostname`, so `gh api`'s own value-taking flags left their value in the positional list and shifted `graphql` out of the position the GraphQL read exemption reads -- `gh api -H 'Accept: ...' graphql -f query=...` was denied. Naive whitespace splitting did the same for any quoted value holding a space, so tokenization is now quote-aware with a fallback for unbalanced quoting. `gh pr revert` and `gh pr update-branch` both mutate the remote and were missing from MUTATING_GH. GH_API_FIELD_FLAGS is reordered to gh's actual pairing (`-F, --field` / `-f, --raw-field`); the check is a flat membership test, so this is accuracy only.

`shlex.split` keeps `curl "-d hello" URL` as the single word `-d hello`, which matched nothing in the body-flag set, so a real POST was allowed. Split the flag off at the first space as well as the first `=`. Also cover the `_tokens` unbalanced-quote fallback -- reached whenever a `;` inside a quoted argument splits a segment mid-quote -- and move the tokenizer's rationale out of its docstring into an inline comment.

… quoted separators curl accepts single-dash short options bundled and with their values glued on, so a flag is neither the whole token nor its leading characters: `-sXPOST`, `-d@body.json`, `-Fk=v` and `-skXDELETE` all executed real requests past the body/method matchers (verified live against a loopback socket). `_curl_bundled_flag` walks a single-dash curl token character by character against the short body and method flags together, and both legs -- the body-flag loop and `_flag_value` for `-X` -- now consult it. wget is untouched: its flags are all long-form. A `;` inside an ordinary quoted value is no separator, but `_classify_bash` split the raw string before tokenizing, so `curl -H "Content-Type: application/json; charset=utf-8" -d '{"body":"x"}' URL` was cut at the charset directive and its POST was never seen. Quoted spans are now masked before the split and unmasked before `_segment_reason`, which is otherwise unchanged; unbalanced quoting matches nothing and degrades to today's behaviour. The redirect scan still reads the raw command.

…model Rounds 4-5 of this session tried to make review_guard survive an adversarial shell: a pre-pass that masked quoted spans before splitting on separators, and a character walk over curl's bundled short flags. Both reimplemented semantics that belong to bash and curl, and both got them wrong -- a 4706-command sweep found 71 new fail-opens that the commit before them did not have, plus a false deny of ordinary reads like `curl -o/dev/null URL`. Revert that escalation: `_QUOTED_SPAN`/`_mask_quoted`/`_unmask`, `_curl_bundled_flag`, the `bundled` leg of `_flag_value` and their cases are gone. Kept, because they are data-table and false-deny fixes rather than parser work: the `gh api` implicit-POST deny with its graphql exemption, the `MUTATING_GH` completions, the `_GH_VALUE_FLAGS` extension, the shlex-based `_tokens` (a bounded per-segment word split, and what both the graphql read and the quoted curl body flag depend on), and the curl body-flag space split. The module docstring now states the threat model plainly -- an honest mistake by a reviewer, not an attacker -- and names quoted separators, bundled curl flags and interpreter indirection as accepted limits, so this is not reopened as a defect.

The opening paragraph still described the remote leg as keying only on a named method, which stopped being the whole story once the field-flag implicit-POST check landed. Say both legs plainly and name the graphql exemption instead of leaving the docstring to contradict the code.

Copilot

Pull request overview

Adds repository-configurable “standards” and “reviewer” skill hooks, each executed via new dedicated agents, enabling repos to opt into custom standards resolution and additive review without changing default behavior when unset. Also strengthens the review guard to prevent accidental remote-side mutations during review-mode runs, and updates validation + docs to pin the new contracts.

Changes:

Introduces skills.standards and skills.reviewer config keys plus new repo-standards and repo-review agents to dispatch them safely and consistently.
Extends sy_tools/guards/review_guard.py to deny remote writes via gh and curl/wget while preserving essential read paths (notably gh api graphql).
Adds/updates validators, tests, and documentation to enforce naming, scoping, and routing of reviewer findings/reporting.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
sy_tools/tests/test_validate.py	Adds a synthetic test to pin agent frontmatter `name` matching filename stem using real floors/defaults.
sy_tools/tests/test_tracker_seam.py	Refines seam scanning to scope only the bare `gh` token exemption to guard-related directories and adds regression tests.
sy_tools/tests/test_config.py	Adds config validation/resolve tests for new `skills.standards` and `skills.reviewer` keys and their constraints.
sy_tools/tests/guards/test_review_guard.py	Expands guard tests to cover sandbox-write mode distinctions and remote-write denial/remote-read allowance.
sy_tools/guards/review_guard.py	Implements remote mutation deny-listing (gh + curl/wget) and adds tokenization improvements via `shlex`.
skills/standards/references/review.md	Updates standards “review mode” guidance to reference `skills.standards` explicitly.
skills/standards/references/resolve.md	Updates standards “resolve” routing to be explicitly keyed on `skills.standards`.
skills/spec/SKILL.md	Routes standards resolution through `repo-standards` and adds the conditional “reviewer orientation” plan field.
skills/ship/SKILL.md	Extends worker contract to route `reason: reviewer_findings` via `AskUserQuestion`.
skills/ship/references/start-resume.md	Updates standards resolution step to use `repo-standards` via `agent_model {"name": "repo-standards"}`.
skills/ship/references/immutable-gate.md	Defines reviewer report posting and reviewer-findings “needs-decision” handling in the fix cycle.
scripts/validate.py	Adds new agents to expected set, pins new contract text via targeted validators, and checks name/stem drift.
README.md	Documents the two new agents in the repo layout overview.
docs/usage.md	Documents that `sy:gate` also runs a repo reviewer when `skills.reviewer` is configured.
docs/configuration.md	Documents `skills.standards` and `skills.reviewer` configuration keys and updates tier wording.
config/schema.json	Adds schema for `skills.standards` and `skills.reviewer` plus wording update for tier rationale.
config/floors.json	Adds floors for `repo-standards` and `repo-review`.
config/defaults.json	Ships null defaults for new config keys and adds default per-agent model bindings for the new agents.
agents/repo-standards.md	New agent contract for standards delegation (resolve/review) with compact return requirements.
agents/repo-review.md	New agent contract for repo reviewer execution, vetting, and writing a posted report artifact.
agents/gate.md	Dispatches `repo-review` when `skills.reviewer` is set, threads “reviewer orientation”, and carries report path.
agent-guide.md	Documents the new optional config keys at a high level for operators.
.claude-plugin/plugin.json	Bumps plugin version to 1.28.0.

Suppressed comments (1)

sy_tools/guards/review_guard.py:288

The deny reason hard-codes --request, but for wget the mutating-method flag is --method (and for curl it might be -X). Use a flag-agnostic message so the error is accurate and actionable.

    method = _flag_value(rest, REMOTE_METHOD_FLAGS[cmd])
    if method is not None and method.upper() in MUTATING_HTTP_METHODS:
        return f'{cmd} --request {method.upper()} writes to the remote'

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot review thread on PR #32: the gh-api method deny hard-coded --method in its message even though -X denies it too. The curl/wget method deny had a second bug Copilot didn't name: it hard-coded --request for both commands, but wget only accepts --method, so a denied 'wget --method POST' reported a flag wget does not take. Both messages now name the method, not how it was written.

bretttully · 2026-08-30T23:57:13Z

Review coverage

REVIEW_BASE_SHA: 0fa09a2f646fa57c457b484ef9f3bf36d9c7f7f0
REVIEWED_SHA: 3ecd3c4a8600ebe6f9081e534498e9a956849b1c
REVIEW_MODEL_REQUESTED: opus
REVIEW_EFFORT: max

bretttully · 2026-08-30T23:57:14Z

Acceptance evidence — head `3ecd3c4`

Verification (independently re-run at head, not taken from a builder's self-report)

pixi run pytest -q --deselect sy_tools/tests/test_config.py::test_all_worktrees_resolves_a_relative_gitdir_record → 904 passed, 0 failed. The deselected test fails on this build box only because git worktree add --relative-paths needs git >= 2.48 and the box runs 2.43.0; reproduced identically on clean main, so not a regression.
python scripts/validate.py → clean
pixi run ruff check sy_tools/ → clean
pixi run ty check sy_tools/ → clean
python sy_tools/guards/review_guard.py self-test → passed

CI — 5/5 checks green at 3ecd3c4 (Validate/validate, Validate/sy-tools, CodeQL Analyze-actions, CodeQL Analyze-python, CodeQL). All passed on the first attempt, no retries. Confirmed independently via the GitHub API, not just the poller.

Automated review — GitHub Copilot was requested post-promotion and landed within 150 seconds. One genuine inline finding: a deny-reason message hard-coded a specific CLI flag spelling. Fixed in commit 3ecd3c4, along with a second instance of the same bug shape that Copilot's comment did not call out (a wget-specific message naming a flag wget does not accept). Thread resolved.

Ship history — this branch went through 7 gate rounds.

Rounds 1-3 fixed real gaps in a new remote-mutation deny-list added to sy_tools/guards/review_guard.py: an implicit-POST gh api check, MUTATING_GH subcommand completeness, and a gh api value-taking-flag parsing bug.
Rounds 4-5 attempted to harden the same guard against adversarially quoted/bundled shell spellings. Round 5's attempt introduced two HIGH-severity fail-open regressions, confirmed via a 4706-command fuzz sweep.
That parser-hardening escalation was reverted in full (commit 900be98) per an explicit decision to scope this guard as a backstop against honest mistakes, not an adversarial-shell parser. The module's docstring now states this threat model plainly and names the accepted out-of-scope bypass shapes. Adversarial-shell hardening for this file is deferred to AM-1274, already an open ticket against this module.

Final state — no undispositioned findings. The last two independent sy:gate reviews (base 9224e53 → head 3ecd3c4, and the round-7 review before it) both returned "safe to ship" with zero findings.

A self-check of the shipped prose found three wrong statements and two real code defects behind them. The `gh api graphql` exemption was documented in three places as "a read carried over POST". GraphQL carries mutations too, and this plugin's own `skills/pr` runs a `requestReviews` mutation through exactly that shape. The exemption keys on the literal command shape and never reads the query, so it exempts writes as well; that is now stated as an accepted gap alongside the docstring's other accepted bypasses rather than implied away. No query-body inspection is added: that detection was tried and backed out. `gh release delete-asset` deletes an asset from a published release and was missing from `MUTATING_GH['release']`. `MUTATING_GH['pr']` was re-read against `gh pr --help`'s full subcommand list and is complete, so it is unchanged. `_GH_VALUE_FLAGS` was missing `gh api`'s field flags, so `gh api -f query=x graphql` -- flags before the endpoint, valid gh syntax -- had `query=x` read as the endpoint, pushed `graphql` out of the position the exemption reads, and was denied outright: a false deny against a legitimate read. It now splats `GH_API_FIELD_FLAGS`, tied to the one definition instead of a second copy. `test_tracker_seam.py`'s docstring claimed `CONTRIBUTING.md` and `check_seam` both name the literal triple `gh issue`/`gh project`/`gh gist`. Only `check_seam` does; `CONTRIBUTING.md` writes `gh-project` and reaches `gh gist` only through its catch-all. The substantive claim holds and stays. `-p` stays unconditionally value-taking: no natural gh spelling puts a boolean `-p` ahead of the group or subcommand word, so the over-skip never moves `words[0]`/`words[1]`.

Round-8's doc-accuracy fix reworded the false 'gh api graphql is a read carried over POST' claim in review_guard.py's own three spots but left the identical claim standing in two places that exist to justify/pin that same code: the pytest mirror's own docstring (test_review_guard.py) and validate.py's check_seam pin rationale. Both now key on the exempt command shape, matching the fixed wording. Also: the seam-test docstring's correction from last round undercounted CONTRIBUTING.md's tracker-vocabulary examples (it names gh issue and gh-project, not just gh-project) and dropped the fact that gh issue is the same literal check_seam matches -- fixed. And: _GH_VALUE_FLAGS's comment now discloses that its subcommand-wide value-skip has a fail-open consequence for a field flag written before its own subcommand (gh pr -f merge, gh release --input create) -- an accepted limit in the same direction as this module's other documented bypasses, not a defect. No behavior change: comment/docstring/pin-rationale text only.

sy:gate's review of the round-9 doc-accuracy fix found a regression I introduced while fixing the false 'graphql is a read' claim: the replacement docstring in test_review_guard.py said any field-carrying graphql call is allowed, dropping 'that names no method' from the base wording. That's false -- an explicit mutating --method still denies, graphql or not (the exemption only covers the unmethoded-field-flag leg). Restored the qualifier and named the still-denied case. Also tightened review_guard.py's _GH_VALUE_FLAGS disclosure comment: 'steps over the subcommand word along with the flag's value' implied a three-token skip; the actual mechanism is a two-token skip landing on the subcommand word's slot, and a third token (gh pr -f x=1 merge) still denies. Named that boundary so the comment can't be misread as claiming a wider fail-open than exists. No behavior change: comment/docstring text only.

bretttully · 2026-08-31T01:02:53Z

Review coverage (updated)

REVIEW_BASE_SHA: 0fa09a2f646fa57c457b484ef9f3bf36d9c7f7f0
REVIEWED_SHA: 1a6e13a3c5e4c7cf74f5a3d5903933d9adafba12
REVIEW_MODEL_REQUESTED: opus
REVIEW_EFFORT: max

Three further gate rounds landed after the acceptance-evidence comment above (head 3ecd3c4c...): a HANDOFF doc-accuracy self-check found four real factual defects in shipped prose/comments (a false "gh api graphql is a read carried over POST" claim in three places, MUTATING_GH['release'] missing delete-asset, _GH_VALUE_FLAGS missing gh api's field flags — a real live false-deny — and a seam-test docstring overclaiming literal parity between CONTRIBUTING.md and check_seam), all fixed in commit b48818a; two follow-up rounds (30408d9, 1a6e13a) caught and corrected two further wording regressions the fix itself introduced, both comment/docstring-only with zero behavior change (verified via AST/token-stream diff against base). Final independent verification at head 1a6e13a: pixi run pytest 916 passed / 0 failed (the git-2.43 --relative-paths case remains the sole expected deselection), scripts/validate.py clean, ruff/ty clean on sy_tools/, guard self-test clean. All guard-behavior verification this round was done exclusively via in-process decision() calls — no live gh/curl/wget command was executed against any real endpoint. One non-blocking LOW remains, explicitly deferred by the reviewing pass: gh api graphql combined with an explicit mutating --method denies correctly today but isn't pinned by an existing test case — a genuine but small, disclosed coverage gap, not a behavior defect, left for a follow-up. CI green 5/5 at 1a6e13a, first attempt, independently confirmed.

bretttully and others added 10 commits August 30, 2026 04:12

bretttully marked this pull request as ready for review August 30, 2026 23:46

bretttully requested a lite review from Copilot August 30, 2026 23:46

Copilot started reviewing on behalf of bretttully August 30, 2026 23:46 View session

Copilot AI reviewed Aug 30, 2026

View reviewed changes

Comment thread sy_tools/guards/review_guard.py Outdated

bretttully added 3 commits August 31, 2026 00:30

bretttully merged commit 7e9828c into main Aug 31, 2026
5 checks passed

bretttully deleted the AM-1430/config-skill-agents branch August 31, 2026 01:29

Navigation Menu

Uh oh!

AM-1430 - Let a repo name its own standards and reviewer skills - #32

AM-1430 - Let a repo name its own standards and reviewer skills#32
bretttully merged 14 commits into
mainfrom
AM-1430/config-skill-agents

bretttully commented Aug 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bretttully commented Aug 30, 2026

Uh oh!

bretttully commented Aug 30, 2026

Uh oh!

bretttully commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bretttully commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Net-new agent-facing text — justification, one line per addition

Seam narrowing — justification

Contract compatibility — all three callers of resolve.md's return block still read true

Not discharged by this PR — manual, post-install

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

bretttully commented Aug 30, 2026

Uh oh!

bretttully commented Aug 30, 2026

Acceptance evidence — head 3ecd3c4

Uh oh!

bretttully commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bretttully commented Aug 30, 2026 •

edited

Loading

Contract compatibility — all three callers of `resolve.md`'s return block still read true

Acceptance evidence — head `3ecd3c4`