Skip to content

docs(skills): pin shared go-guide and architecture-review - #79

Merged
andybons merged 3 commits into
mainfrom
docs/shared-review-skills
Oct 12, 2026
Merged

andybons merged 3 commits into
mainfrom
docs/shared-review-skills

Conversation

@andybons

@andybons andybons commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Gatekeeper had no shared Go rules and no architecture checklist for agents or reviewers, unlike harness (majorcontext/harness#524) and boxes (meetneptune/boxes#1596). This pins go-guide and architecture-review from andybons/kit in skills-lock.json at the same hashes, installs them under .agents/skills/, and adds .github/skills/gatekeeper-architecture-review with gatekeeper-specific checks (proxy auth 407, injection scope, MITM CA, token exchange, library boundary), written from AGENTS.md and docs/. AGENTS.md, a new .github/copilot-instructions.md, and the Claude code-review workflow prompt point at them. It also moves .claude/skills/verify/SKILL.md to .agents/skills/verify/SKILL.md (content unchanged) because .agents/skills/ is the agents protocol location, consistent with harness and boxes, which keep skills only there. The product skills under skills/ are untouched; no Go code changes. The installed skill files are byte-identical to the harness and boxes copies.

Agents and Copilot reviewed gatekeeper changes with no shared Go rules
and no architecture checklist, unlike harness and boxes.

Pin the go-guide and architecture-review skills from andybons/kit in
skills-lock.json and install them under .agents/skills/ at the same
hashes harness and boxes use. Add
.github/skills/gatekeeper-architecture-review for the gatekeeper checks:
proxy authentication, credential injection scope, the MITM CA, token
exchange, and the library boundary. Point AGENTS.md, a new
.github/copilot-instructions.md, and the Claude code-review workflow
prompt at them.

Verification: both installed skill files are byte-identical to the
copies in harness and boxes; no Go code changed.
Copilot AI balanced review requested due to automatic review settings October 12, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The authentication checklist incorrectly requires an HTTP 407 response from the Postgres protocol path.

1 open finding
What changed in this PR

Pins shared review skills and integrates them into Gatekeeper’s agent and review guidance; no Go code changes.

Changes:

  • Adds locked Go and architecture review skills.
  • Adds Gatekeeper-specific architecture checks.
  • Updates Copilot, Claude, and agent instructions to apply them.
File Description
skills-lock.json Pins shared skills.
AGENTS.md References installed review guidance.
.github/​workflows/​claude-code-review.yml Expands Claude’s review prompt.
.github/​skills/​gatekeeper-architecture-review/​SKILL.md Adds project-specific checks.
.github/​copilot-instructions.md Adds Copilot review instructions.
.agents/​skills/​go-guide/​SKILL.md Installs shared Go guidance.
.agents/​skills/​architecture-review/​SKILL.md Installs shared architecture guidance.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/skills/gatekeeper-architecture-review/SKILL.md Outdated
The description held an unquoted colon, which is invalid YAML. Several
questions read so that a yes was the safe answer, against the rule that
a yes is a finding. The auth question applied the HTTP 407 rule to the
Postgres listener, and the CA question said the key never reaches disk.

Quote-free the description, write each question as a failure condition,
split HTTP proxy auth from the Postgres run-token check, and describe
the NewCA ca.key file mode.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation-only changes are internally consistent, correctly pinned, and introduce no unresolved issues.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Harness and boxes keep agent skills only under .agents/skills/. Move the
verify skill there, content unchanged.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation-only changes consistently install and apply the intended review guidance without affecting runtime behavior.

0 open findings

🧠 Review effort: Balanced

@andybons
andybons merged commit ccc232a into main Oct 12, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants