Delegate workflow installation to the skills CLI

Replace per-tool distribution generation with a canonical committed skills build so installation and updates share one format.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
2026-08-20 13:42:31 -07:00
parent 4cbf426df2
commit 32d1487dcf
27 changed files with 6126 additions and 984 deletions
@@ -0,0 +1,191 @@
# Dimension Checklists
> Part of plan2code-review — loaded during Step 3 (Analyze).
Non-obvious check items, anti-patterns, and "don't flag" guidance for each review dimension. Focus on what LLMs commonly miss.
---
## 1. Correctness
1. Trace variable mutations through async paths — can a value change between check and use?
2. Check type coercion at boundaries — string-to-number, truthy/falsy assumptions on `0`/`""`/`false`
3. Check that map/filter/reduce callbacks handle all element shapes (null members, missing keys)
4. Verify copy operations produce deep copies when mutation independence is required
5. Check array bounds — does code handle empty arrays and out-of-range indices?
6. Verify regex patterns reject edge cases (empty string, special chars, Unicode)
7. Check for race conditions — shared mutable state across async reads/writes
8. Verify error handling paths return/throw correctly — no silent swallowing
**Anti-patterns:** `catch(e) {}` silent suppression · `if (value)` when `0`/`""`/`false` are valid · `indexOf > 0` instead of `!== -1` · Mutating arguments instead of returning new values
**Don't flag:** Intentional `==` for null coalescing (`if (x == null)`) · Missing switch default with TypeScript `never` exhaustive check · Optional chaining `?.` returning undefined
---
## 2. Completeness
1. Search for TODO, FIXME, HACK, XXX, TEMP — each must be intentional or tracked
2. Check cleanup/teardown exists for every setup/initialization
3. Verify rollback/undo logic for multi-step operations that can partially fail
4. Check event listeners and subscriptions have corresponding unsubscribe/cleanup
5. Verify all file/resource handles closed in both success and error paths
6. Check all enum/union type values have handling — no missing cases
7. Verify pagination — does code fetch all pages or just the first?
8. Check retry logic has backoff and maximum retry count
**Anti-patterns:** Happy-path-only functions that return undefined on error · Event listeners without cleanup · Missing else in critical chains where the "impossible" case can occur
**Don't flag:** TODOs with linked ticket numbers · Features explicitly "not in scope" · Bare `throw` in catch blocks (intentional re-throw)
---
## 3. Security
1. Check for injection — string concatenation in queries instead of parameterized
2. Search for hardcoded secrets, API keys, tokens in source files and configs
3. Verify sensitive data (passwords, tokens, PII) is not logged, even at debug level
4. Check file path operations prevent traversal — no unsanitized `../` from user input
5. Check for mass assignment — `Object.assign`/spread from user input without allow-list
6. Verify error responses don't leak stack traces, internal paths, or system info
7. Verify session tokens use cryptographic randomness, not `Math.random()`
8. Check dependency versions against known CVEs
**Anti-patterns:** Dynamic code evaluation with user-influenced input · Plaintext password storage · JWT `none` algorithm accepted
**Don't flag:** Hardcoded non-sensitive values (page sizes, route paths) · Internal tools documented as trusted-environment-only · Test fixture credentials in `.test.`/`.fixture.` files
---
## 4. Performance
1. Check for N+1 queries — loops issuing a query per iteration instead of batching
2. Check for synchronous blocking in async contexts (fs.readFileSync in server request handlers)
3. Check for memory leaks — growing arrays/maps without bounds, unclosed streams
4. Check for quadratic complexity — nested iterations over the same collection
5. Verify bulk operations used where available (bulk insert vs individual)
6. Check regex for catastrophic backtracking potential (nested quantifiers)
7. Verify connection pools for database and HTTP clients, not per-request connections
8. Check event handlers debounced/throttled for high-frequency events
**Anti-patterns:** `await` in `for` loops instead of `Promise.all` · Loading entire tables to filter in app code · New regex instances inside loops
**Don't flag:** `readFileSync` at startup/module load · Loading small files (<100KB) once · Missing caching in one-shot scripts/CLI tools
---
## 5. Standards
1. Check function length — functions over 50 lines warrant scrutiny
2. Verify DRY applied judiciously — shared logic extracted, not over-abstracted
3. Check magic numbers/strings are extracted to named constants
4. Verify comments explain "why" not "what"
5. Check file organization matches project conventions
6. Verify consistent async/await vs callbacks vs promises within a module
7. Check naming conventions consistent (camelCase, snake_case, PascalCase)
8. Verify linting rules not disabled without justification
**Anti-patterns:** Mixed naming conventions in one module · God objects/functions · Deep nesting (>3 levels) instead of early returns
**Don't flag:** Framework-imposed patterns · Single-use helpers for readability · Comments on complex algorithms/regex
---
## 6. Tech Debt
1. Check for deprecated API usage — verify against current library versions
2. Identify commented-out code blocks — should be removed or tracked
3. Check for copy-paste blocks that could be shared utilities
4. Look for dead feature flags — always true/false with no toggle
5. Check for orphaned files — modules with no imports from the codebase
6. Verify error messages reference current code, not stale names
7. Check for inconsistent abstraction levels — mixing orchestration with low-level ops
8. Look for workarounds with "temporary" comments that persisted
**Anti-patterns:** `TODO: remove after migration` with no date/ticket · Wrapper functions that just forward args · Multiple implementations of the same utility
**Don't flag:** Intentional per-platform duplication · Verbose code prioritizing clarity · Deprecated APIs with tracked migration tickets
---
## 7. Test Quality
1. Check assertions are meaningful — not just `toBeTruthy()` on objects
2. Verify tests are isolated — no shared mutable state between cases
3. Check error case tests actually trigger the error path, not just catch any error
4. Verify async tests properly await results — no fire-and-forget assertions
5. Check for tests that pass for wrong reason — wrong assertion target, always-true conditions
6. Verify mock/stub scope is minimal — only mock what's necessary
7. Check test descriptions describe behavior, not implementation
8. Verify critical paths have coverage — happy path, error path, edge cases
**Anti-patterns:** `expect(fn).not.toThrow()` without checking return value · Mocking the module under test · Assertions in callbacks that may never execute
**Don't flag:** Shared test utility files · Missing tests for generated/scaffolded code · Integration tests using real databases when project prefers it
---
## 8. Maintainability
1. Check cyclomatic complexity — functions with >10 branch paths are hard to maintain
2. Verify dependencies between modules are explicit, not implicit through globals
3. Check for tight coupling — can this module be tested independently?
4. Verify data transformations are traceable — can you follow a value from input to output?
5. Check file length — files over 500 lines warrant scrutiny
6. Verify similar operations use consistent approaches throughout
7. Check boolean parameters are replaced with enums or option objects for clarity
8. Verify function/variable names describe purpose without needing comments
**Anti-patterns:** Functions requiring implementation knowledge to call · Circular dependencies · God files · Stringly-typed APIs
**Don't flag:** Long cohesive files (comprehensive test suites) · Inherently complex domain functions · Coupling between genuinely related modules
---
## 9. Spec Compliance
1. Verify every acceptance criterion has corresponding implementation
2. Check file paths, names, directory structures match spec exactly
3. Verify implementation doesn't add undocumented behavior beyond spec
4. Check all spec-defined edge cases have explicit handling
5. Verify task completion state in phase files matches actual implementation
6. Check integration points match spec contracts
7. Verify function signatures match spec definitions
8. Check data formats match spec (JSON schema, file formats)
**Anti-patterns:** Implementing a "better" approach without raising the divergence · Assuming spec intent on ambiguous points · Marking tasks complete when implementation differs
**Don't flag:** Minor naming variations preserving intent · Defensive coding beyond spec · Details spec leaves to developer judgment
---
## 10. UX/DX
1. Verify error messages are actionable — tell user what to do, not just what went wrong
2. Check empty states have clear messaging — not blank pages or silent failures
3. Verify configuration has sensible defaults — zero-config produces working setup
4. Check breaking changes are communicated — deprecation warnings, migration guides
5. Verify CLI tools have `--help`, consistent flags, meaningful exit codes
6. Check bulk operations provide progress feedback and handle partial failures
7. Verify documentation examples are runnable, not pseudo-code
8. Check loading states exist for operations over 1 second
**Anti-patterns:** Stack traces shown to end users · Required config with no example file · 200 OK with error in body · Silent failures
**Don't flag:** Verbose debug output · Internal tooling with minimal polish · CLI tools requiring initial setup
---
## 11. Improvements
1. Check for manual implementations of standard library functionality
2. Identify error-prone patterns replaceable with safer abstractions
3. Look for synchronous operations that could be parallelized
4. Identify complex conditionals clearer as lookup tables or strategy patterns
5. Check for hardcoded limits that should be configurable
6. Look for opportunities to improve test coverage on critical paths
7. Identify documentation gaps — undocumented public APIs
8. Check for opportunities to use newer language features improving clarity
**Anti-patterns:** Rewrites for aesthetic reasons · Patterns from different ecosystems · Optimizations without evidence of problems · Premature DRY abstractions
**Don't flag:** Correct code written differently than you'd write it · Performance appropriate for actual load · Features tracked in backlog
@@ -0,0 +1,105 @@
# False-Positive Catalog
> Part of plan2code-review — loaded before presenting findings in Step 3 (Analyze).
Check every finding against this catalog before presenting it. These patterns represent common categories of false findings that waste reviewer and developer time. Each entry includes the bias, a concrete example, why it's wrong, and how to verify before flagging.
---
## 1. Optimization Bias
**Pattern:** Recommending removal or reduction of something to optimize a metric (context size, file count, memory, line count) when the thing being removed is necessary for correctness.
**Example:** Reviewer recommends removing `jira-api-catalog.md` and `jira-field-schema.md` from agent read loading to "reduce context overhead." The reviewer sees large files being loaded and assumes smaller context = better performance.
**Why it's wrong:** Agents consume these files at runtime to know which API endpoints exist and what fields are available. Without them, agents construct invalid API calls. The "overhead" is actually essential working knowledge. Optimizing for context size sacrificed correctness.
**How to check before flagging:**
1. Identify what metric you're optimizing (file count, context size, line count, memory).
2. Ask: "What happens if this is removed?" Trace the downstream impact.
3. Search for all consumers of the thing you want to remove — not just direct imports, but Read directives, config references, and runtime loading.
4. If any consumer depends on it for correct execution, the finding is invalid.
---
## 2. Simplification Bias
**Pattern:** Recommending consolidation or simplification of something that is intentionally structured for separation of concerns, different consumers, or independent evolution.
**Example:** Reviewer flags that `config/platforms/windsurf.json` and `config/platforms/cursor.json` have "similar structure" and recommends consolidating into `flat-file-platforms.json` to "reduce duplication."
**Why it's wrong:** The per-platform files exist deliberately. Each platform has distinct frontmatter requirements, path conventions, and feature flags. They share a common schema but contain different values. Consolidation would require conditionals everywhere and make per-platform changes harder. The separation is a design choice, not an oversight.
**How to check before flagging:**
1. Ask: "Why are these separate?" Read the commit history or documentation for context.
2. Check if the "duplicated" files serve different consumers or contexts.
3. Verify whether the files are expected to diverge further over time.
4. If separation serves independent evolution, different consumers, or different deployment targets, the finding is invalid.
---
## 3. Duplication False Alarm
**Pattern:** Flagging intentional redundancy as a DRY violation when the similar-looking code serves genuinely different purposes or domains.
**Example:** Reviewer flags that `handleTicketCreate()` and `handleTicketUpdate()` share 80% of their code and recommends extracting a common `handleTicketMutation()`. Both functions validate input, call the API, and format the response — but they use different validation rules, different API endpoints, different error messages, and different response transformations.
**Why it's wrong:** The structural similarity is coincidental. Each function handles a distinct domain operation with distinct requirements. Extracting a common function would create a complex conditional monster that's harder to maintain than two clear, self-contained handlers. DRY applies to shared knowledge, not shared structure.
**How to check before flagging:**
1. Compare the "duplicate" functions at the detail level, not the structural level.
2. Ask: "If I change one, should the other change identically?" If no, they're not duplicates.
3. Check if the functions handle different domain concepts, even if the code shape is similar.
4. If combining them would require conditionals or parameters to differentiate behavior, the separation is likely intentional.
---
## 4. Performance Theater
**Pattern:** Flagging operations as "too expensive" when the operation is not in a hot path, runs infrequently, or when the "cost" is actually necessary for correctness.
**Example:** Reviewer flags that the installer reads 15 platform configuration files at startup and recommends lazy loading. The installer runs once during setup, takes <200ms total, and needs all configurations to determine which platforms to install.
**Why it's wrong:** The installer is a one-shot CLI tool, not a server handling concurrent requests. Startup performance of a tool that runs once per install is irrelevant. The "optimization" would add complexity (lazy loading, caching, error handling for deferred loads) with zero user-visible benefit.
**How to check before flagging:**
1. Determine the execution context: Is this a hot path (server request handler, tight loop) or a cold path (startup, CLI command, migration script)?
2. Measure or estimate the actual cost. "Reads many files" is not a performance issue if total I/O is <1 second.
3. Ask: "Would a user notice the difference?" If the answer is no, the optimization is theater.
4. Check whether the "expensive" operation is necessary for correctness — if so, optimization means finding a faster way to do it, not skipping it.
---
## 5. Missing Context
**Pattern:** Flagging something as wrong, unused, or unnecessary because the reviewer didn't read all related files, packages, or documentation before forming a conclusion.
**Example:** Reviewer flags `export function formatJiraKey()` in `utils/jira.ts` as "unused export — remove or make private." The function is not imported in any file within the current package.
**Why it's wrong:** The function is consumed by a different package in the monorepo (`packages/cli/src/commands/jira.ts`). The reviewer only searched the current package directory, not the entire workspace. Cross-package consumption is common in monorepos and multi-package projects.
**How to check before flagging:**
1. Search the ENTIRE codebase for references, not just the current package or directory.
2. Check for dynamic imports, string-based requires, and config-driven module loading.
3. For exports: search all packages in the workspace, not just the current one.
4. For files: check build scripts, installation scripts, CI/CD configs, and documentation references.
5. If you can't find all consumers, state that explicitly rather than assuming there are none.
---
## Detection Shortcuts
Quick checks to run against any finding before presenting. If a check triggers, investigate further before flagging.
- **Does the finding recommend removing or simplifying something?** Trace who consumes it first. Search all packages, config files, scripts, and documentation for references. "I didn't find references in this file" is not "nothing references this."
- **Does the finding flag duplication?** Check if the "duplicates" serve different consumers, contexts, or domain concepts. Ask: "If I change one, must the other change identically?" If no, they aren't duplicates.
- **Does the finding flag performance?** Verify the operation is actually in a hot path. CLI tools, installers, migration scripts, and one-shot operations don't need the same performance treatment as request handlers.
- **Does the finding flag unused code?** Search ALL packages, consumers, and entry points — not just the current file or directory. Check for dynamic loading, Read directives, and cross-package imports.
- **Does the finding assume a different architecture?** Verify against AGENTS.md, README, and actual project conventions. The project may have deliberately chosen a pattern that differs from your preferred approach.
- **Does the finding flag complexity?** Check if the complexity maps to genuine domain complexity. Not all complex code is accidental complexity — some problems are inherently complex.
- **Does the finding recommend a "modern" replacement?** Verify the replacement is compatible with the project's runtime targets, platform constraints, and dependency policies. "Newer" is not always "better" for the specific context.
@@ -0,0 +1,18 @@
# Review — Session End Next-Step Routing
Loaded at the end of a review session to suggest what genuinely helps next.
Principles to reason from, not a lookup table — adapt; when a case doesn't fit
cleanly, say what you verified and ask.
- **Plan2Code Workflow Pipeline:** `/plan2code-1-plan``PLAN-*` files · `/plan2code-2-document``overview.md` + `phase-*.md` (the "spec docs") in `specs/<feature>/` · `/plan2code-3-implement` → checks off phase tasks, one phase per run · `/plan2code-4-finalize` → archives to `specs--completed/`.
- **Find specs (any OS/shell):** `specs/` is gitignored, and search tools (Glob/Grep/project search) skip gitignored paths on many platforms — an empty search result is not evidence either way. Check with a terminal listing: `ls specs/<feature>/` (bash/zsh) · `Get-ChildItem specs/<feature>` (PowerShell) · `dir specs\<feature>` (cmd). Feature dir unknown? List `specs/` first. Command errors? Try another shell's form, then read the expected files directly — file reads see gitignored paths. Conclude "no specs" only after a terminal listing or a failed direct read.
- **Reconcile three signals:** session context (what this conversation was doing — a fresh session may have none), user intent (what they asked reviewed), the disk check above. Disk wins on state; context wins on intent and on disk silence; no context → intent + disk decide.
- **plan2code artifact reviewed (a plan, the spec docs, phases) — route on the reviewed feature's own `specs/<feature>/` state (another feature's specs prove nothing here), to the earliest unmet stage, suggesting only a step whose input exists:**
- `PLAN-*` without `overview.md``/plan2code-2-document`
- `overview.md` without `phase-*.md``/plan2code-2-document`
- Unchecked `- [ ]` tasks in `phase-*.md``/plan2code-3-implement` (checkboxes are ground truth; flag overview conflicts)
- All phase tasks checked → `/plan2code-4-finalize`
- Archived spec → pipeline complete; summary only
- **Anything else** (code/PRs, logs, docs, tickets, emails, a codebase): no pipeline step — close with the summary; add a next action only if the review makes one obvious and actionable.
- **Gates:** unresolved Criticals → fixing them (H/A/S) is the next step. Signals the rules above can't reconcile, or multiple candidate specs → ask one targeted question.
- **Output:** "Next (NEW conversation): `/plan2code-<step>` — [why + how you know]"; otherwise "Review complete -- [summary]."
@@ -0,0 +1,72 @@
# Verification Protocol
> Part of plan2code-review — loaded during Step 3 (Analyze).
Apply this protocol to every finding before presenting it. Findings that fail verification are dropped. No exceptions.
## Code Finding Verification
1. **Re-read the source.** Open the file and read the actual line(s) cited. Do not rely on memory or earlier reads.
2. **Read surrounding context.** At least 20 lines above and below. Many "bugs" are handled by guards, defaults, or patterns in surrounding code.
3. **Verify the issue is real.** Trace the variable/function's actual usage. Is the edge case reachable? Does a try/catch or guard upstream already handle this?
4. **Verify the fix doesn't break callers.** Search all call sites. Check if any caller depends on the current behavior. Verify the fix maintains the function's contract.
5. **Check the test suite.** Do tests cover this case (making the "bug" intentional)? Would your fix break existing tests?
## Architectural Finding Verification
Architectural findings carry the highest false-positive risk.
1. **Identify all consumers.** Search the entire codebase — imports, requires, config files, build scripts, documentation, dynamic references (string-based requires, Read directives).
2. **Trace a concrete use case end-to-end.** Start at the entry point, follow execution through every module, document where the component you want to change is touched. If you can't trace a complete use case, you don't understand the system well enough to recommend changes.
3. **Simulate the change.** Walk the same use case with your change applied. At each step: can it still complete successfully? Pay attention to steps that load data, read config, or reference files.
4. **Check indirect dependencies.** Reference data loaded by agents at runtime via Read directives. Config consumed by external tools or CI/CD. Exports consumed by other packages.
5. **Verify the motivation.** Am I improving correctness, or satisfying an aesthetic preference? Is the complexity I'm flagging intentional?
## Design Finding Verification
1. **Check project conventions.** Read AGENTS.md, README, existing patterns. Is the "inconsistency" a deliberate choice?
2. **Simulate across all consumers.** Does the change improve their code, or just move complexity? Would it require coordinated updates across files?
3. **Evaluate migration cost.** Is the current design causing actual bugs, or merely suboptimal?
4. **Check platform constraints.** Does the recommendation work across all supported platforms?
## Confidence Calibration
| Level | Definition | Action |
|-------|-----------|--------|
| **High** | Verified in source code AND use-case traced (for arch/design). Issue confirmed real, fix confirmed safe. | Present. |
| **Not High** | Any doubt remains. | Investigate further or drop entirely. |
No Medium or Low tier. A finding is verified or it isn't.
## Adversarial Self-Check
Run against EVERY finding. If any question raises doubt, re-investigate or drop.
1. **Am I optimizing for the wrong metric?** Reducing file count, context size, or complexity when the structure serves correctness or platform compatibility?
2. **Does my fix remove something the system depends on?** Verified by searching ALL consumers, not just the current file?
3. **Have I traced a real use case end-to-end with my fix applied?**
4. **Am I recommending removal because I don't understand the purpose?**
5. **Would a domain expert disagree with this finding?**
## Escalation Rules
- **One question uncertain:** Re-investigate. If doubt persists, drop.
- **Two+ questions uncertain:** Drop entirely.
- **Cannot investigate:** Drop entirely. Note the gap in Step 4 if the dimension matters.
## Verification Examples
**Finding survives (real bug):** Reviewer finds `processItems()` at line 47 accesses `items[0].id` with no empty-array check. Re-reads source — confirmed no guard. Traces callers — `orchestrator.js:82` can pass empty array. Adversarial check passes (correctness issue, additive fix). Presented at High.
**Finding dropped (pcweb-jira false positive):** Reviewer recommends removing `jira-api-catalog.md` from consumer loading to "reduce context." Adversarial check: "Does my fix remove something the system depends on?" YES — agents Read these files at runtime to construct valid API calls. Four of five checks fail. Dropped.
**Finding dropped (false consolidation):** Reviewer flags per-platform config files as "duplicates." Adversarial check: "Am I optimizing for the wrong metric?" YES — platforms have distinct values and will diverge further. Separation is a design choice. Dropped.
## Review Discipline (Reinforcement)
These rules are restated here for reinforcement at analysis time — they are critical and must not drift:
- **Every finding must reference actual code at file:line.** Never fabricate. If you can't cite it, don't report it.
- **High confidence required.** No Low confidence findings. Investigate until verified or drop entirely.
- **Read every changed file completely before flagging.** Context missed = false positive generated.
- **Never silently narrow scope.** Review what was asked, not what's convenient.
- **Verify docs match code.** Documentation that contradicts implementation is a finding.