path: root/.ai/workflows/spec-review.org

#+TITLE: Spec-Review Workflow
#+AUTHOR: Craig Jennings
#+DATE: 2026-05-23
#+STARTUP: showall

* Overview

The spec-review workflow evaluates a feature/specification document before implementation and decides one thing: can an engineer implement it confidently, test it thoroughly, and ship behavior that matches the user's mental model? If yes, say so and stop. If no, record every blocking gap and the concrete change that closes it as findings in the spec's own =* Review findings= section.

This is the *reviewer* side of a pair. Its counterpart is the spec-response workflow, which the spec's author runs to disposition the findings. The contract between them lives *in the spec*: a =* Review findings= section carrying one =TODO= task per finding, with a =[/]= cookie — the same shape the spec's =* Decisions= section already uses. spec-review writes the findings; spec-response completes them. No separate review file is written, so nothing dangles when a review is processed and the full review/response trail stays in the spec.

The goal is not to prove the spec is clever. It is to leave the implementer with *fewer* hidden decisions, not more prose.

* Problem We're Solving

Without a disciplined review before implementation:

- *Specs look finished but force implementers to invent product behavior.* Ownership, conflict, empty-state, and error semantics get discovered (and decided ad hoc) mid-implementation, where they're expensive to change.
- *API assumptions ship unverified.* A spec quietly depends on a field/mutation/enum shape that turns out wrong, and the work stalls at integration.
- *Reviews are vague or sprawling.* "Add tests," "consider edge cases," or a full rewrite of the spec — none of which an implementer can act on.
- *Deferred decisions live only in chat.* The next session re-litigates them.

*Impact:* viability and consistency problems that should surface in review instead surface during implementation, when they cost the most.

* Exit Criteria

A review is complete when:

1. *The implementation-readiness gate has been evaluated* and a rubric label assigned (=Ready= / =Ready with caveats= / =Not ready= / =Needs research=).
2. *If ready:* the user is told plainly ("This spec is implementation-ready. I have no further blocking review notes."), and the review stops — no churn for its own sake.
3. *If not ready:* findings are recorded in the spec's =* Review findings= section as =TODO= tasks (one per finding, =[/]= cookie on the heading), each specific and actionable (current behavior named, risk explained, change recommended, blocking-or-not stated).
4. *The spec's review history is updated* with who reviewed it, when, which iteration it was, what changed or was recommended, and why.
5. *Deferred work is logged* to =todo.org= (v1 = =[#B]=, vNext/someday = =[#D]=), not left only in chat.
6. *Implementation tasks are enumerated* — the spec's =Implementation phases= section is lifted into a drop-in =todo.org= block (one entry per phase plus a test-surface entry), or, if the spec has no phase decomposition, that gap is raised as a finding.
7. *Generalizable review questions are harvested.* If Craig or the review uncovers a question/topic that applies beyond the current spec, rewrite it generically, add it to this workflow, and note the workflow change in the rulesets inbox/update note. Spec-specific observations stay in the spec review; reusable review practice moves here.

*Measurable validation:* after reading the review, an implementer knows exactly what to decide or change before starting, and an author running spec-response can give every finding an accept/modify/reject without guessing what it means.

* When to Use This Workflow

Trigger when:

- A spec is up for implementation and someone needs to know if it's ready.
- A spec changes materially (significant design conclusions changed → fresh review round).
- Craig says "review the spec" / "is this spec implementation-ready?" / "let's run the spec-review workflow."

Run it *early* — design review exists to catch viability problems and costly mistakes before implementation, not after.

* Precondition: spec filename

Before Phase 1, verify the file under review ends with =-spec.org=. Every design, decision, or planning document under a project's =docs/= directory carries that suffix as its identifier. The =.org= extension alone is not enough because =docs/= holds non-spec org files too (tutorials, frozen inventories, reference material).

If the file does not end with =-spec.org=, stop immediately and surface the mismatch:

#+begin_example
The file <path> does not end with -spec.org. Either the wrong file was named, or
the file should be renamed first. Spec workflows require the -spec.org suffix as a
guard against pointing the workflow at tutorial, inventory, or setup docs.
#+end_example

The user resolves the mismatch (rename the file, or point at a different one) and re-invokes the workflow. Do not proceed with the review against a misnamed file.

Anywhere the rest of this workflow refers to "the spec," "the spec file," or "the file under review," the path is the =-spec.org= file confirmed here.

* Approach: How We Work Together

** Phase 1: First-pass readiness gate

This is a fast triage on the spec's face — can it be rejected as obviously not ready before you invest in the full read? It is not the final word: several items below (architecture fit, API verification, integration points) can only be judged after Phase 2's code read, so Phase 3 re-runs this gate as the authoritative pass. Here, decide what you already can, and flag the rest to confirm after reading.

Mark the spec implementation-ready only if *all* of these hold:

- The user problem and target behavior are clear.
- Scope is explicit: v1, out-of-scope, and vNext are separated.
- Project principles are explicit where the feature touches user data, remote systems, destructive actions, long-running operations, or public extension points.
- All prior review questions/recommendations are answered or intentionally deferred.
- Every decision in the spec's =* Decisions= section is resolved — no =TODO= remains (the =[/]= cookie reads complete; =SUPERSEDED= and =CANCELLED= count as resolved), or each still-open decision carries a consciously-accepted, recorded risk. A spec still on the retired =State: proposed | accepted | superseded= field model fails this item until converted to decision tasks — convert on the next material touch.
- The expected UX is concrete enough to implement without guessing.
- Existing platform/framework affordances are reused, wrapped, or intentionally replaced; lost opportunities are called out.
- The data model, state ownership, persistence, and sync behavior are defined.
- Error strings, conflict, empty-state, and partial-failure behavior are defined with actionable user-facing messages.
- Security and privacy behavior is defined wherever the feature touches credentials, personal or shared data, or external services — or there is nothing sensitive to define and that's stated.
- Observability and diagnostic tooling are defined: how the user (or operator) sees what the feature is doing, how failures surface, how logs/debug commands isolate issues, and how sensitive data is redacted — or the feature is simple enough not to need it, and that's stated.
- Performance expectations and likely bottlenecks are considered, including long-running operations and how they are tested without making the normal suite slow/flaky.
- For transfer, sync, backup, import/export, queue, or other long-running data-movement specs: common and edge failure modes have been researched against current sources and comparable applications, and each material risk is classified as covered, avoided by scope, deferred with safeguards, or missing.
- The architecture fits the existing codebase and names its integration points.
- Architecture weak points are named with mitigation steps, especially around concurrency, lifecycle states, extension points, and failure isolation.
- The public customization/configuration surface is named with defaults, safety notes, and clear user-facing variable names.
- The documentation plan is defined: README, user guide, developer guide, migration/upgrade notes, or an explicit reason they are unnecessary.
- The project tooling is part of scope where appropriate: Makefile/package scripts expose setup, test, compile, lint, coverage, cleanup, and slow/manual verification.
- The test strategy covers unit, integration/fixture, regression, user-flow, slow/long-running behavior, coverage reporting, and optional live/external tests as appropriate.
- Rollout, migration, rollback/reversibility, and compatibility are defined when the feature changes public behavior, persisted data, external resources, package APIs, user configuration, or shared remote state.
- The plan can be phased without shipping broken intermediate states, and phases are small enough to reach a clean stopping point in one focused work session.
- External API assumptions are verified or explicitly listed as prerequisites.

If all true → tell the user it's ready and stop unless they ask for more. If any false → continue and record findings (Phase 5). A "ready" at this phase is provisional; confirm it at Phase 3 after the code read.

** Phase 2: Required reading order

Never review a spec in isolation.

1. *Read the existing implementation first.* The code paths the spec would touch: public commands and entry points, internal helpers/boundaries, current data representation, persistence/write-back, async/sync, caching, error handling, existing tests, naming/style. Capture current-state facts with function names and file paths. Don't recommend designs that ignore how the package works today.
2. *Read related specs and task tracking.* Companion specs, relevant =todo.org= tasks, README/testing docs, prior reviews (in each spec's =* Review findings= and =Review and iteration history=). Record which tasks the spec absorbs, which stay separate, which decisions are already made, which are still open.
3. *Read the target spec end to end — twice.* First for its problem/behavior/phases/assumptions; second looking only for gaps. The second read asks: "What would an implementer still have to invent?"

** Phase 3: Re-run the gate (authoritative)

After reading code and spec, re-run the Phase 1 gate — this is the pass that counts, because now you can actually judge the items that needed the code: architecture fit, API verification, integration points. If now ready, don't manufacture churn. If not, record findings (Phase 5).

** Phase 4: Evaluate across dimensions

Work the spec against these. Each is a source of concrete findings, not a box to tick.

- *User problem & mental model.* Real problem or just machinery? Does UX match how users think? Predictable names/commands/prompts/layouts? Is editable-vs-generated content obvious? Are destructive/workspace-mutating actions explicit and confirmed? For org features, be *strict about ownership* — users treat visible org text as editable unless told otherwise.
- *Project principles and non-negotiables.* Does the spec state do/don't rules that protect user trust? For filesystem/remote/data-mutating features, does it forbid silent data loss, require residue/temp-file cleanup verification, make destructive and fallback paths visible, define cancellation/hung-work behavior, and prevent secret leakage? For extension points, does it define contracts plugin authors must obey?
- *Reuse and lost opportunities.* What does the underlying platform/framework/library already provide? Does the spec preserve useful existing affordances, naming, configuration, keymaps, commands, hooks, logs, caches, or test utilities? If it replaces them, is that intentional and justified? Are there obvious user-value opportunities being left on the table?
- *Scope & boundaries.* What's v1 / out-of-scope / vNext? Are deferrals captured (=todo.org=)? Does the phased plan avoid half-working states? If a decision is deferred, is v1 still coherent without it? Can each phase finish cleanly in one focused work session, or is it too large to stop safely?
- *Requirements clarity.* Each requirement unambiguous, testable, traceable to a source (user ask / bug / API capability / decision)? Are "good/fast/intuitive/smart/seamless" replaced with concrete behavior? Terms used consistently? Infeasible/unverified requirements flagged?
- *Data model & ownership.* Durable IDs vs display-only names? What's local/remote/generated/cached/user-authored? Who owns each editable region? What's replaced on refresh, survives view-switch / restart? What if one remote object could appear in multiple local places? Prefer stable IDs for execution, names for display; don't overload generic org properties (=:ID:=) for app-specific remote IDs unless intentional.
- *Sync, refresh, conflict.* What triggers sync (command / save hook / state hook / timer) and refresh? One-way or bidirectional? No-op detection? Remote-conflict detection? v1 conflict behavior? Dirty-buffer behavior? API success but GraphQL-level failure? Network failure mid-pagination/mutation? For v1, simple is fine if explicit: detect, refuse, tell the user what to do next.
- *Transfer/sync failure-mode research.* For features that move or reconcile user data, do online/current research on how the domain fails in general and in comparable products. Look for partial writes, temp/residue files, source mutation during transfer, locked/read-only files, permission/ACL/xattr loss, quota/space failure, case/restricted-name collisions, invalid encodings, duplicate remote objects, symlink/hardlink/special-file behavior, clock skew/mtime unreliability, watcher delays, missing roots/unmounted volumes, ignored/selective trees, mass deletes, ransomware-like churn, reconnect/offline gaps, conflict propagation, stale status, and "stuck but no file list" complaints. Classify every material item as =covered=, =avoided by v1 scope=, =deferred with guardrails=, or =missing=; missing items become findings.
- *API & external dependencies.* Which fields/mutations/filters/enums are assumed, and are they verified against current schema/docs/live responses? Permission differences, rate limits, pagination, nulls, partial errors handled? Any optional external tool — what's the fallback? Don't let a spec silently depend on an unverified API shape.
- *CLI/tool-wrapper value.* If the feature wraps command-line tools or external executables, why would a user prefer the package over running the tool directly? Specs must cover common tool-specific failures, exit codes, stderr/log signatures, destination/backend gotchas, preflight/doctor checks, redaction, retry/cancel behavior, and a failure explainer. Helpful-error success criteria: identify the failure class, explain likely cause, prove it with redacted evidence, state safety outcome, offer repair actions, and provide a redacted reproduction command/log bundle.
- *Architecture & maintainability.* Fits current module boundaries, or should new ones precede growth? Which parts are pure vs side-effecting? Are transport/parsing/normalization/rendering/command-orchestration separated enough to test? Will it multiply special cases? Backward-compat preserved where needed? Favor small pure cores surrounded by thin IO/command layers. *Generated config across namespaces:* when a generated config (a package-manager config, a mount unit, a container or VM image, an installer profile) is consumed in more than one namespace, does every path or URL inside it resolve in the namespace that *consumes* it, not the one that *wrote* it? Build host, chroot, image runtime, installed target system, container, and VM are distinct namespaces — a path valid in the one that generates the config (a build-time staging dir) can be absent or different in the one that reads it (the live or target root).
- *Simplicity and complexity control.* Is the UX dead simple on the hot path? Are advanced controls out of the way? Are command functions thin? Are branchy functions split or justified? Would cyclomatic complexity, coverage, or code-size reports reveal refactoring targets? Are there explicit refactoring triggers before complexity hardens?
- *Extension/developer experience.* If plugins/backends/extensions are part of the feature, is the public ABI precise? Are capability flags defined? Are lifecycle callbacks, failure states, redaction rules, and contract tests documented? Can another engineer add an extension without reading private internals? Is the extension contract tiered so authors can build a minimum runnable plugin quickly, then graduate to publishable/excellent diagnostics? Does the spec clearly say what information is required, what is optional, and what can be unknown/conservative at first? Will authors know what evidence/patterns/capability metadata to add, or are they expected to infer it from core internals? Are scaffolds, templates, fixture capture tools, pattern-table helpers, contract-test tiers, and "where the line is" docs provided? Could the contract be so complex that reasonable plugin authors opt out, and if so, what helper API removes that burden? Treat plugin authors as end-users of the developer experience and require their plugins to give downstream users the same diagnostic quality the core promises.
- *Robustness & error handling.* Expected failure modes? Errors distinguishable from empty-but-successful results? Actionable messages? Do error strings name the operation, object, backend/API/tool involved, and next step? Retries? Stale/missing caches? Can malformed local files or missing properties crash commands? nil/null/missing-field behavior defined? Data-loss-by-default avoided? Anything that writes files or remote data needs a clear data-loss story.
- *Performance & scale.* Expected counts (issues/comments/labels/teams/projects/views)? Server-side filtering where possible? Bounded, visible pagination? Cached name→ID lookups? Sync calls in the command path acceptable? Could a save hook or whole-file scan make N network calls? Rendering linear? Full-file rewrites avoided? Long-running operations async/cancellable/observable? Is concurrency/queueing/backpressure defined? Are high-output process filters throttled and cheap? Is progress/ETA exposed only when defensible, and are hung/stalled operations detectable and killable? Identify UI freezes, repeated network calls, unbounded pagination — without premature optimization.
- *Security & privacy.* API keys safe? Debug logs leaking secrets or private issue text? Confirmations before mutating shared workspace objects? Personal vs shared distinguished? Local files holding sensitive descriptions/comments? Anything to redact from messages/logs? Any work-tracker integration may handle private company data.
- *UX & accessibility.* Discoverable commands? Recoverable mistakes? Prompts ordered to the task? Safe, useful defaults? Informative-not-noisy status messages? Does the UI avoid implying unsupported actions are supported? Match the upstream product's permissions/concepts? Are customizations named in user language, with clear defaults and docstrings? For Emacs packages, command names, completion candidates, buffer layout, defcustom names, and message wording *are* the UX.
- *Test strategy and coverage.* Characterization tests before behavior changes? Pure functions to unit-test? API responses needing fixtures? Command flows needing stubs? Regression tests for prior bugs? Boundary/error cases? What's covered elsewhere and shouldn't be re-tested? Which existing tests must change? How is coverage generated, summarized, and used to find untested/refactor-worthy code? Prefer tests that lock contracts: representation shape, query compilation, sync no-op, conflict refusal, pagination, dirty-buffer protection, log redaction, and long-running/slow-operation behavior via fakes rather than flaky live dependencies.
- *Observability & operations.* How does a user see what the package is doing? Progress messages for long ops? Useful, safe debug logging? Are logs structured enough to isolate issues from a bug report? Are commands provided to inspect/clear caches, test connectivity, diagnose backends/tools, copy redacted debug info, or reproduce command invocations? How are terminal states discovered: completion, failure, partial success, stalled/hung, cancelled, cleanup-unverified, and "needs user action"? Does the product notify only when useful, avoid noisy success spam, and keep non-success states visible until acknowledged? For generated org files, headers should often carry source, filter/view name, refresh time, count, truncation state.
- *Comparable-product sentiment.* When there are obvious adjacent products, research what users love and hate about them from official docs plus current community reports. Do not cargo-cult their feature set; translate findings into the spec's scope. For each loved behavior, say whether the spec provides it, intentionally omits it, or defers it. For each hated behavior, say whether the spec avoids, resolves, inherits, or accepts it.
- *Documentation & migration.* README/config changes? New commands and defcustoms documented? User guide needed? Developer guide/API guide needed? Makefile or package scripts documented as part of the product? Old behaviors removed/renamed/migrated? Breaking changes acceptable? Changelog note needed? Don't ship with stale examples.
- *Rollout, compatibility, and rollback.* How does the feature ship safely? Are old config/data formats migrated or read compatibly? Can users opt out or revert? If remote/shared data is mutated, is there a dry-run, backup/trash/versioning, or recovery path? If a public API/plugin contract changes, is the compatibility story explicit? For local packages, "rollback" may mean disabling the module, removing generated files, or restoring prior config; spell that out where it matters.
- *Development tooling.* Does the repo give contributors obvious commands for setup, fast tests, specific tests, compile, lint, coverage, cleanup, slow/manual tests, and release checks? Are optional/live tests gated by explicit environment variables? Is the Makefile/script surface consistent with sibling projects?
- *Small enhancement radar.* Are there low-complexity, high-value affordances already provided by the platform that should be surfaced now or explicitly deferred? Examples: archive/compress commands in file managers, built-in history, previews, diagnostics, or doctor commands. Keep the hot path simple; capture the opportunity rather than accidentally losing it.

** Phase 5: Record findings in the spec

Findings live in the spec, not a sibling file. Add (or append to) a =* Review findings= section near the spec's =* Decisions= section, with a =[/]= cookie on the heading. Each finding is a =** TODO= task: the heading is the smallest noun phrase naming the gap; the body names current behavior, the risk, and the recommended change. Tag a blocking (high-priority) finding =:blocking:= — it holds the rubric at =Not ready= until dispositioned; leave non-blocking findings untagged. Findings accumulate across review rounds the way decisions do, and the responder completes each one in place (Phase 4 of spec-response), so the section becomes the full review/response trail.

#+begin_src org
,* Review findings [/]
,** TODO Comment edit-back is undefined :blocking:
The spec says fetched comments render as subheadings but doesn't define whether
editing one syncs back. Linear only lets users edit their own comments. V1 should
treat fetched comments as remote-owned display content and support only adding new
comments; editing own comments can be vNext. (blocking)
,** TODO Empty result and fetch error render identically
A failed fetch and a successful-but-empty fetch produce the same buffer, so the
user can't tell "no issues" from "the query broke." Define a distinct empty-state
message. (non-blocking)
#+end_src

Where the old review-file sub-sections go now: the scope-reviewed and overall-assessment narrative goes in the =Review and iteration history= entry (Phase 6); suggested spec edits are the recommended-change line in each finding's body; agreed decisions flip the spec's own =* Decisions= tasks; open questions are =:blocking:= findings or open decisions; vNext candidates are logged to =todo.org= as =[#D]= (Phase 6). The Phase 4 review dimensions are where findings come *from* — not headings to reproduce in the spec.

** Phase 6: Assign the rubric and update tracking

Assign one label consistently:

- =Ready= — no blocking open questions; implementation can start. Requires both cookies complete: no decision in =* Decisions= and no =:blocking:= finding in =* Review findings= still =TODO= (the =[/]= cookies read complete; =SUPERSEDED=/=CANCELLED= and a completed or rejected finding count as resolved) — a still-=TODO= decision or =:blocking:= finding holds the rubric at =Not ready=, or =Ready with caveats= if the author consciously accepts and records the risk. A non-blocking finding left =TODO= is author's discretion and does not hold the rubric.
- =Ready with caveats= — can start if the caveats are accepted and tracked.
- =Not ready= — blocking ambiguity / missing decisions would force implementers to invent product behavior.
- =Needs research= — external API/library/platform assumptions must be verified first.

The most useful reviews move a spec from =Not ready= to =Ready with caveats= or =Ready= once decisions are captured.

Finding severity maps to blocking power: *high-priority findings block =Ready=* — they hold the rubric at =Not ready= (or =Ready with caveats= if the author accepts and tracks them) until dispositioned; *medium-priority findings are the author's discretion* and don't block. State the blocking status on each finding so the author running spec-response knows which ones gate the rubric.

Then update the spec's review history. Specs should carry a bottom section named =Review and iteration history= (or the nearest existing equivalent) that tracks each material author/reviewer pass. Add a concise entry for this review even when the spec is ready and no findings are recorded.

Each entry is an org subheading with a compound id followed by three body fields.

Heading format: =YYYY-MM-DD Day @ HH:MM:SS -ZZZZ — Contributor — Role=

The timestamp + contributor + role concatenate as an opaque id — nothing more should be read into it. Timestamp matches the project's todo-format event-log convention and gives natural temporal sort without implying any decision ordering. Contributor uses short parenthetical project context where session matters (e.g. =Claude Code (linear-emacs)=, =Claude Code (rulesets)=, =Codex=, =Craig Jennings=). Role can be compound (e.g. =reviewer + responder=) when one pass fused multiple roles. Author, reviewer, responder, verifier, and researcher are the standard role names.

Body fields:

- *What changed or was recommended:* high-signal summary, not a duplicate of the whole review.
- *Why:* the decision pressure or rationale that caused the contribution.
- *Artifacts:* links to the relevant findings, commits, task IDs, or source checks when useful.

If the spec has no such section, add it at the bottom. Keep the history short and cumulative; it is provenance for future readers, not a session transcript.

*Check the spec decomposes into phases.* A =Ready= spec needs an =Implementation phases= section an implementer can turn into one task per phase plus a test surface. Confirm it's present and decomposable — each phase small enough to reach a clean stopping point in one focused session, with no broken intermediate states. If it's missing or can't be phased, file that as a =:blocking:= finding; don't invent the phases. The phase-to-task breakdown itself is spec-response's job (its Phase 6 reads =Implementation phases= directly once the author confirms =Ready=); the reviewer only verifies the section exists and is sound.

Then log deferred work to =todo.org=: v1 implementation = =[#B]= (unless urgent or speculative); vNext/someday = =[#D]=. Tag =:feature:= / =:bug:= / =:refactor:= / =:test:= / =:quick:= / =:solo:= only when accurate. Don't leave important deferred decisions only in chat.

* Principles to Follow

** Read the code before critiquing the spec
A recommendation that ignores how the package works today is noise. Cite function names and file paths; ground findings in current behavior.

** Specific over vague
Name the current behavior or spec text, explain the risk, recommend the change, say whether it blocks. Avoid vague preferences, whole-spec rewrites, architecture astronautics, unbounded "consider" lists, and questions whose answer doesn't change implementation.

Model finding:
#+begin_quote
The spec says fetched comments render as subheadings but doesn't define whether editing an existing comment syncs back. That's risky because Linear only allows users to edit their own comments. V1 should treat fetched comments as remote-owned display content and support only adding new comments; editing own comments can be vNext.
#+end_quote

** Decide readiness first, critique second
The gate is the product of the review. A long critique on a spec that's actually ready just creates churn; a "looks fine" on one that isn't ships hidden decisions.

** Review as a risk reducer, not a rewrite contest
Good spec review improves the chance of a correct implementation. It is not a venue for proving cleverness, redesigning the whole product, or teaching broad lessons at the author's expense. Prefer the smallest spec change that removes the implementation risk. If a finding implies a major redesign, name the decision pressure and the options; don't silently rewrite the architecture in review prose.

** Separate blockers from preferences
Every material comment should be tagged by force: blocking, should-fix, or optional. Avoid burying a true blocker in a long list of preferences, and avoid making stylistic or taste comments look like correctness concerns. Bikeshedding belongs in linters, formatters, local conventions, or explicit optional notes.

** Make feedback author-usable
Review comments should be specific, neutral, and actionable: quote or name the spec behavior, explain the risk, recommend the smallest concrete change, and say how the author can verify the fix. Avoid personal language, rhetorical questions, vague "this needs work" comments, and comments that require the author to infer the desired edit.

** Keep review and response roles explicit
If the user asks for review plus "enhance the spec" in the same turn, produce the findings first. Make only low-risk provenance and tracking edits unless the user clearly wants the reviewer to respond too. Don't silently resolve product decisions on the author's behalf — a proposed default belongs in a finding until it's accepted, modified, or rejected.

** Preserve iteration provenance
Future reviewers and implementers need to know not just the current decision, but how the spec got there: how many review/response loops happened, who contributed, what they changed or recommended, and why. Keep that record in the spec itself under =Review and iteration history= so the trail survives chat loss and agent handoffs.

** Be strict about ownership
Especially for org-mode features: a user treats visible text as editable unless the representation says otherwise. Make generated-vs-editable explicit.

** Never depend on an unverified API shape
If the spec assumes fields/mutations/enums, they're verified against current schema/docs/live responses, or listed as a research prerequisite. =Needs research= is a real, useful verdict.

** Source external-dependency checks in the finding
When a finding turns on a current external-dependency fact (release version, API capability, platform behavior, package availability, hosted-service terms), cite the checked source in the finding body. Stale dependency assumptions are common, and the next reviewer needs to tell "verified this pass" from "remembered from prior context."

** Favor small pure cores and thin IO layers
Push findings toward separable, unit-testable pure functions surrounded by thin command/transport layers.

** Block only on answer-changing questions
Open questions in the review are the ones whose answer materially changes implementation. Everything else is a suggestion, not a blocker.

* Living Document

Update this workflow as review practice sharpens — new dimensions worth checking, better model findings, refinements to the readiness gate and rubric.

When a review conversation surfaces a question that would improve future reviews, decide whether it is generalizable:

- *Generalizable:* applies to a class of future specs (for example, "How will users discover long-running failures?", "What do comparable products make users love/hate?", "Which transfer/sync edge cases are covered, avoided, or deferred?"). Rewrite it in product-neutral language, add it to this workflow, and append a short note to the rulesets inbox/update note explaining what changed and why.
- *Spec-specific:* depends on this product's exact domain, names, or implementation choices. Keep it in the review/spec history and do not bloat the workflow.

This workflow is allowed to grow, but only by turning repeated or broadly useful review instincts into clear checks that produce implementation-changing findings.

* Research notes behind this checklist

Combines the review process used on the linear-emacs issue query/representation specs with broader design/spec-review practice:

- Design reviews should happen early enough to find viability problems and costly mistakes before implementation (Google design-review research).
- Product/spec docs should create shared understanding — goals, assumptions, and an explicit "what we're not doing" (Atlassian PRD guidance).
- Review sessions should capture open questions and gaps so they're tracked and resolved (Atlassian design-review template).
- Requirements should be checked for correctness, ambiguity, completeness, consistency, verifiability, traceability, feasibility (SRS checklist categories).
- Design-doc review should be iterative but restart/refocus when significant conclusions change (Gerrit design-doc guidance).

Sources:

- [[https://research.google/pubs/improving-design-reviews-at-google/][Google Research: Improving Design Reviews at Google]]
- [[https://www.atlassian.com/agile/product-management/requirements][Atlassian: Product Requirements Documents]]
- [[https://www.atlassian.com/software/confluence/templates/design-review][Atlassian: Design Review Template]]
- [[https://www.geeksforgeeks.org/software-engineering/software-requirement-specification-srs-document-checklist/][GeeksforGeeks: SRS Document Checklist]]
- [[https://gerrit-review.googlesource.com/Documentation/dev-design-docs.html][Gerrit: Design Docs Review Process]]
- [[https://google.github.io/eng-practices/review/reviewer/][Google Engineering Practices: Code Review Developer Guide]]
- [[https://google.github.io/styleguide/docguide/best_practices.html][Google Documentation Guide: Best Practices]]
- [[https://spec-coding.dev/guides/spec-review-checklist][Spec Coding: Spec Review Checklist]]

* Review and iteration history

** 2026-05-23 Sat @ 00:00:00 -0500 — Claude Code (linear-emacs) — author
- *What:* Initial draft of the workflow file. Validated by a real 2026-05-23 run against a linear-emacs ticket before being handed off to rulesets.
- *Why:* Craig wanted to use each contributor for their value — his time goes into user scenarios and direction-setting, agents' time goes into structured technical review (gate-checking, finding-tracking, disposition rigor) where agents are more consistent than humans. spec-review formalized the reviewer-side discipline so Craig doesn't spend his time on gate-checks an agent can do better.
- *Artifacts:* Delivered to =rulesets/inbox/= on 2026-05-23 as part of a 4-file handoff (2 workflow drafts + 2 handoff notes). Inbox files since deleted after install. Heading timestamp is approximate per the opaque-id convention; no exact time was recorded.

** 2026-05-23 Sat @ 12:59:31 -0500 — Claude Code (rulesets) — reviewer + responder
- *What:* Three tightening edits to the review workflow itself before install: Phase 1 reframed as fast triage with Phase 3 the authoritative gate after the code read; readiness gate expanded from 11 to 13 items adding security/privacy and observability (each with an escape hatch for trivial features); severity-to-blocking-power mapping added in Phase 6 (high blocks Ready, medium is author discretion). Paired response-workflow edits landed in the same commit and are recorded in spec-response.org's history.
- *Why:* Three parallel research subagents (Definition-of-Ready / INVEST, requirements-quality IEEE 830 / ISO 29148, disposition-tracking peer-review practice) confirmed gaps. Close-read identified the Phase 1 vs Phase 3 ordering tension (gate items needed code-read context) and a security/observability blind spot.
- *Artifacts:* Commit =7f2aea1=, +706 lines across 6 files including INDEX entries and the paired spec-response.org edits.

** 2026-05-28 Thu @ 03:22:25 -0500 — Craig Jennings — author
- *What:* Added the Review-and-iteration-history requirement itself. New goal item ("the spec's review history is updated"); new Phase 6 sub-section with the entry shape (heading-as-compound-id + What/Why/Artifacts body); new "Preserve iteration provenance" principle.
- *Why:* Craig had no way to scan a spec under review and tell which iterations had already been processed, which were new, and which were pending — file mtime / created-time was the only signal, silly and error-prone. The iteration-history section makes processing state visible inside the spec content itself.
- *Artifacts:* Commit =55adf6e=.

** 2026-05-28 Thu @ 03:23:01 -0500 — Claude Code (rulesets) — author
- *What:* Backfilled this section with entries for the workflow file's prior iterations (Draft 1, Review-and-Fold, Requirement addition). Used commit author dates where exact; Draft 1's timestamp is an opaque-id placeholder since no exact time was recorded.
- *Why:* The iteration-history requirement landed in =55adf6e= without retroactive entries for the workflow file's own prior history. This pass closes that gap so the section starts with full provenance rather than empty.
- *Artifacts:* Working draft sources at =working/spec-workflows-iteration-history-backfill/draft-entries.org= (deleted on this commit). Spec-response cycle (two Codex reviews and three rulesets responses on the draft) validated the entries and the entry-shape before splicing.

** 2026-05-31 Sun @ 10:39:49 -0500 — Claude Code (rulesets) — author
- *What:* Added a Phase 6 step, "Emit implementation tasks (drop-in for todo.org)": lift the spec's =Implementation phases= section into a paste-ready =todo.org= block (one =[#B]= TODO per phase plus a test-surface entry mirroring =Acceptance criteria=), with the missing-phase-decomposition case raised as a finding rather than invented. Added Exit Criterion 6 to match.
- *Why:* From a pearl handoff. Phase 6 only logged deferred/v1 work in passing; this makes the implementer handoff one paste instead of a re-read, and forces specs to be decomposable into phases (a spec that resists it surfaces a shape problem before =Ready=).
- *Artifacts:* =todo.org= task ([#C] =:feature:solo:=, closed this commit). Phase 6 + Exit Criteria edits in this file.

** 2026-06-06 Sat @ 04:52:13 -0500 — Codex (emacs-d) — reviewer + author
- *What:* Added reusable checks from DUET's third review pass: transfer/sync failure-mode research, covered/avoided/deferred classification, comparable-product love/hate sentiment, terminal-state discovery/notification, and explicit harvesting of generalizable user questions into the workflow with a rulesets inbox note.
- *Why:* Craig asked for deeper online research into how transfers and sync go wrong, whether DUET covers or avoids those failures, whether users would discover failures, and how future spec reviews should absorb new reusable questions rather than leaving them in chat.
- *Artifacts:* DUET spec/review updates in =docs/design/duet-spec.org= and =docs/design/duet-spec-review.org=; rulesets inbox note =2026-06-06-spec-review-workflow-update.md= updated.

** 2026-06-06 Sat @ 05:18:30 -0500 — Codex (emacs-d) — researcher + author
- *What:* Added reviewer anti-pattern guidance and a rollout/rollback/reversibility dimension after a meta-review of design/spec-review advice. New principles: review as risk reduction rather than rewrite contest, separate blockers from preferences, and make feedback author-usable. Added source links for Google code-review/documentation guidance and a modern spec-review checklist.
- *Why:* Craig asked for online research into what good spec reviewers should always do and avoid, including reputable sources and developer-community advice. The workflow already covered most readiness mechanics; the missing reusable content was reviewer discipline and compatibility/rollback framing.
- *Artifacts:* This workflow; DUET final review pass; rulesets inbox note =2026-06-06-spec-review-workflow-update.md= updated.

** 2026-06-06 Sat @ 07:25:11 -0500 — Claude Code (rulesets) — responder
- *What:* Promoted the two Codex (emacs-d) passes above into the canonical workflow. The reusable checks had been written into =.emacs.d='s synced copy, which the startup rsync overwrites from canonical, so they would not have survived or propagated. Folded them into =claude-templates/.ai/workflows/spec-review.org= and synced the =.ai/= mirror, so every project picks them up on next startup. Verified the promotion is a pure superset — every change is an added or expanded generic check, no regressions.
- *Why:* The handoff =inbox/2026-06-06-spec-review-workflow-update.md= asked for it: reusable review practice belongs in the canonical workflow, spec-specific findings stay in the spec. Exit Criterion 7 and the Living-Document harvesting guidance, added in the same passes, name this as the intended path.
- *Artifacts:* Handoff preserved at =docs/design/2026-06-06-spec-review-reusable-checks-handoff.md=. Paired spec-response.org Phase 6 (author-side implementation-task breakdown) committed in the same session.

** 2026-06-12 Fri @ 19:09:00 -0500 — Claude Code (rulesets) — responder
- *What:* Reconciled this workflow to spec-create's new Decisions convention (each decision is an org =TODO= task that flips to =DONE= on agreement, with a =[/]= cookie on the =* Decisions= heading). The Phase 1/3 readiness gate gains an all-decisions-=DONE= item, and the Phase 6 =Ready= rubric label now requires the =[/]= cookie to read complete (a still-=TODO= decision holds the rubric at =Not ready=, or =Ready with caveats= on a recorded, consciously-accepted risk).
- *Why:* The convention change landed in spec-create.org via an .emacs.d handoff (originated in its keymap-consolidation spec); the rubric still gated on the retired =State:= field model.
- *Artifacts:* Handoff =inbox/2026-06-12-1906-from-.emacs.d-spec-create-decisions-todo-note.org=. Paired spec-create.org and spec-response.org edits in the same commit.

** 2026-06-12 Fri @ 19:30:03 -0500 — Claude Code (rulesets) — author
- *What:* Two refinements to the same-day decisions convention after Craig's review: the gate item and =Ready= rubric now read "no decision is still =TODO=" with =SUPERSEDED= and =CANCELLED= counting as resolved (spec-create's template defines them as done-class keywords via a =#+TODO:= header), and a spec still on the retired =State:= field model explicitly fails the gate item until converted — closing the vacuous-pass hole on old specs.
- *Why:* Review of the freshly-landed convention flagged that TODO/DONE alone lost the old model's superseded state and that the gate as written would silently pass a spec with no decision tasks at all. Craig chose the two done-class keywords and the auto-added =#+TODO:= header (the in-file header is what makes custom keywords portable).
- *Artifacts:* Paired spec-create.org edits (keyword scheme + template header) in the same commit.

** 2026-06-21 Sun @ 23:16:06 -0400 — Claude Code (rulesets) — responder
- *What:* Moved findings from a sibling =<spec>-review.org= file into the spec itself. Findings are now =** TODO= tasks under a =* Review findings= section with a =[/]= cookie, mirroring =* Decisions=; =:blocking:= marks high-priority. Phase 5 records findings in the spec instead of writing a review file; the Phase 6 =Ready= rubric gates on both the decisions and the findings cookie; the implementation-task drop-in (which lived in the review file) is gone, leaving the reviewer to verify the spec decomposes into phases and spec-response to build the breakdown. Also added two reviewer-practice principles harvested from a home spec-review: keep review and response roles explicit, and source external-dependency checks in the finding.
- *Why:* The delete-the-review-file convention left the iteration-history =Artifacts= line dangling and dropped the verbatim review; keeping the file instead collided with spec-response's file discovery and its "no review files remain" done-condition. Craig's call: incorporate the review into the document, reusing the decisions machinery so the readiness signal is a cookie, not a file's presence or absence. The role-explicit and source-checking practices came in from the home finance-report spec via inbox handoffs.
- *Artifacts:* Paired spec-response.org edits in the same commit. Inbox handoffs =2026-06-20-2339-from-home-spec-response-readiness-gate-proposal.org=, =2026-06-21-0156-from-home-companion-to-tonight-s-spec-response.org=, and the home-edited =2026-06-21-0156-from-home-spec-review.org=.