skill: add mygo-api-repo-review skill.
This commit is contained in:
@@ -0,0 +1,85 @@
|
|||||||
|
---
|
||||||
|
name: mygo-api-repo-review
|
||||||
|
description: Review the MyGO backend repository for material, evidence-backed architecture, security, resilience, data-consistency, performance, and implementation flaws. Use only when explicitly asked to run a MyGO repository audit in diff, main, or full mode, optionally limited to a target path, package, or component; do not use for ordinary implementation or debugging tasks.
|
||||||
|
---
|
||||||
|
|
||||||
|
# MyGO API Repository Review
|
||||||
|
|
||||||
|
Perform a read-only, evidence-first review of the MyGO backend. Find real defects and systemic risks without manufacturing findings from stylistic preferences, unsupported hypotheticals, or intentional design choices.
|
||||||
|
|
||||||
|
## Resolve the review scope
|
||||||
|
|
||||||
|
Resolve two independent inputs: `mode` and `target`.
|
||||||
|
|
||||||
|
- Default `mode` to `diff` when the user does not provide one.
|
||||||
|
- Default `target` to `all`.
|
||||||
|
- Treat `partly` as a request for a limited `target`, not as a fourth mode. Require or safely infer the path, Go package, or logical component to inspect.
|
||||||
|
|
||||||
|
Use these mode semantics:
|
||||||
|
|
||||||
|
- `diff`: compare the current workspace with `HEAD`. Include staged changes, unstaged changes, and relevant untracked files.
|
||||||
|
- `main`: compare the current workspace with the merge base of `HEAD` and the local `main` branch. Record the local `main` and merge-base commit IDs. Do not fetch or silently substitute a remote ref. If local `main` is unavailable, report the limitation.
|
||||||
|
- `full`: inspect the current repository as a whole, including tracked code, tests, configuration, and documentation plus relevant untracked source files. Exclude repository metadata, caches, runtime data, build outputs, and generated artifacts unless they are directly relevant.
|
||||||
|
|
||||||
|
For a limited target, inspect not only the named files but also the directly affected callers, dependencies, interfaces, tests, configuration, and documentation needed to judge the target correctly.
|
||||||
|
|
||||||
|
## Preserve the workspace
|
||||||
|
|
||||||
|
Keep the review read-only unless the user separately asks for fixes.
|
||||||
|
|
||||||
|
1. Capture the initial branch, `HEAD`, and complete Git status, including untracked files.
|
||||||
|
2. Do not run commands intended to rewrite source or dependency files, including `go fmt ./...` and `go mod tidy`.
|
||||||
|
3. Do not fetch, pull, commit, stage, or discard changes.
|
||||||
|
4. Capture Git status again before reporting. If the workspace changed, identify the change and do not erase it.
|
||||||
|
|
||||||
|
## Establish context
|
||||||
|
|
||||||
|
Read `AGENTS.md`, `docs/roadmap.md`, `docs/architecture.md`, `docs/decisions.md`, and `docs/development.md` before judging the implementation. Treat documented decisions as intent, not as proof that their implementation is correct.
|
||||||
|
|
||||||
|
Map the reviewed packages, dependency direction, public entry points, state owners, external resources, and important lifecycle operations. Use `rg` and targeted file reads; do not read `go.sum` in full.
|
||||||
|
|
||||||
|
## Review objective
|
||||||
|
|
||||||
|
Find material, evidence-backed flaws in the repository's architecture, design, and implementation. Prioritize problems that can meaningfully harm correctness, security, resilience, data integrity, scalability, extensibility, decoupling, customizability, or maintainability. Favor systemic causes and cross-component interactions over local code smells.
|
||||||
|
|
||||||
|
Do not begin from a predetermined list of expected bugs. Reconstruct intended behavior, boundaries, invariants, state transitions, and trust assumptions from code, tests, configuration, architecture documentation, technical decisions, and the roadmap.
|
||||||
|
|
||||||
|
## Analyze in phases
|
||||||
|
|
||||||
|
1. Build a working system model before producing findings.
|
||||||
|
2. Perform an open-ended pass by tracing important control, data, state, ownership, resource, and failure flows across component boundaries.
|
||||||
|
3. Perform a coverage pass using the review lenses below.
|
||||||
|
4. Challenge each candidate by seeking evidence that the behavior is intentional, unreachable, contained, or already protected.
|
||||||
|
5. Classify and report findings only after discovery and falsification.
|
||||||
|
|
||||||
|
Treat these lenses as minimum coverage, not as an exhaustive checklist, required taxonomy, or finding quota:
|
||||||
|
|
||||||
|
- **Architecture and evolution:** Evaluate whether responsibilities, dependencies, boundaries, data flows, or resource use create material obstacles to supported behavior, realistic scaling, roadmap work, extensibility, decoupling, customization, or maintenance.
|
||||||
|
- **Security:** Evaluate whether trust, identity, authorization, ownership, confidentiality, integrity, availability, or exposure assumptions can be violated through a reachable execution path or component interaction.
|
||||||
|
- **Resilience:** Evaluate whether plausible partial failures, dependency failures, cancellation, concurrency, malformed inputs, or interrupted lifecycle operations can cause disproportionate impact, cascading failure, loss of service, or an unrecoverable state.
|
||||||
|
- **Data consistency:** Evaluate whether persistent state, external resources, ownership, and lifecycle transitions preserve required invariants across normal, concurrent, retried, interrupted, and partially failed execution paths.
|
||||||
|
|
||||||
|
Continue searching for material problems outside these lenses. Do not manufacture a finding for every lens.
|
||||||
|
|
||||||
|
## Use parallel review selectively
|
||||||
|
|
||||||
|
Keep small `diff` and narrowly targeted reviews in the main agent by default. For a broad `main` or `full` review, use read-only subagents when they are available and parallel review materially improves coverage.
|
||||||
|
|
||||||
|
Give every reviewer the complete, non-exhaustive review objective and one primary emphasis such as architecture and performance, security, or resilience and data consistency. Make the emphasis a starting perspective rather than an exclusive boundary. Wait for all reviewers, then independently verify, deduplicate, and rank their candidates in the main agent.
|
||||||
|
|
||||||
|
## Verify deterministically
|
||||||
|
|
||||||
|
Run the repository-wide checks below unless the user explicitly limits command execution or the environment prevents them:
|
||||||
|
|
||||||
|
- `go build ./...`
|
||||||
|
- `go vet ./...`
|
||||||
|
- `go test ./...`
|
||||||
|
- a non-mutating `gofmt -l` check over tracked Go files
|
||||||
|
|
||||||
|
Record each check and its result. When a test fails, inspect the test first and follow the debugging principles in `AGENTS.md`. Do not convert a command failure into an architecture finding without validating its cause. Put checks that cannot run and their reasons under `Unverified areas`.
|
||||||
|
|
||||||
|
## Filter and report
|
||||||
|
|
||||||
|
After discovering candidate findings, read [review-calibration.md](references/review-calibration.md) and apply its evidence and falsification rules. Before writing the final response, read [report-template.md](references/report-template.md) and follow it exactly.
|
||||||
|
|
||||||
|
Report a candidate only when another developer can verify its location, causal path, and material impact. Place unresolved hypotheses under `Unverified areas` with the evidence needed to resolve them. If no candidate qualifies, state `Findings: None` rather than lowering the threshold.
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
interface:
|
||||||
|
display_name: "MyGO API Repository Review"
|
||||||
|
short_description: "Evidence-based MyGO backend repository review"
|
||||||
|
default_prompt: "Use $mygo-api-repo-review to review the current working tree against HEAD."
|
||||||
|
policy:
|
||||||
|
allow_implicit_invocation: false
|
||||||
@@ -0,0 +1,52 @@
|
|||||||
|
# Review Report Template
|
||||||
|
|
||||||
|
Write the report in the language requested by the user; otherwise match the language of the review request. Preserve source identifiers, commands, file paths, and code symbols exactly.
|
||||||
|
|
||||||
|
Use category-based finding IDs:
|
||||||
|
|
||||||
|
- `ARCH`: architecture and evolution
|
||||||
|
- `SEC`: security
|
||||||
|
- `REL`: resilience
|
||||||
|
- `DATA`: data consistency
|
||||||
|
- `PERF`: performance and scaling
|
||||||
|
- `IMPL`: implementation correctness
|
||||||
|
|
||||||
|
Number IDs within the report, for example `SEC-001`. Assign one primary category to each root cause.
|
||||||
|
|
||||||
|
Use these severity levels independently of category:
|
||||||
|
|
||||||
|
- `Critical`: a reachable issue with catastrophic impact, such as broad authorization compromise, widespread sensitive-data exposure, irreversible systemic data loss, or reliable total-service failure.
|
||||||
|
- `High`: a realistic issue causing material authorization or ownership bypass, user-data exposure or corruption, service-wide outage, or a fundamental design failure that blocks supported behavior.
|
||||||
|
- `Medium`: a material but contained correctness, consistency, resilience, scalability, or maintainability failure with a plausible trigger.
|
||||||
|
- `Low`: a verified, bounded defect with limited impact. Do not use `Low` for style preferences or optional improvements.
|
||||||
|
|
||||||
|
Sort findings by severity, then by impact within the same severity. Security is a category, not a severity; a security finding can have any severity.
|
||||||
|
|
||||||
|
Use this structure:
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
- Summary
|
||||||
|
- Mode: diff | main | full
|
||||||
|
- Target: all | <path/package/component>
|
||||||
|
- Baseline: <commit or none>
|
||||||
|
- Reviewed snapshot: <HEAD and workspace state>
|
||||||
|
- Verification: <build, vet, test, and format-check results>
|
||||||
|
|
||||||
|
- Findings
|
||||||
|
- ID: <CATEGORY-NNN>
|
||||||
|
- Severity: Critical | High | Medium | Low
|
||||||
|
- Location: <filename:line>
|
||||||
|
- Analysis: <evidence, reachable causal path, and material impact>
|
||||||
|
- Recommendation: <root-cause repair direction>
|
||||||
|
|
||||||
|
- Architecture drift
|
||||||
|
- <material differences between documented and implemented architecture, referencing finding IDs instead of duplicating analysis>
|
||||||
|
|
||||||
|
- Documentation drift
|
||||||
|
- <documentation inaccuracies that can materially mislead implementation, operation, or review>
|
||||||
|
|
||||||
|
- Unverified areas
|
||||||
|
- <unverified area, reason, and required validation>
|
||||||
|
```
|
||||||
|
|
||||||
|
If there are no qualifying findings, write `Findings: None`. Write `None` for any other empty section. Do not omit verification failures or review limitations.
|
||||||
@@ -0,0 +1,27 @@
|
|||||||
|
# Review Calibration
|
||||||
|
|
||||||
|
Use this reference only after the open-ended discovery pass. Apply it to challenge candidate findings and suppress false positives.
|
||||||
|
|
||||||
|
## Finding eligibility
|
||||||
|
|
||||||
|
Accept a candidate as a finding only when all of these are present:
|
||||||
|
|
||||||
|
- Concrete evidence in current code or documented design.
|
||||||
|
- A violated invariant or reachable, plausible failure scenario.
|
||||||
|
- A material consequence under supported behavior or realistic roadmap use.
|
||||||
|
- A precise source location that lets another developer verify the analysis.
|
||||||
|
- A recommendation that addresses the underlying cause.
|
||||||
|
|
||||||
|
Actively seek the strongest disconfirming evidence before accepting a candidate. Check whether another layer enforces the invariant, the path is unreachable, the behavior is explicitly unsupported, or the tradeoff is intentional and adequately contained.
|
||||||
|
|
||||||
|
## Calibration rules
|
||||||
|
|
||||||
|
- Do not report a missing abstraction, restriction, validation, limit, test, or defensive mechanism solely because it is absent. Demonstrate the failure path or violated invariant caused by the absence.
|
||||||
|
- Do not treat a permissive configuration or documented tradeoff as a problem by itself. Report it only when a supported setting bypasses required processing or breaks security, lifecycle, or consistency invariants.
|
||||||
|
- Do not treat a simple implementation as an architecture flaw without concrete coupling, duplicated responsibility, boundary leakage, unsafe change propagation, or realistic scaling impact.
|
||||||
|
- Do not report theoretical performance limits without a reachable hot path, resource-growth mechanism, and material consequence under realistic use or roadmap requirements.
|
||||||
|
- Do not report style preferences, naming preferences, speculative future features, or optional hardening as defects.
|
||||||
|
- Do not weaken or reinterpret a valid test to remove an implementation failure. Confirm intended behavior before deciding whether the test or implementation is wrong.
|
||||||
|
- Do not duplicate one root cause across review lenses. Assign one primary category and describe secondary impacts in its analysis.
|
||||||
|
|
||||||
|
Place credible concerns that lack required evidence in `Unverified areas`. State what is unknown, why it matters, and the smallest validation needed to resolve it.
|
||||||
@@ -16,6 +16,12 @@
|
|||||||
5. Verify: `go vet ./... && go test ./...`
|
5. Verify: `go vet ./... && go test ./...`
|
||||||
6. Update `docs/roadmap.md`, `docs/decisions.md`, or `docs/architecture.md` if anything changed
|
6. Update `docs/roadmap.md`, `docs/decisions.md`, or `docs/architecture.md` if anything changed
|
||||||
|
|
||||||
|
## Repository Review
|
||||||
|
|
||||||
|
- Explicitly invoke `$mygo-api-repo-review` for repository-wide or scoped architecture, design, security, resilience, data-consistency, performance, and implementation audits.
|
||||||
|
- Repository reviews are read-only unless the user separately requests fixes.
|
||||||
|
- Use `diff`, `main`, or `full` review mode and optionally limit any mode with a target path, Go package, or logical component.
|
||||||
|
|
||||||
## Go Conventions
|
## Go Conventions
|
||||||
|
|
||||||
- **Format**: `go fmt ./...` before every commit
|
- **Format**: `go fmt ./...` before every commit
|
||||||
|
|||||||
@@ -26,6 +26,22 @@ go vet ./...
|
|||||||
go fmt ./...
|
go fmt ./...
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Repository Review
|
||||||
|
|
||||||
|
Use the repository-scoped `$mygo-api-repo-review` skill for evidence-based architecture, design, security, resilience, data-consistency, performance, and implementation audits. Reviews are read-only unless fixes are requested separately.
|
||||||
|
|
||||||
|
Select one review mode:
|
||||||
|
|
||||||
|
- `diff` compares the current workspace, including relevant untracked files, with `HEAD`.
|
||||||
|
- `main` compares the current workspace with the merge base of `HEAD` and the local `main` branch without fetching remote refs.
|
||||||
|
- `full` reviews the current repository as a whole.
|
||||||
|
|
||||||
|
Optionally limit any mode to a target path, Go package, or logical component. For example:
|
||||||
|
|
||||||
|
```text
|
||||||
|
Use $mygo-api-repo-review in main mode with target internal/storage.
|
||||||
|
```
|
||||||
|
|
||||||
## Dependencies
|
## Dependencies
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
Reference in New Issue
Block a user