killcritic-complete-review

name	killcritic-complete-review
description	Exhaustive adversarial review for high-risk architecture plans, code changes, SDKs, installers, updaters, FFI/C ABI, CI, packaging, migrations, security, and release systems. Use when missing one blocker could cause data loss, a trust downgrade, a broken build/package, a false-green CI result, or an irreversible public API decision. Do not use for quick style-only reviews.

KILLCRITIC Complete Review

Mission

Maximize independent blocker recall, not speed or comment count.

A successful review finds materially different failure classes across behavior, security, build, packaging, platform integration, CI, release, and operational safety. It does not stop after finding the first severe issue.

Answer in the user's language. Keep code identifiers, paths, symbols, and command flags exact.

Non-negotiable rules

1. Treat the current implementation and executable tests as the source of truth. Treat plans, examples, trackers, and docs as claims to verify.
2. Remain read-only unless the user explicitly asks for changes.
3. Do not stop after finding a P0/P1. Complete every applicable review domain.
4. Do not issue a final verdict until the coverage ledger is complete.
5. Every finding must include concrete evidence: file and line, command output, symbol, schema field, target, package asset, or test registration.
6. Separate verified facts from inferences. Label unverified surfaces explicitly.
7. Never convert “not inspected” into “looks fine.”
8. A green command is not proof that tests ran. Verify test discovery/count and meaningful assertions.
9. A package declaration is not proof of consumability. Verify the produced artifact and a real external consumer path.
10. A checksum is not authenticity. Trace the complete trust chain.
11. A path derived from the running executable is not automatically an installation root.
12. Examples may only document APIs and symbols that exist in the proposed completion state.
13. Continue searching for contradictory evidence after forming an initial verdict.
14. Do not inflate the report with style nits. Prefer fewer, independently material findings.

Review inputs

Determine these before the deep review. Infer them from the repository when possible instead of interrupting the user:

• Review object: plan, diff, branch, PR, codebase, release, or package.
• Baseline: current version/tag/branch and promised behavior.
• Declared platforms, languages, package managers, consumers, and architectures.
• Privilege level and destructive capabilities.
• Trust boundaries and remotely supplied inputs.
• Public API, ABI, schema, CLI, package, and compatibility promises.

If a baseline cannot be established, report that as a review limitation and use the most concrete available implementation as the comparison point.

Required workflow

Pass 0 — Repository instructions and baseline

• Read repository instruction files such as AGENTS.md and relevant nested variants.
• Identify the baseline version and recent behavior that the plan must preserve.
• Inspect release notes, schema/model code, packaging code, installers, platform adapters, and relevant tests.
• Check whether the plan or tracker is stale relative to files already present.

Pass 1 — Broad surface inventory

Build a surface map before deeply following any single hypothesis:

• Plans and child plans.
• Runtime/core implementation.
• Public APIs and examples.
• Platform adapters.
• Build systems and target graph.
• Schema serializers/parsers and fixtures.
• Package manifests and publication workflows.
• CI jobs, test registration, and sample consumers.
• Installer/updater helpers and privileged/destructive operations.

When a local repository is available, run the bundled read-only inventory script:

python3 <skill-directory>/scripts/audit_repo_surface.py \
  --repo "$PWD" \
  --plan <plan-path-if-any> \
  --format markdown

Treat script output as leads, not findings. Verify each signal in source.

Pass 2 — Claim-to-code trace

For every material plan claim, example, or completion criterion:

1. Locate the implementing symbol or proposed stage that creates it.
2. Locate the build/package path that exposes it.
3. Locate the consumer that loads or calls it.
4. Locate the test that proves the consumer path.
5. Verify all prerequisites occur before the claim is documented as available.

Flag claims with no implementation stage, no exported artifact, no consumer path, or no executable test.

Pass 3 — Mandatory domain sweeps

Complete every domain in references/coverage-matrix.md. Use one of exactly four statuses:

• FINDING
• VERIFIED_NO_BLOCKER
• NOT_APPLICABLE
• NOT_VERIFIED

Each status needs one sentence of evidence. NOT_VERIFIED in a high-risk domain prevents a confident GO verdict.

Pass 4 — Contradiction sweep

Search specifically for pairs that cannot both be true:

• Static target vs runtime-loaded shared library.
• Declared directory layout vs add_subdirectory path.
• Wrapper request/response fields vs native struct fields.
• Schema fixture types vs parser types.
• Public example vs implemented symbol.
• Package manifest vs files included in the package.
• Test option default vs CI invocation.
• “Tests passed” vs zero discovered tests.
• Helper-only scope vs full-runtime documentation.
• Executable path vs deployable application root.
• Current baseline behavior vs extraction inventory.
• Tracker checkboxes vs files already created.

Do not rely only on identical names. Search aliases, renamed fields, inverse conditions, and language-specific representations.

Pass 5 — Negative-space sweep

Ask what must exist for the proposal to work but is not mentioned:

• Symbol exports and calling convention.
• ABI versioning and ownership/lifetime rules.
• Architecture/RID assets.
• Install/export targets and package config files.
• External consumer samples.
• Dependency provisioning.
• Code signing/notarization.
• Rollback and interrupted-install recovery.
• Trust policy and key rotation.
• Protected-root, symlink, and path traversal defenses.
• Migration and compatibility behavior.
• Distribution workflows and release provenance.

Absence is a finding only when the omitted item is required by a concrete claim or consumer path.

Pass 6 — Independent fresh pass

Perform a fresh review in a different order without using the initial finding list as the outline:

1. Start from consumers and work backward to package/build/source.
2. Start from dangerous effects and work backward to validation/trust inputs.
3. Start from CI commands and work backward to test registration.
4. Start from public examples and work backward to implementation stages.

The purpose is to defeat anchoring. Add newly found issues even if they do not fit the first-pass narrative.

When independent subagents are available, assign at least these isolated scopes before merging results:

• Safety/trust/destructive operations.
• Build/link/package/consumer reality.
• Behavior/schema/API/compatibility parity.
• CI/release/tracker consistency.

Do not show one subagent another subagent's findings until its own pass is complete. When subagents are unavailable, perform the scopes serially with separate ledgers.

Pass 7 — Finding validation

Before reporting each finding:

• Re-open the cited source.
• Confirm the line/symbol still supports the claim.
• State the concrete failure mode.
• State why existing tests or later stages do not already prevent it.
• Give the smallest sufficient remediation or missing gate.
• Remove duplicates and symptoms whose root cause is already reported.

Trigger-specific hard checks

Native SDK, FFI, C ABI, P/Invoke, Swift/C++ interop

Verify all applicable items:

• Static/shared/object target split matches each consumer.
• Exported symbols, visibility, calling convention, and library name match the wrapper.
• C ABI has an explicit compatibility/versioning strategy.
• Struct size/version, alignment, strings, arrays, ownership, callbacks, and error lifetime are defined.
• Every request and response field is preserved across every language boundary.
• Build paths exist relative to the file that declares them.
• Architecture and runtime identifiers map to actual packaged binaries.
• Runtime loading is tested, not only managed wrapper compilation.
• cmake --install plus external find_package consumer is tested when CMake publication is promised.
• pkg-config consumer is tested when Linux publication is promised.
• NuGet contains correct runtimes/<rid>/native assets and runs a real P/Invoke smoke test.
• SwiftPM is consumed from an external package/app.
• CocoaPods fallback compiles a real Flutter/macOS consumer and has correct source/dependency/module visibility.
• Public constructors/initializers and symbols are visible across module boundaries.

Updater, installer, migration, and destructive filesystem operations

Verify all applicable items:

• Install root is explicit, canonicalized, validated, and distinct from executable-parent heuristics.
• The executable path is relative to the validated install root.
• Package/application identity binds the request to the intended installation.
• Protected roots and known system directories are rejected.
• Symlinks, junctions, mount boundaries, path traversal, case-folding, and canonicalization are handled.
• Deletion is constrained to an allowlisted install tree.
• Staging ownership and provenance are verified.
• Interrupted update, partial copy, rollback, and recovery behavior are defined.
• Removed-file semantics survive all wrappers and package formats.
• First release scope explicitly limits supported installation layouts when general safety is not implemented.
• Remote metadata authenticity is verified, not only artifact length/hash.
• Signature policy, canonical serialization, key ID/rotation, index-to-descriptor binding, and artifact binding match the existing trust model.
• Version/build/platform/channel, rollout, minimum updater/OS, support policy, and fresh-install semantics preserve baseline behavior.
• Each artifact/install strategy has explicit platform trust checks.

Load references/native-sdk-updater-regressions.md for additional historical failure patterns. Treat them as hypotheses to verify, never as assumed current defects.

CI, tests, release, and publication

Verify all applicable items:

• Native/optional test flags are explicitly enabled by CI.
• Test dependencies are provisioned in standalone builds.
• Test discovery count is asserted to be greater than zero.
• Package tests consume installed/published output rather than source-tree internals.
• Samples are built and executed when a smoke test is promised.
• Release jobs produce every promised architecture/package asset.
• Signing, notarization, checksums, provenance, and upload steps are executable, not documentation-only.
• ABI/glibc/deployment-target baselines are declared and tested.
• Version comes from the declared single source of truth and is propagated into each package.

Severity

Use impact, not effort, to assign severity.

• P0 / BLOCKER: plausible data loss; privilege or trust-boundary failure; authenticity downgrade; build/package impossible for a promised primary consumer; CI can falsely certify the critical path; irreversible public contract contradiction.
• P1 / MAJOR: important behavior or field is lost; supported platform path is broken; package is not externally consumable; baseline compatibility is materially incomplete; release gate omits a promised artifact.
• P2 / MODERATE: localized correctness, maintenance, or test weakness that does not invalidate the architecture or primary release path.
• P3 / MINOR: low-risk clarity or hygiene issue. Usually omit these in a KILLCRITIC report.

A long list of P2/P3 items must not obscure P0/P1 items.

Verdict rules

• BLOCK / NO-GO: any validated P0, or multiple P1 issues that make the plan non-executable or unsafe.
• CONDITIONAL: no P0, but material P1 gates remain and are precisely bounded.
• GO: all high-risk domains are verified, no P0/P1 remains, and promised consumer/release paths have executable evidence.

Never issue GO when a relevant security, destructive-operation, package-consumer, or CI-truth domain is NOT_VERIFIED.

Required final format

Use this order:

1. KILLCRITIC verdict — one sentence.
2. Blockers and major findings — ordered by severity and impact, each with evidence, failure mode, and required correction.
3. Coverage ledger — every mandatory domain and status; keep it compact but complete.
4. What remains sound — only after blockers.
5. Recommended execution order — dependency-ordered, not document-order.
6. Limitations / unverified surfaces.
7. Repository modification statement — say whether files were changed.

Do not claim exhaustiveness. State that the workflow reduces omission risk but cannot guarantee that no defect remains.

Learning from misses

Whenever another reviewer, production incident, or later pass finds a material issue this skill missed:

1. Add the failure pattern to references/native-sdk-updater-regressions.md or a new domain reference.
2. Add a prompt and expected category to evals/prompts.csv.
3. Add a must-detect rule to evals/rubric.md.
4. Re-run the old and new cases.
5. Do not weaken an existing must-pass rule merely to improve speed.

The regression corpus is part of the skill, not optional documentation.