deepseek-harness/.agents/skills/dsh-code-review/SKILL.md

name, description

name	description
dsh-code-review	Use when reviewing a pull request in the deepseek-harness repo — orients the reviewer to this codebase's standards (AGENTS.md conventions, defensive patterns, ADRs, quality gates) and the review-specific checks that code alone can't show

Reviewing a DeepSeek-Harness PR

This skill is guidance, not a complete checklist. Read the diff against the PR's current base and enough surrounding code to understand the design, then verify suspected defects before reporting them. Re-establish that base after a retarget or merge. Prioritize correctness, lifecycle, security, and contract failures over style; a short review with one substantiated blocker is better than a list of nits.

Sources of truth

• AGENTS.md and packages/AGENTS.md: repository and package rules.
• docs/defensive-patterns.md: subprocess, callback, async-state, and disposal bug classes.
• docs/AGENTS.md: documentation placement and prose discipline.
• dsh-prose-standard: required coverage and editorial judgment for comments, docs, prompts, and visible strings.
• docs/testing.md and the quality-gates RFC: required test tiers and gates.
• RFC index: design rationale. Treat disagreement with an RFC as a design discussion, not an automatic veto.
• For bilingual changes, read translation-rules.md, terminology.md, and dsh-translate-docs.

Blocking requirements

1. New prose receives semantic review. Use dsh-prose-standard to critically review every added or changed Markdown passage, JSDoc, comment, prompt, description, diagnostic, and visible string. Verify required coverage, accuracy, placement, and editorial quality against the owning code or behavior; automated checks do not establish those properties.
2. Docs match the code. Config, defaults, errors, wire fields, events, and public behavior update the package README and JSDoc in the same diff. Comments state non-obvious contracts; flag implementation narration, test walkthroughs, review history, and duplicated rationale for deletion or a link to their one home.
3. Core type docs match. Changes to spine or seam vocabulary update the appropriate core-data-structures page and any type-equiv entry. Internal types need no catalog entry.
4. Registrations clean up. A new registry contribution has a test that disposes its owner and observes removal.
5. Required gates pass. Trust the current readiness sequence and pnpm run check:pre-push for their enforced inventory; review the semantic gaps they cannot detect.

Manual checks

• Intent and seam contracts: trace both sides of every changed interface. Confirm the implementation matches the PR and any RFC, including errors, cancellation, ownership, and disposal.
• Lifecycle and concurrency: for async setup, callbacks, processes, or teardown, apply defensive-patterns.md. Check races before publication, cancellation during awaits, independent error reporting, callback containment, ownership before reentry, complete detach cleanup, and quiescent disposal.
• Capability shape: a swappable capability follows the interface / implementation / consumer split. Consumers depend on the interface, not a backend.
• Scope, ownership, and necessity: tie each abstraction, state machine, option, defensive copy, and compatibility path to a current contract or production consumer. Challenge unrelated features, speculative generality, and behavior placed outside its owning plugin or service.
• Configuration: deployment-varying timeouts, caps, models, URLs, paths, and retry counts are validated Config fields, not literals or DEFAULT_* constants.
• Enforcement boundaries: hidden schema fields, filtered prompts, facades, wrappers, and listener ordering are not authoritative enforcement when direct or alternate callers can bypass them. Exercise denial paths at the boundary that actually executes the operation.
• Borrowed and derived state: determine whether retained caller-owned values are borrowed or snapshotted by contract; do not demand copies at typed same-process seams. Materialize mutable values that cross queues, model/tool JSON, durable logs or files, workers, processes, or wire boundaries. Commit notifications and derived state only at the documented success boundary, and trace caches, prompts, UI echoes, replay, and query views to one authoritative source.
• Bounds cover the final operation: verify byte, token, item, and time limits at the boundary that owns the complete emitted or retained result, including wrappers and metadata. Probe tiny limits, exact thresholds, oversized single chunks, and multibyte text for byte limits.
• Real entry path: tests exercise the shipped Loader, bin, worker, ACP bridge, or subprocess where relevant. A hand-mounted plugin does not catch Loader export-shape failures; a function plugin must named-export its namespace and have no default export.
• Test strength: assertions fail on the intended regression and verify external state, logs, events, or disposal rather than restating the implementation or trusting an agent's report. Coverage is necessary but not evidence that the scenario is correct.
• Changed checks have a negative control: a new automated check, or a changed acceptance path in one, has a deliberately invalid case that reaches the real top-level runner and fails for the intended rule; a green happy path does not prove the check is wired.
• Implemented RFCs match shipped reality: when a PR implements a proposed RFC, move and rewrite it as present-tense shipped state in the same diff, then verify paths, names, and mechanisms against the implementation.
• Transcript changes: editor-visible or model-visible changes update snapshots or explain why no snapshot applies. Review golden diffs as behavior changes, not formatting noise.
• Bilingual changes: compare meaning and terminology on both sides; a green pairing hash does not prove translation quality.

Reporting findings

State the defect, location, impact, and evidence. Separate blockers from suggestions and omit issues already enforced by a green gate. Use the existing GitHub review thread for replies. When receiving review, verify each claim and fix or rebut it on technical grounds without performative agreement.