Repository navigation
feat(agents): add zsh-syntax-reviewer domain role - #789
Conversation
897461a to
7bbeaaf
Compare
ss-o
left a comment
There was a problem hiding this comment.
Fallback review under ADR-0026: maintainer elected, no Copilot request on 7bbeaaf
Findings
No actionable findings.
Scope: Reviewed commit 7bbeaaf and complete diff (+254, -0 across 7 files) for PR #789.
Note
This is a maintainer-elected self-review, not an independent review or approval to merge.
Validation
- Passed:
python3 automation/knowledge/knowledge-delivery.py --check: verified 57 delivered consumers match source files byte-for-byte.python3 automation/knowledge/knowledge-coverage.py: verified 362 files (57 imported, 305 referenced, 0 unclassified).python3 automation/agents/validate-agent-policy.py: validated instruction surface metadata and schema conformance.python3 automation/agents/org-routing.py validate: validated organization instruction surface routes and inventory.python3 automation/governance/decision-records.py --check: verified decision records.python3 automation/ci/validate-zsh-standard-policy.py: verified Zsh standard policy data.git diff --check: verified clean whitespace and diff formatting.- Character set scan: verified zero U+2014 em dash characters across all created and modified files.
- Hosted GitHub Actions checks: all 12 checks passed on head commit 7bbeaaf (Commit Lint, Agent Instruction Validation, Org Routing, Lychee, CodeQL).
- Skipped or unavailable:
- Live hosted GitHub Copilot chat agent discovery in a browser/IDE UI remains unexercised locally as it requires a live Copilot chat session.
Checklist verdicts and supporting evidence
- Instruction architecture (ADR-0014): pass. The canonical role source lives under
knowledge/domains/zsh/roles/, is declared inknowledge/delivery.json, generated viaknowledge-delivery.pyinto.github/agents/zsh-syntax-reviewer.agent.md, and routed via.github/instruction-surfaces.jsonwithout modifying the rootAGENTS.mdor introducing vendor files. - Surface routing and task mapping: pass. Surface
agent-zsh-syntax-reviewerdeclares advisory authority, copilot consumer, and specific tasks (zsh-syntax-review,zsh-ast-review,zsh-dialect-review,zsh-lint-parser-gap,zsh-lint-rule-proposal,code-review). - Domain coverage and catalog: pass. Registered in
knowledge/repository-files.jsonas imported and referenced inknowledge/domains/zsh/index.mdandrepository-resources.md. - Security and read-only invariants: pass. The role instructions explicitly require read-only auditing, evaluate syntax via
zsh -f -n, forbid unreviewedeval, and mandate reporting without local modifications.
Limits and follow-up
- Hosted Copilot discovery requires interactive client evaluation.
- Staging and merge operations remain separately authorized.
ss-o
left a comment
There was a problem hiding this comment.
Fallback review under ADR-0026: maintainer elected, no Copilot request on 5d92e80
Findings
No actionable findings on the reviewed head. Both initial review findings are resolved: the role now selects the canonical five execution profiles and requires function-scoped option isolation while rejecting sourced top-level local_options as caller-state protection.
Scope: Re-reviewed the delta from 7bbeaaf6e32b80fdb89a01a144615e8f725e2f47 to 5d92e809, then checked the corrected role against its canonical contracts and delivery map. The full PR remains seven files, with 246 additions, against merge base eef8360a6defc1afe449b23169df33c817c7d2f6; the observed base tip is bb533df520010a19ce9cb780778001258dbc1cfc.
Note
This review includes corrections authored in the reviewing session. Merge authority remains a separate maintainer decision.
Checklist verdicts
- Profile classification: pass. The role's profile set exactly matches
execution_profilesinknowledge/domains/zsh/data/zsh-standard-policy.json, includingstartup-fileandtest-fixture. - State preservation: pass. The option check cites
zsh/options/localizeandzsh/options/no-top-level-leak, requires a function beginning withbuiltin emulate -L zshfor reusable work, and explicitly rejects top-levellocal_optionsas isolation. A Zsh 5.9.2 runtime probe confirmed that sourced top-levellocal_optionsleakssh_word_split, while the localized anonymous function restores it. The released Zsh manual defines restoration at function return. - Ownership and generated delivery: pass. The editable source remains under
knowledge/domains/zsh/roles/; the delivered agent is regenerated throughknowledge/delivery.json. All 57 consumers match their generated content, and the repository inventory covers all 362 files. - Routing and scope: pass. The manifest declares an optional advisory Copilot surface and appropriate review task selectors. The role remains read-only and defers language authority to the canonical standard and released Zsh manual. No mandatory policy is delegated solely to this role.
- Documentation: pass. The domain index and imported resource record remain consistent with the delivery map. The PR description answers all seven instruction-impact questions and explicitly marks live discovery unverified.
Validation
- Passed:
python3 -B automation/knowledge/knowledge-delivery.py --check,python3 -B automation/knowledge/knowledge-coverage.py,python3 -B automation/agents/validate-agent-policy.py,python3 -B automation/agents/org-routing.py validate,python3 -B automation/governance/decision-records.py --check, andpython3 -B automation/ci/validate-zsh-standard-policy.py. - Passed: source-only
trunk check --no-fix --ci knowledge/domains/zsh/roles/zsh-syntax-reviewer.md, full merge-basegit diff --check, execution-profile comparison, and local verification of the new commit's signature. - Passed: all 12 hosted checks on
5d92e80930dc032be97448b6e002f8cb1502217f, covering Agent Instruction Validation, Org Routing, Commit Lint, link checking and CodeQL. - No review threads exist at the time of review.
Limits
Live Copilot custom-agent discovery and invocation remain unverified. The runtime probe covers Zsh 5.9.2; it does not establish a compatibility matrix or measure reviewer behavior. Desktop and narrow-width rendered review inspection is unavailable in this session.
Summary
knowledge/domains/zsh/roles/zsh-syntax-reviewer.mdunder ADR-0014 portable instruction delivery.knowledge/delivery.json, generate.github/agents/zsh-syntax-reviewer.agent.md, and route the optional surface for syntax, AST, dialect, parser-gap, rule-proposal and code reviews.setopt local_optionsat sourced top level as caller-state protection.Refs #475 for the portable instruction architecture.
Instruction impact review
.github/agents/zsh-syntax-reviewer.agent.mdfrom this repository. Other runtimes may explicitly read the canonical role. No downstream installation or runtime invocation is claimed by the inventory.knowledge/domains/zsh/roles/zsh-syntax-reviewer.mdowns the role, andknowledge/delivery.jsonmaps its generated consumer. Organization policy remains inAGENTS.mdand the canonical Zsh standard.agent-zsh-syntax-reviewerto.github/instruction-surfaces.json, its source-to-target mapping toknowledge/delivery.json, and its imported disposition toknowledge/repository-files.json. No downstream route changes are needed for this optional organization-hosted role.knowledge-delivery.py --checkverifies all 57 consumers,knowledge-coverage.pyverifies 362 files with zero unclassified files, and agent-policy validation passes the repository's inventory and size checks.Verification
python3 -B automation/knowledge/knowledge-delivery.py --check: passed, 57 consumers.python3 -B automation/knowledge/knowledge-coverage.py: passed, 362 files, zero unclassified.python3 -B automation/agents/validate-agent-policy.py: passed.python3 -B automation/agents/org-routing.py validate: passed.python3 -B automation/governance/decision-records.py --check: passed.python3 -B automation/ci/validate-zsh-standard-policy.py: passed.trunk check --no-fix --ci knowledge/domains/zsh/roles/zsh-syntax-reviewer.md: passed.git diff --check: passed.local_optionsleavessh_word_splitenabled; an anonymous function beginning withbuiltin emulate -L zshrestores the caller's option state.Agent handoff
Both initial review findings are corrected and the published head
5d92e809has been re-reviewed with no actionable findings. All 12 hosted checks pass, and the maintainer-elected fresh review of record is posted on that head. Merge remains a separate maintainer decision.