Commit Graph

4 Commits

Author SHA1 Message Date
Clay Good 83be9d113e feat(validate): add --archived to lint task completion of archived changes (#1604)
* feat(validate): add --archived to lint task completion of archived changes

`openspec validate --archived` scans every change under changes/archive/
and fails (exit 1) if any has unchecked tasks in tasks.md. This catches
changes archived with unfinished work — which the normal validate flow
never sees, since it only looks at active changes — and is meant for a
pre-commit or CI hook.

It is a standalone, opt-in scope: it returns before any existing bulk
path, so no current `validate` invocation changes behavior, and it does
not re-validate already-applied spec deltas. Reuses getTaskProgressForChange
(the same counter status/list/archive use) so task counting never forks,
and reads root.archiveDir so it is store-aware.

Closes #205

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(validate): fail loudly on archive read errors and unreadable task files

Address adversarial + CodeRabbit review of `validate --archived`:

- listArchivedChangeIds now returns [] only for ENOENT (missing archive
  dir) and rethrows permission/I/O/ENOTDIR errors, so a real archive-read
  failure exits 1 instead of silently reading as "no archived changes".
- Add getTaskProgressDetailForChange, which reports task files that exist
  but cannot be read; --archived turns those into an ERROR (naming the
  file) rather than silently counting them as zero tasks. The shared
  getTaskProgressForChange now wraps it and drops the detail, so
  status/list/archive totals are byte-identical.
- Start the spinner after listing so a thrown listing error never leaves
  a spinner running.

Adds regression tests (archive path is a file; archived tasks.md is
unreadable) and unit tests for the new detail variant. Docs: align the
--archived table verb and add a troubleshooting one-liner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(validate): address CodeRabbit nits on archived-task tests

- Build the unreadable-fixture path from separate path.join components
  instead of a hard-coded Unix-separator string.
- Assert the reported unreadable path (canonicalized with
  realpathSync.native), not just the count, so a wrong path can't pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(validate): address round-3 review of --archived

From three fresh adversarial reviews (scale/perf, flag/output-shape,
filesystem/security):

- perf: memoize schema→glob resolution across archived changes via a
  run-scoped SchemaGlobCache, so the same schema.yaml isn't re-parsed
  once per change (the archive is append-only and can hold thousands).
  Threaded as an optional arg; existing callers are unchanged. Loop stays
  sequential by design (per-change work is synchronous) — now documented.
- output shape: issue `path` now follows validate's convention —
  'tasks.md' for incomplete tasks, and the POSIX root-relative file path
  for an unreadable file (one issue per file) instead of the bare 'tasks'.
- plain output: print `change/<id>` (matching the JSON `type` and bulk
  validation) instead of `archived/<id>`.
- docs: correct the "Never throws" docstrings (glob resolution can throw
  on a malformed/unsafe schema; the caller guards it) and note the
  load-bearing projectRoot override for the archive path depth.

Store-mode resolution confirmed correct by review. Tests updated + a memo
regression test added.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-11 20:53:06 +00:00
Clay Good 427abf40ac fix(tasks): count indented sub-tasks in task progress (#1486)
Both checkbox parsers anchored the bullet at column 0, so an indented
sub-task was invisible to `openspec list`/`view` progress, to the apply
task list, and to archive's incomplete-task check. A change whose
sub-tasks were unfinished reported "✓ Complete" and archived with no
warning.

One shared `parseTaskLines()` now backs both surfaces and allows leading
whitespace. It matches every line the two patterns it replaces matched,
and more - including a tab or non-breaking space inside the brackets,
which the old counting pattern accepted - so task counts can rise but
never fall: no change starts reporting less work than before, and
archive's gate can only get stricter.

Checkboxes still count wherever they sit, including inside a code fence.
Skipping fenced ones was implemented and dropped: every rule for deciding
which fence is real has an input where a stray or unbalanced ``` swallows
genuine tasks, which is the silent failure this fix exists to remove.

Verified differentially against a build of main over hand-built fixtures
and the repo's own 120 tasks.md files: 0 files count fewer tasks, 0 lose
an incomplete-task warning.

Closes #1485

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 22:41:57 +00:00
Clay Good a874d1d671 chore(security): create test temp dirs with mkdtemp and override two pinned CVEs (#1432)
* test: create temp dirs with fs.mkdtemp

Every one of these suites built its temp directory by hand:

    testDir = path.join(os.tmpdir(), `openspec-test-${randomUUID()}`);
    await fs.mkdir(testDir, { recursive: true });

That check-then-create is what CodeQL's js/insecure-temporary-file flags,
and it accounted for 452 of the repo's 467 dismissed code-scanning alerts.
Dismissing them one by one is a treadmill: every new suite that copies the
idiom mints fresh alerts, and a real finding is easy to lose in that volume.

fs.mkdtemp creates the directory atomically at mode 0700 with a random
suffix, so there is no window to pre-empt and no name to guess. The suites
that already used it are the evidence this silences the rule: 33 of the 34
files calling mkdtemp carry zero alerts. The lone exception, archive.test.ts,
was scanned one commit before its own mkdtemp fix landed and is left to that
change rather than conflicting with it.

The dirs named on Date.now() alone were genuinely predictable; the randomUUID
ones were not, but they trained the same copy-paste. Both are gone now.

Behavior is unchanged: mkdtemp creates the root the old mkdir created, and
no assertion depended on the root being absent. Verified with the full suite
(2196 tests, 111 files).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(deps): override postcss and sharp in the website

Two advisories on the docs site have no Dependabot PR and never will:
Next pins postcss at 8.4.31 as a direct dependency and sharp at ^0.34.5 as
an optional one, so Dependabot cannot raise either without a Next release
that does it first. 16.2.11 does not — it still pins both. Left alone these
sit open indefinitely.

  postcss  8.4.31 -> 8.5.22   GHSA-qx2v-qp2m-jg93  (XSS via unescaped </style>)
  sharp    0.34.5 -> 0.35.3   GHSA-f88m-g3jw-g9cj  (4 libvips CVEs)

A version-ranged selector (`postcss@<8.5.10`) was the first instinct, since it
lapses on its own once Next moves past it. It is the wrong tool: it pins the
override to one advisory's floor, and silently stops applying when the next
advisory raises that floor. GHSA-6g55-p6wh-862q landed while this branch was
open and moved postcss's patched floor to 8.5.12 — under the ranged selector a
dependency pinning 8.5.11 would have resolved to 8.5.11 and stayed vulnerable.
A plain floor cannot under-match, so that is what this uses.

The floor also covers the new advisory: 8.5.22 is past 8.5.12. postcss dedupes
to the single copy the site already had for Tailwind.

These are the last two open Dependabot alerts on the repo. `pnpm audit` on
website/ goes from 2 advisories to "No known vulnerabilities found". The site
builds clean, OG image generation included, and the package set grows by
exactly two platform-gated wasm32 binaries that never install on CI.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ci(security): audit the docs site too

Both `pnpm audit` steps run at the repo root. The docs site keeps its own
lockfile and is not a workspace member, so neither step could see it — the
root audit passed all the way through while postcss and sharp sat open in
website/. That blind spot is why they needed a manual override to find.

Blocking rule copied from the published-dependency audit: advisory on pull
requests, blocking on the weekly schedule and on pushes to main. An
always-advisory step would only relocate the blind spot — the sweep would stay
green with a live advisory and someone would have to read the log of a passing
run to notice.

`!cancelled()` because the two audits above can fail hard. Without it a root
advisory would skip this step entirely, in exactly the situation where the
site's own state matters most.

Verified against the pre-fix lockfile — the step reports the two advisories it
would have caught:

  2 vulnerabilities found
  Severity: 1 moderate | 1 high

and reports "No known vulnerabilities found" against the fixed one. Confirmed
it reads website/pnpm-lock.yaml and not the root: with a vulnerable website
lockfile and a clean root, it exits 1; a missing website lockfile is a hard
ERR_PNPM_AUDIT_NO_LOCKFILE rather than a false clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-23 16:17:26 +00:00
Clay Good a3253051ea fix(resolution): converge validate, view, and archive onto canonical resolution (#1182, #1202, #1156) (#1280)
* docs(openspec): propose resolution/validation parity bug bundle (#1182, #1202, #1156)

Planning artifacts only (proposal/design/spec deltas/tasks) for a focused
bug-fix bundle. Three read/validate paths silently diverge from the canonical
logic a sibling command already gets right:

- #1182 validate ignores workspace planning homes that status/instructions resolve
- #1202 view counts only changes/<name>/tasks.md, ignoring the schema tasks glob
- #1156 the SHALL/MUST body-keyword hint fires for deltas but not main specs

Fix converges each divergent path onto the canonical one; parity is asserted by
test. No new surface, no behavior change to the already-correct paths. Validates
--strict.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(openspec): bulletproof the parity bundle after adversarial source review

Hardened all three bugs after tracing each path to source with parallel
verification agents. Material corrections:

- #1182: reframed from "workspace planning home resolution" (planning homes are
  repo-only; the feature is now 'stores', and validate already accepts --store)
  to the real, reproducible-at-HEAD mechanism: validate's proposal.md membership
  gate (getActiveChangeIds) vs status/instructions' directory-existence rule
  (validateChangeExists). Pulled nested specs/<area>/<cap> delta discovery and
  bulk --all into scope; noted show.ts sibling.
- #1202: widened from view-only to the shared helper's real blast radius — also
  the archive incomplete-task gate (silently archives unfinished glob-tasks
  changes: data safety) and a 2nd hardcoded copy in change.ts. Pinned apply.tracks
  as the source, change-dir scope containment, and the no-schema fallback. Added
  cli-archive delta for the gate.
- #1156: the main-spec parser discards the requirement header before Zod runs, so
  the hint can't be "lifted" — fix needs header recovery (reuse requirement-blocks)
  + Zod de-dup, and the main-spec message can't be byte-identical to the delta's
  (no ADDED prefix). Pinned the actionable sentence + single-emission + regression
  scenarios across all main-spec surfaces.

4 deltas (cli-validate x2, cli-view, cli-archive). Validates --strict.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(openspec): deep-harden the parity bundle with empirical reproduction

Round 2 of bulletproofing: 3 parallel agents reproduced every bug against the
built (pre-fix) CLI and traced fix sites. This pass corrected two substantive
errors in my own prior spec and closed several gaps.

#1202 (two corrections to the prior draft):
- apply.tracks is a FILENAME that selects the tracked artifact, NOT a glob; the
  glob is that artifact's `generates`. status resolves via
  resolveArtifactOutputs(changeDir, artifact.generates). Fixed all wording.
- "view/archive counts equal status" is FALSE: status checks file EXISTENCE, not
  checkboxes (proven: status calls a 3/5 change isComplete:true). Deleted the two
  count-parity scenarios; reframed as resolution-mechanism parity (same files).
- Added schema-resolution-failure fallback (resolveSchema throws; helper must
  catch or view/list/archive crash). Added projectRoot param + 6-site wiring.
- Empirically PROVEN data-safety bug: archive moved a 3/5 unfinished change into
  changes/archive/.

#1182:
- Found a THIRD getActiveChangeIds site (interactive selector, validate.ts:97).
- Proven: --all with a lone proposal-less change exits 0 silently. Added
  exit-code scenarios. Trimmed over-scope: getSpecIds spec-side is NOT a bug;
  no store-specific scenario needed; noun-form scoped out.

#1156:
- Refine-relaxation regression resolved: deltas don't use the Zod refine
  (validate imperatively), so REMOVE it (not relax) once applySpecRules owns both
  header-only and no-keyword cases. Added RENAMED (out-of-scope), lowercase, and
  the new no-body-line-valid-today scenarios; pinned exact message + prefix.

Still 4 deltas; validates --strict; empirical evidence section added to design.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: converge validate/view/archive onto canonical resolution (#1182, #1202, #1156)

Implements the resolution/validation parity bug bundle planned in
openspec/changes/fix-validate-view-resolution-parity. Each fix points a
divergent read/validate path at the canonical implementation a sibling
command already gets right, with parity tests guarding against re-forking.

#1182 — validate resolves changes like status. validate now resolves a
change by directory existence (shared getAvailableChanges) instead of
requiring proposal.md, at all three sites (targeted, bulk, interactive
selector). A scaffolded/still-authoring change is validated rather than
reported Unknown item; a resolved-but-invalid change exits non-zero.
show.ts and the deprecated noun-form change validate are scoped out.

#1182b — validateChangeDeltaSpecs recurses the nested multi-area layout
(specs/<area>/<capability>/spec.md) via a new findDeltaSpecFiles walker,
so a resolved multi-area change validates its deltas instead of reporting
"No delta sections found".

#1202 — getTaskProgressForChange resolves task progress through the
tracked-tasks artifact's generates glob (the same resolveArtifactOutputs
status uses), aggregating checkboxes across every matched tasks.md scoped
to the change dir, with a never-throw fallback to a single top-level
tasks.md. Updates all four callers (view/list/archive x2) for the new
projectRoot arg and folds the second copy in change.ts onto the helper.
Fixes view's Draft misclassification and the archive incomplete-task
gate that let an unfinished glob-tasks change archive (data safety).

#1156 — the SHALL/MUST body-keyword hint applies to main specs.
applySpecRules recovers the requirement header via extractRequirementsSection
and emits the targeted hint (header-only) or generic message (no keyword),
exactly once; the Zod refine is removed (deltas never used it). The
actionable sentence is byte-identical to the change-delta path.

Adds parity/regression tests (Decision 7): validate<->status resolution
incl. exit code, view/archive resolve the same files as status, and the
main-spec<->delta actionable-sentence parity. Full suite green (1791
passed; only the pre-existing, environment-specific zsh-installer
failures remain). Change validates --strict; all 36 repo specs pass
--specs --strict with no new false positives.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-03 08:00:44 +00:00