Specification-Compliance Review

Overview

Use this skill when an implementation is described as partial, passing, or complete and the task is to determine whether it actually satisfies an explicit specification. This is not a generic code review and not a re-run of the test suite. The central question is: does every normative requirement have both a correct implementation path and a meaningful proof?

A green build proves syntax, compilation, and perhaps the tested examples. It does not prove race behavior, cancellation semantics, exact backpressure policy, integration parity, or compliance with explicit non-goals.

Workflow

1. Establish the review baseline

Read, in order:

1. The task specification and surrounding plan/release gates.
2. Repository guidance (AGENTS.md, README, project instructions).
3. The changed files and their callers.
4. The tests added or claimed as evidence.
5. Git status/diff and recent relevant history.

Create a requirement matrix with one row per normative clause:

Requirement	Production evidence	Proving test	Execution result	Status
Exact behavior	path:lines	test name + fixture	command/output	pass/fail/unknown

Keep unknown distinct from fail. A test that could not run is not a passing test.

1B. Preflight implementation plans for load-bearing assumptions

When the artifact under review is an implementation plan rather than completed code, identify every external behavior that several downstream tasks assume: server-owned tool execution, plugin loading, request-context propagation, restart/reload ownership, stable host identity, or update/rollback semantics.

Do not let later RED tests encode an unproven API contract. Insert the smallest real-runtime spike before dependent work, capture the exact request/event/context contract, and gate downstream tasks on it. If the plan routes opaque identifiers through model-visible prompt text, require a deterministic metadata/session channel or explicitly stop for the missing platform capability; model copying is not a trusted transport.

Apply the plan's stated threat model before calling a security concern blocking. In particular, do not pretend Origin headers or current-user secret storage defend against hostile code already running as that OS user, and do not move a server-side secret into JavaScript merely to satisfy an authentication requirement.

1C. Trusted context and server-owned tool binding

For server-owned agent/plugin integrations, treat any identifier that selects a workbook, document, tenant, user, or session as security-sensitive request context. A UUID embedded in model-visible prompt text is only a hint: the model can omit, alter, replay, or substitute it. Require an out-of-band, server-recognized binding such as request metadata, a trusted header propagated by the gateway, a server-issued capability, or a validated context object. Trace the value through the actual server implementation and plugin handler; do not infer support from client tests or comments.

When a plugin receives a caller-supplied target ID, inspect whether plugin authentication authorizes the whole service rather than the specific target. If so, a wrong-but-valid ID can become cross-workbook or cross-tenant access. Add an adversarial test with two active targets: deliver a valid target A, make the model/plugin attempt target B, and assert the server rejects it or rewrites it to A before any side effect. Tests that only assert the ID appears in a prompt are vacuous for this requirement.

For OpenAI-compatible gateways, inspect the gateway source or run a real request spike to verify which body fields, headers, and tool schemas reach the native agent loop. Caller tools/tool_choice may be accepted for fingerprinting yet ignored for execution; similarly, custom context fields may be discarded. Record the exact supported transport before designing the client seam.

For localhost-only private deployments, audit proxy topology end-to-end: a loopback listener is not sufficient if its configured upstream host can be remote. Require loopback validation for the upstream as well, or classify the deployment as a separately documented remote mode; never send an injected gateway key over plain HTTP to an arbitrary API_SERVER_HOST.

When a review runs alongside another agent or an external process, capture the candidate SHA and review git show <sha>:<path> content if the worktree becomes dirty. Do not silently review concurrent edits as part of the candidate, and do not revert them without authorization.

2. Decompose the specification

Classify each clause as one or more of:

• functional behavior;
• concurrency or ordering invariant;
• cancellation/timeout behavior;
• integration compatibility;
• explicit scope boundary or non-goal;
• required test coverage;
• build/release gate.

Give special attention to words such as exactly, only, latest, must, never, preserve, cancellable, and no refactor. These are acceptance criteria, not descriptive prose.

3. Inspect implementation and seams

Trace each requirement from its public entry point through state ownership and asynchronous boundaries. For concurrency work, identify:

• who owns the worker/task;
• how pending work is represented;
• how generations/tokens identify stale work;
• who owns and cancels the transport;
• whether cleanup is identity-safe;
• whether stop awaits completion;
• whether callbacks can escape after cancellation;
• whether caller-created tasks can outlive the session.

After an extraction or refactor, compare the old path and new path for behavior parity. Common regressions occur in normalization, language mapping, configuration defaults, error formatting, and interruption handling rather than in the extracted core.

4. Audit tests for meaningfulness

A test proves a requirement only when its setup actually creates the required condition and its assertion observes the specified invariant. Reject vacuous coverage such as:

• a race test that releases the first operation before starting the second;
• an “active count” that measures only one callback rather than worker/transport lifetime;
• a timeout test that checks an error callback but not cancellation and eventual worker cleanup;
• a stale-generation test that does not inject a late frame after a new generation begins;
• a latest-wins test that checks the final text but not that the intermediate request was not synthesized;
• a cancellation test that stops after scheduling cancellation without awaiting the old worker.

Review test helper semantics too. An actor, lock, continuation, or deferred cleanup task can make a test appear deterministic while failing to measure the intended interval.

5. Verify in increasing strength

Run the smallest useful focused test first, then the complete relevant suite, then a clean build. Also run static searches for forbidden mechanisms or explicit non-goals (for example, semaphore-based blocking, direct credentials, or an unintended framework refactor).

Record command, target, destination, and result. If infrastructure prevents execution, report the exact limitation and downgrade the verification status; do not convert a prior passing claim into independently verified evidence.

6. Report findings

Lead with the verdict: PASS, PASS WITH GAPS, or FAIL.

For every finding include:

• severity;
• title;
• exact file and line range;
• requirement violated or left unproved;
• concrete execution scenario;
• why the current test does not catch it;
• smallest corrective direction (without implementing unless asked).

Then include:

• verified-good controls;
• verification commands and real output;
• unverified items and environmental limitations;
• scope/non-goal compliance;
• a short residual-risk summary.

Do not bury a failed acceptance criterion under a long list of passing tests.

Concurrency-specific checklist

For a single-worker latest-pending pipeline, prove all of the following independently:

• no more than one worker and one transport exist during normal handoff;
• a busy request retains exactly one newest pending request;
• intermediate pending requests are not silently synthesized or sent;
• worker exit cannot strand a request enqueued during handoff;
• old cleanup cannot cancel or clear a newer transport;
• stop invalidates the generation before late callbacks are processed;
• send timeout cancels the transport and reports promptly;
• first-audio timeout cancels a receive that ignores task cancellation;
• stop awaits the old worker when the contract requires it;
• session interruption/output interruption reaches the same cancellation path;
• provider integration preserves prior wire normalization and does not enqueue stale work after reconnect.

Pitfalls

• RED evidence must be exact, not just nonzero. A suite that shows 14 failed is not meaningful unless every failure proves the corresponding requirement was violated for the right reason. Inspect the first error message, stack, and assertion for each failing test. A test that fails because _verify_active_hermes_plugin does not exist (AttributeError) is exact RED — it proves the verify path cannot run at all. A test that fails with an unrelated import error or a vacuous assertion is not meaningful RED. Record the concrete failure messages alongside the count.
• When RED tests are run against a baseline before the fix, verify that the passing tests in that same baseline are not false positives. A passing test in a pre-fix RED run that asserts behavior the baseline does not implement is a false-positive pass, not meaningful GREEN.
• Do not equate “build succeeded” with “specification satisfied.”
• Do not accept a test by name; inspect its synchronization and assertion timing.
• Do not review only the extracted class; inspect every caller and lifecycle edge.
• Do not call a requirement failed solely because a simulator is unavailable; call it unverified, then distinguish any source-level defect separately.
• Do not propose broad fixes before identifying the exact requirement, data flow, and missing proof.
• Do not report speculative security or product concerns as compliance failures unless the specification makes them normative.
• Call out scope drift separately: if changed files include out-of-spec artifacts (for example, scheme metadata, generated project knobs, or tooling configuration) with no explicit task requirement, record them as out-of-scope configuration drift rather than silently inheriting them into the implementation path.

Provider-session safety addendum

For provider-backed audio/TTS sessions, treat these as separate acceptance criteria rather than one generic “cleanup” requirement:

• Caller-task safety: track provider-created enqueue tasks that can outlive a callback; stop must cancel and await them, and reconnect must explicitly reset the tracker before accepting new work.
• Language normalization parity: normalize language tags at the provider seam (trim, lowercase, map -/_ regional forms to the base language, then apply an allowlisted fallback) before constructing wire requests.
• Timeout shutdown: a timeout must complete the caller promptly, cancel the underlying transport, and await cleanup where the lifecycle contract requires it—even if the operation ignores task cancellation.
• Cleanup-race proof: gate old transport cleanup, start the new transport first, then release old cleanup; assert the new transport is neither cancelled nor cleared.
• True global one-active proof: instrument every transport instance and measure its active interval, not just one worker callback. Assert the maximum across the whole handoff lifecycle is one.

When validating Swift concurrency implementations, compile after each lifecycle change. Actor-isolated helper calls require await; deferred tracker updates and fire-and-forget cleanup can make a green-looking test observe the wrong interval. Prefer deterministic await-based helper accounting.

A focused iOS verification should use an actually available simulator destination and then run the complete test target. Record the exact destination and distinguish “unverified due to unavailable destination” from a source or test defect. A generic destination such as generic/platform=iOS is valid for compilation but not for XCTest execution; discover a concrete simulator with xcodebuild -project <project> -scheme <scheme> -showdestinations (or xcrun simctl list devices available) and rerun tests against that destination. If xcrun reports it cannot find simctl, re-point command-line tooling with sudo xcode-select -s /Applications/Xcode.app/Contents/Developer (or the active Xcode bundle path) and retry discovery before declaring the test layer unverified. If no concrete destination is available, report tests as unverified rather than treating a different device as equivalent.

When validating compile-time release/debug policy, do not rewrite both policy tests to assert the current configuration. Keep configuration-specific assertions honest: use separate Debug and Release build/test invocations, or expose a pure injectable policy factory for unit tests while retaining #if DEBUG as the production compile-time gate. A test named for Release must not pass merely because it was compiled in Debug.

Milestone acceptance versus whole-product completion

When development is staged, never promote a milestone-level PASS into a whole-product verdict. A tested native launch, clean Git synchronization, or verified shortcut proves only that the accepted artifact runs; it does not prove that later milestones, prototype controls, user-reachable CRUD, settings, packaging, or persistence paths exist.

Before declaring a milestone-built product complete:

1. Inventory every normative specification clause and meaningful prototype control.
2. Trace each through UI, controller, native/backend seam, persistence, relaunch, tests, and native evidence.
3. Report milestone acceptance and whole-product acceptance separately.
4. Treat user-confirmed expectations that resolve specification ambiguity as explicit requirements.
5. If the user chooses phased execution, produce only the requested bounded phase and stop at its gate; do not resend the entire campaign.

Support files

• references/spec-compliance-review.md — reusable requirement matrix, adversarial test audit, and async pipeline race patterns.

• references/amended-commit-rereview.md — spec rereview of an amended commit: prior-finding disposition table, delta baselines, native closeout gate as separate verification layer.

• references/milestone-vs-product-completeness.md — distinguish accepted milestone scope from whole-product completeness and structure bounded phase handoffs.

• references/typescript-validator-adversarial-review.md — hidden-own-property, cap-proof, deterministic-ordering, and content-free-error probes for unknown-input validators.

• references/load-bearing-assumption-spikes.md — pre-implementation plan review: turn unproven runtime/API assumptions into gated spikes and apply explicit threat-model boundaries.

• references/cross-repository-contract-reconciliation.md — reconcile producer/consumer health, auth, lifecycle, ownership, and script contracts; reject dead test helpers and wrapper-exit acceptance.

• references/start-stop-generation-race.md — session-specific pattern for rejecting queued provider/audio callbacks from stale lifecycle generations.

• references/late-callback-generation-gating.md — session-specific pattern for rejecting queued provider/audio callbacks from stale lifecycle generations.

• references/ios-simulator-triage.md — concrete destination discovery + simctl/toolchain triage flow for simulator-based verification.

• Treat instrumentation scope as part of the requirement: a counter around individual send() calls does not prove a transport- or worker-lifetime invariant. For a true one-active proof, enter when each transport/worker is created, leave only after its receive loop and cleanup have completed, and assert the maximum across the entire handoff.

• When a test claims to prove cancellation-resistant cleanup, inspect the fake transport's cancellation semantics and wait for both cancellation and worker completion. A test that only observes an error callback or a single cancellation flag can miss leaked work.

• Before running project-specific verification, confirm the project root and target path from repository guidance or discovery. If a command fails because it was launched from the wrong directory, retry from the actual project directory and report the corrected command/result rather than treating the first failure as an implementation limitation.

• Distinguish source correctness from proof completeness: a lifecycle helper may look race-safe in code while still requiring direct adversarial tests for add-during-stop, reset-after-stop, self-removal, and delayed cleanup.

Provider enqueue-task adversarial pattern

When a provider uses a task bag to bridge a synchronous callback into an async pipeline, review the bag as a lifecycle component rather than as incidental bookkeeping. The proof should cover the linearization point of add versus stop: once stopping begins, a later add must be rejected atomically; a task already accepted must be cancelled and awaited; reset must create a clean generation. An actor is usually the clearest ownership model because the stop/add/reset decisions serialize naturally without manually reasoning about a lock around task creation.

For transport pipelines, cleanup belongs before worker completion. Do not clear the current transport or release the worker slot before cancel() has returned; otherwise a handoff can construct a second transport while the first is still alive. The strongest regression test gates the old transport's cancellation, starts enqueue for the next request, asserts no second transport has been created, opens the cleanup gate, then asserts the next transport becomes active and the global maximum active lifetime is one.

Integration-review lessons from iOS provider pipelines

For a provider extraction that passes focused concurrency tests, perform one final caller-seam audit before declaring approval:

1. Trace every lifecycle requirement back through the public provider and ViewModel, not only the extracted pipeline. In particular, verify that output interruption, session stop, provider failure, and audio-engine shutdown all reach the same cancellation path when the specification requires it.
2. Distinguish implemented and tested, implemented but untested, and not wired. A green pipeline suite proves the seam in isolation; it does not prove that the production integration emits or consumes the relevant event.
3. Check project hygiene as part of integration quality: duplicate/unreferenced source files, generated project drift, and untracked artifacts should be reported even when the build succeeds. A passing Xcode build can still include an accidental duplicate file outside the target.
4. Run both the generic unsigned device build and the actual simulator test destination, recording the exact destination and test count. Treat static searches such as DispatchSemaphore and secret-pattern scans as separate evidence, not as substitutes for runtime tests.

When a broad requirement such as “output interruption cancels TTS” has no provider event path, report it as an unproven integration gap unless source tracing demonstrates a concrete defect. Do not overstate a gap as a failing acceptance criterion without an observable execution path.

Session review template: iOS mode/job clarity + localization-harness audits

For iOS translator-style reviews where the feature spans consumer-job UX, localization catalogs, and lifecycle behavior, follow this sequence before final verdict:

1. Read ios/AGENTS.md plus the relevant feature doc (README, OPERATOR_GUIDANCE, and any active .md plan for the feature).
2. Confirm the review scope from git: current branch, recent commits (for example, the feature head commit and any immediate follow-up), and git status cleanliness.
3. Inspect ConsumerOnboarding, LiveInterpreterView, TranslatorViewModel, localization catalog(s), and their tests together; never review one file family in isolation.
4. Add a requirement matrix row for each normative promise in onboarding copy, routing, direction selection, persistence, and release policy.
5. Run at least one targeted test set for changed contracts and one broader integration test set (same toolchain/simulator session) and report exact counts.
6. For iOS simulator evidence, use a concrete destination and include the exact destination string in the evidence.

If a concrete simulator destination is unavailable, explicitly report that subset as unverified (environmental) and continue with source-level proof for the remaining requirements.

Specific pitfalls for this domain

• A passing full test suite alone does not prove every user-facing onboarding promise. Confirm semantic gaps in the matrix:
• onboarding routing (welcome → consent → permission → job select → route guidance → session)
• persistence restore after relaunch
• policy/availability affordance in Release-visible flows
• reset safety while a session is starting/stopping
• Treat simulator-only audio HAL warnings as non-fatal noise unless assertions are failing; report separately as “environmental noise” so they do not become false negatives.
• For localization/catalog work, ensure Localizable.xcstrings changes preserve parity and that the completeness suite is run with the updated catalog.
• In review findings, separate implemented-but-untested from confirmed defects and from not-wired integration paths.

Record this as a short post-review note in (or equivalent) with:

• tested commands + outcome,
• matrix snapshot,
• top 3 unproven items (if any),
• and any repo-config or generated-file risks that remain.

Consumer onboarding and UX review addendum

For onboarding plus consumer-facing UX changes, audit the feature as a complete launch-to-first-session path rather than approving the coordinator and unit tests in isolation. Build a matrix for each normative UX promise and trace it through persistence, app-root routing, the completed consumer surface, and the ViewModel/provider seam.

Prove separately: