Compare commits

..
Author SHA1 Message Date
openspec-release-bot[bot]andgithub-actions[bot] c794978f4f Version Packages (#2112)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-10-17 01:43:43 +00:00
e33245d22b fix(security): replace fast-glob with tinyglobby to drop braces (#2103)
* fix(security): replace fast-glob with tinyglobby to drop braces

Stop publishing the fast-glob → micromatch → braces chain so consumers
no longer inherit GHSA-vfj7-8cjw-p6xm. Nested generates brace patterns are
rejected before expansion, and existing glob patterns still resolve.

Fixes #2051

Co-authored-by: Nick <surlymo@apache.org>

* fix(artifact): expand nested braces in one pass

Peeling the innermost group turned {a,{a,{a,b}}} into a binary tree, so a
16-deep generates pattern missed the 10s timeout on macOS and Windows.
Expand each top-level group with brace-expansion instead. Deeper patterns
are still rejected before expansion.

Co-authored-by: Nick <surlymo@apache.org>

* chore: update flake.nix pnpmDeps hash

Set the hash scripts/update-flake.sh computed for the tinyglobby lockfile.

Co-authored-by: Nick <surlymo@apache.org>

* fix(artifact): expand braces nested inside a literal group

A group like {prefix{a,b}} has no top-level separator, so skipping every
nested group left the inner {a,b} unexpanded. Scan inside a group that
does not expand, and keep a group with nothing to expand as a literal.

Co-authored-by: Nick <surlymo@apache.org>

* fix(artifact): bound brace expansion and keep absolute patterns out

Three gaps in the tinyglobby swap, each verified against main first:

- `{a,b}` repeated 20 times is one brace level deep, so MAX_BRACE_NESTING
  never fires, but the expander builds 1,048,576 patterns and `openspec
  status` dies with "JavaScript heap out of memory". fast-glob never
  expanded that far because micromatch compiled the braces to a regex.
  A `{1..200000}` range cost 3.4s for the same reason. Both now hit a
  1024-pattern budget, checked before the alternatives are pushed so the
  budget bounds memory rather than just the result.
- `globBase` dropped the leading slash of an absolute pattern, so
  `/etc/*.md` resolved inside the change directory, passed the
  containment check and let the glob read a directory the change does
  not own. fast-glob's generateTasks reported `/etc` as the base and the
  check rejected it. The base now stays absolute.
- The expander parks a non-expanding brace group on a sentinel, and
  U+0001 is a legal filename character, so `x<U+0001>y*.md` came back out
  as `x}y*.md` and matched the wrong file. The sentinels are NUL-prefixed
  now; `path.join` in resolveArtifactOutputPath rejects a NUL byte before
  the expander runs, so no pattern can forge one.

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

* test(artifact): keep the control-character fixture off Windows

Windows forbids U+0001 in a filename, so `fs.writeFileSync` for the
`x<U+0001>y.md` fixture would throw before the test reached the resolver
assertion and the windows-pwsh job would fail.

The decoy `x}y.md` — what the old single-character sentinel returned —
is created on every platform and must never match, so the invariant is
still asserted on Windows; only the real file, which cannot exist there,
is conditional.

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

* fix(artifact): cap brace-expansion's allocation at our own budget

assertExpansionBudget ran after expand(), so brace-expansion's own
100,000-result default was the real ceiling for a single group:
`{1..1000000000}` built 100,000 strings before being rejected.

Passing the remaining budget as `max` makes our 1024-pattern budget the
binding one. `max` truncates silently, so it asks for one result more
than the budget allows and treats that length as over budget; a
truncated group never looks literal, because its first alternative is
not the group itself. The accept/reject boundary is unchanged (1024
resolves, 2048 throws) and `{1..200000}` drops from 13ms to 1ms.

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

* docs(schemas): document the 1,024-pattern brace expansion limit

The schema reference documented the 16-level nesting limit but not the new
expansion budget, so a wide pattern's error had no page explaining it. Both
limits now sit together for `generates`, and the `apply.tracks` note names
both, since tracked tasks resolve through the same `generates` glob. The two
documented examples were checked against the built resolver.

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

* docs(schemas): state the brace expansion error on its own

The 1,024-pattern cap was already on the schema page, but the error a
reader searches for was spliced onto the examples. It now stands as its
own sentence, next to the patterns that exceed the budget.

Co-authored-by: Nick <surlymo@apache.org>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Clay Good <hi@claygood.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-17 00:35:10 +00:00
Clay GoodandClaude Opus 5.5 c9a8adef50 fix(website): preview the built site without a server dependency (#2110)
* fix(website): preview the built site without a server dependency

`pnpm start` ran `serve`, a dev-only tool whose transitive tree kept
publishing advisories into the docs site's graph. Four of them had to be
pinned by hand in #2019 (brace-expansion x3, fast-uri) after the Security
workflow went red on main, for code that never reached the published site.

Replace it with a first-party static preview script, which has no tree to
audit, and drop the three pins that existed only to patch serve's tree
(brace-expansion, fast-uri, compression). The build-time pins (postcss,
sharp, nanoid) stay.

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

* fix(test): import the preview script through a file URL for Windows

A bare absolute path is not a valid module specifier on Windows, so
`await import('D:\\a\\OpenSpec\\website\\scripts\\preview.mjs')` failed the suite
with `SyntaxError: Invalid or unexpected token` on windows-pwsh only. Route it
through `pathToFileURL`, and add the release changeset.

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

* fix(test): load the preview script without a shebang or a built path

test/website-preview.test.ts failed to parse on windows-pwsh only, with
`SyntaxError: Invalid or unexpected token`:

- `preview.mjs` opened with `#!/usr/bin/env node`. The file is imported by
  the suite, and `#` is an unexpected token for any parser that does not
  strip it. The script is never executed directly - `pnpm start` runs
  `node scripts/preview.mjs` - and the file is not executable, so the
  shebang was decorative.
- The suite imported it by a path built with `path.join`, which on Windows
  is `D:\\...`: not a module specifier. Use a literal relative specifier, so
  the runner resolves it the same way it resolves every other import here.

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

* fix(website): keep the preview server inside the export and alive on read errors

- A symlink under out/ that pointed outside it passed the lexical prefix
  check, and the file behind it was served. Each candidate is now checked
  again after resolving symlinks, against the resolved export directory;
  404.html goes through the same lookup.
- A file removed or made unreadable between lookup and streaming threw from
  statSync or emitted an unhandled stream error that killed the server.
  Lookup errors are a miss, and headers are sent only once the file is open,
  so a failed read gets a plain 500 (or 404 for 404.html) instead.
- The traversal test wrote and deleted a fixed filename in the shared temp
  directory. The export now sits inside a per-run parent, so the outside
  file is unique to the run.
- The changeset no longer claims Cloudflare routing: out/_redirects is not
  applied.

The symlink-escape and unreadable-file tests fail against the previous
preview.mjs and pass now.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-17 00:30:30 +00:00
Clay GoodandClaude Opus 5 5251f2ef5e fix(validate): stop the spinner flooding a zero-width TTY (#2108)
* fix(validate): stop the spinner flooding a zero-width TTY

Some terminals report a TTY with 0 columns (a pty inherited from a
pre-commit runner such as lefthook, or `script -qec`). ora turns that
into an infinite lines-to-clear count, so the validate spinner cleared
forever: the command never finished and wrote hundreds of megabytes of
cursor escapes.

Skip the spinner when the stream reports no width, and decide via
isInteractive() so OPEN_SPEC_INTERACTIVE=0 and CI disable it the same
way --no-interactive already did.

Closes #2106

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

* chore(changeset): track the validate zero-width spinner fix

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-17 00:30:25 +00:00
276d015090 fix(workflows): keep glob artifact paths inside the change directory (#2104)
* fix(workflows): bound glob artifact paths to changeRoot (#2099)

* fix(workflows): simplify the changeRoot bound and pin it per bullet

Fold the containment rule into the sentence that already picks the
concrete path, instead of appending a second sentence that repeats it.
One clause everywhere: "remains inside `changeRoot` after resolving any
symlinked parent directories".

Replace the whole-body substring checks with per-line assertions: find
the one line in each surface that chooses a concrete path and require
the bound on that line, so moving the rule elsewhere in the file no
longer passes. Include update-change as the oracle and cover
continue-change's partly-written-artifact bullet.

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

* fix(workflows): check a glob path stays in changeRoot before writing to it

propose and ff-change said to create and write the artifact, then verify the
chosen glob path stays inside changeRoot - so a path through a symlinked
parent was already written by the time it was checked. The same held for
continue-change's ready branch, and its partial-artifact step verified each
expected path without saying to refuse one that fails.

Every surface that turns a glob into a write path now chooses the path,
verifies it, refuses (stop and tell the user) on an escape, and only then
writes. explore's capture-as-change step also writes glob artifacts and had no
bound at all, so it gets the same rule.

glob-path-bounding.test.ts now checks the order: the bound must come before
the write on the line, and the refusal must be stated. It fails on the
previous templates for all eight write surfaces and both partial-artifact
bullets. Skill mirror and parity hashes regenerated.

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

* fix(workflows): choose and check the output path before a delegated writer runs

A custom schema can pair a glob output with an instruction that delegates
creation to another skill or command. propose, ff-change, continue-change and
explore invoked that delegated writer first and only then chose and checked a
concrete path - and only on the direct-write branch - so a delegated writer
could still write outside changeRoot. The artifact-creation guidelines also
restated delegation as a bare "invoke it instead", with no bound at all, and
continue-change's partial-artifact backfill said nothing about delegation.

Every creation step now has one order, the one update-change already used:

1. Choose the output path: for a glob, a concrete path inside changeRoot,
   checked after resolving symlinked parents; stop on an escape. Otherwise the
   output path is resolvedOutputPath.
2. If the instruction delegates creation, invoke the delegated writer only if
   it can write to that output path, otherwise stop; then verify the artifact
   exists at that path (not at the glob).
3. Otherwise write the template to that output path.

The guidelines' restatement carries the same bound, and the backfill holds a
delegated writer to each file's checked path.

glob-path-bounding.test.ts now checks, on all ten skill and command surfaces,
that every delegation is limited to one path and refuses otherwise, that the
creation step's delegation comes after the path choice, that nothing targets
the glob itself, and that the guideline restatement is bounded. 22 of these
assertions fail on the previous templates. Skill mirror and parity hashes
regenerated; the #777 test pins the new wording.

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

* fix(workflows): resolve the destination file too, and never write over a delegated artifact

Two gaps alfred found in the workflow instructions:

- The containment check resolved symlinked parent directories only. A
  chosen path inside changeRoot whose file already exists as a symlink to a
  file outside it passed the check, and the write went through the link. The
  rule in propose, ff-change, continue-change (both branches and the
  partial-artifact backfill) and explore now resolves symlinks in the parent
  directories and in the file itself if it exists. update-change keeps its
  rule: it creates with an operation that fails if the target exists, a link
  included.
- continue-change followed the delegated-creation branch with an
  unconditional "Write the file to that output path", so the template could
  overwrite what the delegated skill or command had just created. The
  template steps now sit inside the Otherwise branch, and the artifact is
  verified at the output path after either branch.

glob-path-bounding.test.ts now pins the file-level rule on every direct-write
surface, that every template write sits in the Otherwise branch (judged per
sentence, so explore's one-paragraph step counts), and that continue-change
verifies after either branch. These fail on the previous templates. Skill
mirror and parity hashes regenerated; the #777 test pins the split wording.

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

---------

Co-authored-by: Clay Good <hi@claygood.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-17 00:30:16 +00:00
Clay GoodandClaude Opus 5 87a69a2bf8 fix(specs): accept a spec.md symlink to a file elsewhere in the project (#2101)
* fix(specs): accept a spec.md symlink to a file elsewhere in the project

A capability's main spec could not be a symlink to the file the project
keeps somewhere else, such as beside the product documentation it belongs
to: `list --specs`, `show`, `validate` and `archive` all failed with
"Path is outside the allowed directory".

The containment check for a `spec.md` canonicalized the file before
testing it and allowed only two boundaries, the specs root or the
capability directory, so any link out of the specs tree was refused even
when its target stayed in the same repository.

The project is now the outer boundary for such a link: the capability
directory holding it must still be reached from inside the specs root,
the link must resolve, and a target outside the project is still refused.
Where a specs root has no project to name (a store given by path), the
rule stays as it was. The merge writes through the link, so the docs file
and OpenSpec keep sharing the same bytes.

Closes #2100

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

* fix(specs): anchor the project boundary on a real planning root

Hardening from a second pass over #2100.

The boundary was the parent of the nearest `openspec` ancestor, which
also matched a directory that merely shares the name: a store kept at
`~/openspec/my-store` handed out `~`, so a spec link anywhere under the
home directory was accepted. The directory below the match must now be
`specs` or `changes`, which is what makes it a planning root.

Also adds the cases this pass checked by hand: the boundary derived from
an active and an archived change's delta specs, a leaf link repointed out
of the project between resolve and read and between resolve and write,
and the store-shaped layout above.

Replaces a check-then-use pair in the archive test with a single
O_NOFOLLOW handle, which is what CodeQL flagged on the PR.

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

* test(specs): guard the reported layout through the real CLI

The regression this PR fixes was invisible to the unit tests of the day
because their fixtures put "outside the specs root" and "outside the
project" at the same path, so the boundary could move without any test
noticing. A test that drives the commands a user actually runs does not
depend on which layer the boundary lives in.

Builds the layout from #2100 - a main spec kept beside the product docs,
reached by a relative symlink - and pins both directions: `list --specs`,
`show`, `validate` by id and via `--specs`, and `archive` merging through
the link accept it; the same commands refuse a link that leaves the
project, and the refused archive writes nothing.

Checked the guard fails without the fix (4 of 9 cases) and that neither
direction is vacuous: removing the project-boundary check fails 9 cases,
and removing the planning-root anchor fails exactly the store case.

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

* fix(specs): pin the verified target when writing through a spec link

A capability's spec.md may be a link to the file the project keeps
elsewhere, so the write follows a link by design. The containment check
ran against the link path and the write then resolved it again, which
left a window: a link repointed in between - including from inside
`beforeMutate`, after archive's fingerprint check has already passed -
would send the write to a file nothing verified.

The link is now resolved once, right after the check, and the read and
the write both use that pinned path. The bytes land where a path-based
write would have put them, so the in-place semantics are unchanged.

Found by review on the PR. Also adds the regression test, which fails
when the write goes back to resolving the path itself.

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

* fix(specs): re-check the spec target after beforeMutate

Pinning the resolved pathname was not enough on its own: that pathname
can itself be replaced by a link to somewhere else, and `beforeMutate`
awaits arbitrary work, so it is the widest gap between the containment
check and the write.

The target is re-checked after the callback and refused unless it still
resolves to the file that was verified. This is not specific to a linked
spec.md - an ordinary openspec/specs/<capability>/spec.md swapped for a
link had the same window - so both cases are now covered by tests, and
both fail if the re-check is removed.

Raised by review on the PR.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-17 00:30:13 +00:00
48 changed files with 2028 additions and 956 deletions
+32
View File
@@ -1,5 +1,37 @@
# @fission-ai/openspec
## 1.14.3
### Patch Changes
- [#2101](https://github.com/Fission-AI/OpenSpec/pull/2101) [`87a69a2`](https://github.com/Fission-AI/OpenSpec/commit/87a69a2bf8eb3315a8c042d001520eb7766ab580) Thanks [@clay-good](https://github.com/clay-good)! - ### Bug Fixes
- **A capability's `spec.md` may be a symlink to a file elsewhere in the project** — `list --specs`, `show`, `validate` and `archive` no longer refuse `openspec/specs/<capability>/spec.md` when it links to the file the project keeps somewhere else, such as beside the product documentation it belongs to. A link that resolves outside the project is still refused.
- [#2103](https://github.com/Fission-AI/OpenSpec/pull/2103) [`e33245d`](https://github.com/Fission-AI/OpenSpec/commit/e33245d22ba6f994596a1747baf17130fe04515d) Thanks [@surlymochan](https://github.com/surlymochan)! - ### Bug Fixes
- Installing OpenSpec no longer pulls in the unpatched `braces` package, so a consumer audit does not report GHSA-vfj7-8cjw-p6xm through this package. A `generates` brace pattern that nests deeply, or expands to more than 1024 paths, is now rejected with a clear message instead of crashing `openspec status` with a stack overflow or an out-of-memory error.
- [#2104](https://github.com/Fission-AI/OpenSpec/pull/2104) [`276d015`](https://github.com/Fission-AI/OpenSpec/commit/276d015090965791d8be3d02daf6bfd71de884c3) Thanks [@CRYPTONIKAV](https://github.com/CRYPTONIKAV)! - Keep glob artifact paths inside the change directory in the propose, ff-change and continue-change workflows
- [#2108](https://github.com/Fission-AI/OpenSpec/pull/2108) [`5251f2e`](https://github.com/Fission-AI/OpenSpec/commit/5251f2ef5e9e83eac68dd6950091928c4324b127) Thanks [@clay-good](https://github.com/clay-good)! - Stop `openspec validate --all` hanging on a zero-width TTY. Some hosts hand the
process a terminal that reports `columns === 0` — a pty inherited from a
pre-commit runner such as lefthook, or `script -qec`. ora computes how many
lines to erase as `ceil(width / columns)`, so zero columns makes that count
`Infinity` and the clear loop never ends: the command burned a core and wrote
hundreds of megabytes of cursor escapes without finishing. The spinner is now
skipped when nothing can be drawn, and validate falls back to its plain output.
`OPEN_SPEC_INTERACTIVE=0` and `CI` now disable the spinner too, the same way
`--no-interactive` already did.
- [#2110](https://github.com/Fission-AI/OpenSpec/pull/2110) [`c9a8ade`](https://github.com/Fission-AI/OpenSpec/commit/c9a8adef5021e9793ed9e104f65834fce0fb16cb) Thanks [@clay-good](https://github.com/clay-good)! - Preview the built docs site with a first-party static server instead of `serve`.
`pnpm start` in `website/` now runs `scripts/preview.mjs`, a dependency-free
server that resolves routes in a Next static export (it does not apply
`out/_redirects`). This
drops the `serve` dev dependency whose transitive tree kept publishing
advisories (brace-expansion x3, fast-uri, compression) that had to be pinned by
hand even though none of that code ever reached the published site.
## 1.14.2
### Patch Changes
+1 -1
View File
@@ -41,7 +41,7 @@ Run `pnpm changeset` if your change affects users, and commit the file it genera
Editors, agents and OpenSpec Desktop run the CLI many times, and each call pays for every module it loads before the command runs. Before this rule, `openspec --version` loaded 485 modules: about 0.5 s per call on a Windows machine. So a command loads only the command definitions and its own code:
- **Definitions** (name, options, help text) go in `src/cli/index.ts` or `src/cli/commands/<name>.ts`. Import nothing heavy there: no zod, yaml, fast-glob, ora, and no other command's code.
- **Definitions** (name, options, help text) go in `src/cli/index.ts` or `src/cli/commands/<name>.ts`. Import nothing heavy there: no zod, yaml, tinyglobby, ora, and no other command's code.
- **The command's code** goes in `src/commands/<name>.ts` or `src/core/`, loaded inside the action with `await import()`.
`test/cli-e2e/startup-modules.test.ts` checks which modules each command loads, and fails if a definition starts pulling in an implementation. When you add a command, add it to that test's list.
+2 -2
View File
@@ -84,7 +84,7 @@ OpenSpec recognizes these glob forms in `generates`:
**Literal filenames**: a leading `!` alone does not make a glob. Use `generates: '!review.md'` to name that file. Plain parentheses such as `(proposal|design).md` and single-element braces such as `review-{api}.md` also remain literal.
OpenSpec rejects absolute paths, paths containing a `..` segment, and globs that nest braces or parentheses more than 16 levels deep, such as `{a,{b,{c,...}}}`. Real patterns need one or two levels.
OpenSpec rejects absolute paths, paths containing a `..` segment, and globs that nest braces or parentheses more than 16 levels deep, such as `{a,{b,{c,...}}}`. Real patterns need one or two levels. It also rejects a glob whose braces expand to more than 1,024 patterns, such as `{a,b}` repeated eleven times or a range like `file-{1..5000}.md`. The error says `Artifact output pattern expands to more than 1024 patterns`. Real patterns expand to a handful.
#### Completion
@@ -140,7 +140,7 @@ The path starts from the change folder. For a change named `add-auth`, `tracks:
openspec/changes/add-auth/tasks.md
```
A glob such as `tracks: "**/tasks.md"` reads every matching file, such as `backend/tasks.md` and `frontend/tasks.md`. OpenSpec combines their tasks and progress. Use the same value for an artifact's `generates` field so status and list track the same files. The 16-level brace limit from `generates` applies here too.
A glob such as `tracks: "**/tasks.md"` reads every matching file, such as `backend/tasks.md` and `frontend/tasks.md`. OpenSpec combines their tasks and progress. Use the same value for an artifact's `generates` field so status and list track the same files. The brace limits from `generates` (16 levels deep, 1,024 expanded patterns) apply here too.
Apply stays blocked if no file matches or the matched files contain no checkbox with task text. OpenSpec counts these checkbox forms:
+1 -1
View File
@@ -53,7 +53,7 @@
inherit (finalAttrs) pname version src;
pnpm = final.pnpm_10;
fetcherVersion = 3;
hash = "sha256-hzhk/JEqix4m/k564GbcVuNtoi21E9TkJprPFnPV+Zk=";
hash = "sha256-fKm4sBdTDeQfIiE0k301jtRbiuJrBE7YIEBu8RZxuXI=";
};
nativeBuildInputs = with final; [
@@ -49,7 +49,7 @@ The fix lands in the shared helper `getTaskProgressForChange`, correcting all fo
Two corrections to an earlier draft, both load-bearing:
- **`apply.tracks` is a filename that *selects* the artifact; it is not the glob.** `apply.tracks` is typed `string | null` and is consumed elsewhere as a literal path (`path.join(changeDir, tracks)` + `existsSync`), so `apply.tracks: "**/tasks.md"` cannot match nested files. The glob `status` actually uses is the tracked artifact's **`generates`**, resolved by `resolveArtifactOutputs(changeDir, artifact.generates)`. So the fix identifies the tracked-tasks artifact (the artifact whose `generates` equals `apply.tracks`, falling back to artifact id `tasks` when no `apply` block is present), then counts checkboxes across `resolveArtifactOutputs(changeDir, thatArtifact.generates)`. `resolveArtifactOutputs` roots `fast-glob` at the change directory (so a sibling `changes/archive/` or another change's `tasks.md` cannot match) and de-dups via a `Set` (so no double counting).
- **`apply.tracks` is a filename that *selects* the artifact; it is not the glob.** `apply.tracks` is typed `string | null` and is consumed elsewhere as a literal path (`path.join(changeDir, tracks)` + `existsSync`), so `apply.tracks: "**/tasks.md"` cannot match nested files. The glob `status` actually uses is the tracked artifact's **`generates`**, resolved by `resolveArtifactOutputs(changeDir, artifact.generates)`. So the fix identifies the tracked-tasks artifact (the artifact whose `generates` equals `apply.tracks`, falling back to artifact id `tasks` when no `apply` block is present), then counts checkboxes across `resolveArtifactOutputs(changeDir, thatArtifact.generates)`. `resolveArtifactOutputs` roots its glob at the change directory (so a sibling `changes/archive/` or another change's `tasks.md` cannot match) and de-dups via a `Set` (so no double counting).
- **`status` checks file *existence*, not checkbox completion.** Empirically `status` calls a 3/5 change `4/4 complete, isComplete:true`. So the parity established here is **resolution-mechanism parity** (`view`/`archive` resolve the same set of files `status` resolves), not count parity — `view`/`archive` additionally count checkboxes. Any "view/archive task counts equal status" claim is false and is removed from the spec.
The signature gains `projectRoot` (needed to resolve project-local schemas via `resolveSchema`); all four call sites plus the two `change.ts` sites can derive it. `resolveSchema` **throws** on an unresolvable/misnamed schema, whereas the current helper never throws — so the helper MUST catch and fall back to single-file `tasks.md`, or `view`/`list`/`archive` would crash on a project whose config names a deleted schema. This fallback is specified and tested.
+5 -2
View File
@@ -1,6 +1,6 @@
{
"name": "@fission-ai/openspec",
"version": "1.14.2",
"version": "1.14.3",
"description": "AI-native system for spec-driven development",
"keywords": [
"openspec",
@@ -63,6 +63,7 @@
"@changesets/changelog-github": "^1.0.0",
"@changesets/cli": "^3.0.1",
"@types/node": "^20.19.43",
"@types/picomatch": "^4.0.3",
"@vitest/ui": "^4.1.11",
"eslint": "^10.5.0",
"smol-toml": "^1.7.1",
@@ -73,12 +74,14 @@
"dependencies": {
"@inquirer/core": "^12.0.0",
"@inquirer/prompts": "^8.5.2",
"brace-expansion": "^5.0.12",
"chalk": "^5.6.2",
"commander": "^14.0.0",
"cross-spawn": "7.0.6",
"diff": "^9.0.0",
"fast-glob": "^3.3.3",
"ora": "^9.4.1",
"picomatch": "^4.0.4",
"tinyglobby": "^0.2.17",
"yaml": "^2.8.3",
"zod": "^4.4.3"
}
+21 -137
View File
@@ -5,7 +5,7 @@ settings:
excludeLinksFromLockfile: false
overrides:
brace-expansion@<=5.0.8: '>=5.0.9 <6'
brace-expansion@<5.0.12: '>=5.0.12 <6'
postcss@<8.5.23: '>=8.5.23 <9'
js-yaml@>=3.0.0 <3.15.1: '>=3.15.1 <4'
js-yaml@>=4.0.0 <4.3.1: '>=4.3.1 <5'
@@ -23,6 +23,9 @@ importers:
'@inquirer/prompts':
specifier: ^8.5.2
version: 8.5.2(@types/node@20.19.43)
brace-expansion:
specifier: ^5.0.12
version: 5.0.12
chalk:
specifier: ^5.6.2
version: 5.6.2
@@ -35,12 +38,15 @@ importers:
diff:
specifier: ^9.0.0
version: 9.0.0
fast-glob:
specifier: ^3.3.3
version: 3.3.3
ora:
specifier: ^9.4.1
version: 9.4.1
picomatch:
specifier: ^4.0.4
version: 4.0.7
tinyglobby:
specifier: ^0.2.17
version: 0.2.17
yaml:
specifier: ^2.8.3
version: 2.9.1
@@ -57,6 +63,9 @@ importers:
'@types/node':
specifier: ^20.19.43
version: 20.19.43
'@types/picomatch':
specifier: ^4.0.3
version: 4.0.3
'@vitest/ui':
specifier: ^4.1.11
version: 4.1.11(vitest@4.1.11)
@@ -541,18 +550,6 @@ packages:
os: [linux]
libc: [glibc]
'@nodelib/fs.scandir@2.1.5':
resolution: {integrity: sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==}
engines: {node: '>= 8'}
'@nodelib/fs.stat@2.0.5':
resolution: {integrity: sha512-RkhPPp2zrqDAQA/2jNhnztcPAlv64XdhIp7a7454A5ovI7Bukxgt7MX7udwAu3zg1DcpPU0rz3VV1SeaqvY4+A==}
engines: {node: '>= 8'}
'@nodelib/fs.walk@1.2.8':
resolution: {integrity: sha512-oGB+UxlgWcgQkgwo8GcEGwemoTFt3FIO9ababBmaGwXIoBKZ+GTy0pP185beGg7Llih/NSHSV2XAs1lnznocSg==}
engines: {node: '>= 8'}
'@pnpm/deps.graph-sequencer@1100.0.1':
resolution: {integrity: sha512-pOr5+q1fLYKwFN3LAJuGZEnfXDcQ73zqgDHMtGy+K+uIoUqyY+6MeDCWFwfu+4EFuq76I5EPFofoNAI+Bmmq4A==}
engines: {node: '>=22.13'}
@@ -719,6 +716,9 @@ packages:
'@types/node@20.19.43':
resolution: {integrity: sha512-6oYBAi5ikg4Pl+kGsoYtawUMBT2zZMCvPNF7pVLnHZfd1zf38DRiWn/gT01RYCdUqkv7Fhr+C9ot4/tb+2sVvA==}
'@types/picomatch@4.0.3':
resolution: {integrity: sha512-iG0T6+nYJ9FAPmx9SsUlnwcq1ZVRuCXcVEvWnntoPlrOpwtSTKNDC9uVAxTsC3PUvJ+99n4RpAcNgBbHX3JSnQ==}
'@typescript-eslint/eslint-plugin@8.71.0':
resolution: {integrity: sha512-pqcS9c1HxZTHt7End4nXqd0s5lJrrFzrgCkKFJrsbUnaL6M3+6oBFZaslg6Gjsl3argl2DDRFROnXARaZ2e4Nw==}
engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0}
@@ -837,10 +837,6 @@ packages:
resolution: {integrity: sha512-YovQ3rzhaLMIrDjNDMkNS01tea93qhEhG5xy8f6+R0l+dw3Ki+5sCoIoI942iuLZTHWogWktgwVDhU09iNEimQ==}
engines: {node: 20 || >=22}
braces@3.0.3:
resolution: {integrity: sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA==}
engines: {node: '>=8'}
cac@7.0.0:
resolution: {integrity: sha512-tixWYgm5ZoOD+3g6UTea91eow5z6AAHaho3g0V9CNSNb45gM8SmflpAc+GRd1InC4AqN/07Unrgp56Y94N9hJQ==}
engines: {node: '>=20.19.0'}
@@ -965,10 +961,6 @@ packages:
fast-deep-equal@3.1.3:
resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==}
fast-glob@3.3.3:
resolution: {integrity: sha512-7MptL8U0cqcFdzIzwOTHoilX9x5BrNqye7Z/LuC7kCMRio1EMSyqRK3BEAUD7sXRq4iT4AzTVuZdhgQ2TCvYLg==}
engines: {node: '>=8.6.0'}
fast-json-stable-stringify@2.1.0:
resolution: {integrity: sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==}
@@ -984,9 +976,6 @@ packages:
fast-wrap-ansi@0.2.2:
resolution: {integrity: sha512-7F2Fl+TjRSenLqlU3UjSH0iyqopqoZIu7eZVpEirP2g1GtWa2G/ecEmBdgz31+Mxr+ELclgg6sokpSFIQiZ02Q==}
fastq@1.19.1:
resolution: {integrity: sha512-GwLTyxkCXjXbxqIhTsMI2Nui8huMPtnxg7krajPJAjnEG/iiOS7i+zCtWGZR9G0NBKbXKh6X9m9UIsYX/N6vvQ==}
fdir@6.5.0:
resolution: {integrity: sha512-tIbYtZbucOs0BRGqPJkshJUYdL+SDH7dVM8gjy+ERp3WAUjLEFJE+02kanyHtwjWOnwrKYBiwAmM0p4kLJAnXg==}
engines: {node: '>=12.0.0'}
@@ -1002,10 +991,6 @@ packages:
file-entry-cache@11.1.5:
resolution: {integrity: sha512-+PFTHITI08JIGhnNpGNI8T8inUpgZfk3GNEqfT9R2zZV2iFXg3CvqzSl/uEhs7TSGujYRELEANyDvS8Fj7+S7Q==}
fill-range@7.1.1:
resolution: {integrity: sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg==}
engines: {node: '>=8'}
find-up@5.0.0:
resolution: {integrity: sha512-78/PXT1wlLLDgTzDs7sjq9hzz0vXD+zn+7wypEe4fXQxCmdmqfGsEPQxmiCSQI3ajFV91bVSsvNtrJRiW6nGng==}
engines: {node: '>=10'}
@@ -1025,10 +1010,6 @@ packages:
resolution: {integrity: sha512-QRbvDIbx6YklUe6RxeTeleMR0yv3cYH6PsPZHcnVn7xv7zO1BHN8r0XETu8n6Ye3Q+ahtSarc3WgtNWmehIBfA==}
engines: {node: '>=18'}
glob-parent@5.1.2:
resolution: {integrity: sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==}
engines: {node: '>= 6'}
glob-parent@6.0.2:
resolution: {integrity: sha512-XxwI8EOhVQgWp6iDL+3b0r86f4d6AX6zSU55HfB4ydCEuXLXc5FcYeOu+nnGftS4TEju/11rt4KJPTMgbfmv4A==}
engines: {node: '>=10.13.0'}
@@ -1078,10 +1059,6 @@ packages:
resolution: {integrity: sha512-qP1vozQRI+BMOPcjFzrjXuQvdak2pHNUMZoeG2eRbiSqyvbEf/wQtEOTOX1guk6E3t36RkaqiSt8A/6YElNxLQ==}
engines: {node: '>=12'}
is-number@7.0.0:
resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==}
engines: {node: '>=0.12.0'}
is-unicode-supported@2.1.0:
resolution: {integrity: sha512-mE00Gnza5EEB3Ds0HfMyllZzbBrmLOX3vfWoj9A9PEnTfratQ/BcaJOuMhnkhjXvb2+FkY3VuHqtAGpTPmglFQ==}
engines: {node: '>=18'}
@@ -1122,14 +1099,6 @@ packages:
magic-string@0.30.21:
resolution: {integrity: sha512-vd2F4YUyEXKGcLHoq+TEyCjxueSeHnFxyyjNp80yg0XV4vUhnDer/lvvlqM/arB5bXQN5K2/3oinyCRyx8T2CQ==}
merge2@1.4.1:
resolution: {integrity: sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==}
engines: {node: '>= 8'}
micromatch@4.0.8:
resolution: {integrity: sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==}
engines: {node: '>=8.6'}
mimic-function@5.0.1:
resolution: {integrity: sha512-VP79XUPxV2CigYP3jWwAUFSku2aKqBH7uTAapFWCBqutsbmDo96KY5o8uh6U+/YSIn5OxJnXp73beVkpqMIGhA==}
engines: {node: '>=18'}
@@ -1198,14 +1167,6 @@ packages:
picocolors@1.1.1:
resolution: {integrity: sha512-xceH2snhtb5M9liqDsmEw56le376mTZkEX/jEb/RxNFyegNul7eNslCXP9FDj/Lcu0X8KEyMceP2ntpaHrDEVA==}
picomatch@2.3.2:
resolution: {integrity: sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA==}
engines: {node: '>=8.6'}
picomatch@4.0.5:
resolution: {integrity: sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==}
engines: {node: '>=12'}
picomatch@4.0.7:
resolution: {integrity: sha512-qcJu88Q2IWqJsDD529JKMdwGm/dvInW4HvQnRwiH9JtihJvzGOscDtHE3x1pBKeUOTysQ8kVmLnJ2kJu7yhcGA==}
engines: {node: '>=12'}
@@ -1226,25 +1187,15 @@ packages:
resolution: {integrity: sha512-+Owyggi9IxT1ePKGafcI87ubSmxol6smwJ+RAHDQlx9+9cPwFWDiKFFCPuWhr9ignlGpZ9vDQLw67N4dcTVFEA==}
engines: {node: '>=20'}
queue-microtask@1.2.3:
resolution: {integrity: sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A==}
restore-cursor@5.1.0:
resolution: {integrity: sha512-oMA2dcrw6u0YfxJQXm342bFKX/E4sG9rbTzO9ptUcR/e8A33cHuvStiYOwH7fszkZlZ1z/ta9AAoPk2F4qIOHA==}
engines: {node: '>=18'}
reusify@1.1.0:
resolution: {integrity: sha512-g6QUff04oZpHs0eG5p83rFLhHeV00ug/Yf9nZM6fLeUrPguBTkTQOdpAWWspMh55TZfVQDPaN3NQJfbVRAxdIw==}
engines: {iojs: '>=1.0.0', node: '>=0.10.0'}
rollup@4.64.3:
resolution: {integrity: sha512-iIMWbptLYsZdGTJMH8ngApXdsCatE6bCMv2eqGIfGE1++kwh7TCWBdWKK0FKC0yx++196vGK5VdagzRXSLMa8A==}
engines: {node: '>=18.0.0', npm: '>=8.0.0'}
hasBin: true
run-parallel@1.2.0:
resolution: {integrity: sha512-5l4VyZR86LZ/lDxZTR6jqL8AFE2S0IFLMP26AbjsLVADxHdhB/c0GUsH+y39UfCi3dzz8OlQuPmnaJOMoDHQBA==}
safer-buffer@2.1.2:
resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==}
@@ -1324,10 +1275,6 @@ packages:
resolution: {integrity: sha512-yau8yJdTt989Mm0Bd/236QnzEiPf2xLLTqUZRUJOo/3CB078LSwzei343DgtJVmfJKJE3TMINY1u42SQsP6mXw==}
engines: {node: '>=14.0.0'}
to-regex-range@5.0.1:
resolution: {integrity: sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==}
engines: {node: '>=8.0'}
totalist@3.0.1:
resolution: {integrity: sha512-sf4i37nQ2LBx4m3wB74y+ubopq6W/dIzXg0FDGjsYnZHVa1Da8FH853wlL2gtUhg+xJXjfk3kUZS3BRoQeoQBQ==}
engines: {node: '>=6'}
@@ -1893,18 +1840,6 @@ snapshots:
'@napi-rs/lzma-linux-x64-gnu@1.5.1':
optional: true
'@nodelib/fs.scandir@2.1.5':
dependencies:
'@nodelib/fs.stat': 2.0.5
run-parallel: 1.2.0
'@nodelib/fs.stat@2.0.5': {}
'@nodelib/fs.walk@1.2.8':
dependencies:
'@nodelib/fs.scandir': 2.1.5
fastq: 1.19.1
'@pnpm/deps.graph-sequencer@1100.0.1': {}
'@polka/url@1.0.0-next.29': {}
@@ -2002,6 +1937,8 @@ snapshots:
dependencies:
undici-types: 6.21.0
'@types/picomatch@4.0.3': {}
'@typescript-eslint/eslint-plugin@8.71.0(@typescript-eslint/parser@8.71.0(eslint@10.12.0)(typescript@6.0.3))(eslint@10.12.0)(typescript@6.0.3)':
dependencies:
'@eslint-community/regexpp': 4.12.2
@@ -2166,10 +2103,6 @@ snapshots:
dependencies:
balanced-match: 4.0.4
braces@3.0.3:
dependencies:
fill-range: 7.1.1
cac@7.0.0: {}
cacheable@2.5.0:
@@ -2319,14 +2252,6 @@ snapshots:
fast-deep-equal@3.1.3: {}
fast-glob@3.3.3:
dependencies:
'@nodelib/fs.stat': 2.0.5
'@nodelib/fs.walk': 1.2.8
glob-parent: 5.1.2
merge2: 1.4.1
micromatch: 4.0.8
fast-json-stable-stringify@2.1.0: {}
fast-levenshtein@2.0.6: {}
@@ -2341,14 +2266,6 @@ snapshots:
dependencies:
fast-string-width: 3.0.2
fastq@1.19.1:
dependencies:
reusify: 1.1.0
fdir@6.5.0(picomatch@4.0.5):
optionalDependencies:
picomatch: 4.0.5
fdir@6.5.0(picomatch@4.0.7):
optionalDependencies:
picomatch: 4.0.7
@@ -2359,10 +2276,6 @@ snapshots:
dependencies:
flat-cache: 6.1.23
fill-range@7.1.1:
dependencies:
to-regex-range: 5.0.1
find-up@5.0.0:
dependencies:
locate-path: 6.0.0
@@ -2381,10 +2294,6 @@ snapshots:
get-east-asian-width@1.6.0: {}
glob-parent@5.1.2:
dependencies:
is-glob: 4.0.3
glob-parent@6.0.2:
dependencies:
is-glob: 4.0.3
@@ -2419,8 +2328,6 @@ snapshots:
is-interactive@2.0.0: {}
is-number@7.0.0: {}
is-unicode-supported@2.1.0: {}
isexe@2.0.0: {}
@@ -2460,13 +2367,6 @@ snapshots:
dependencies:
'@jridgewell/sourcemap-codec': 1.6.0
merge2@1.4.1: {}
micromatch@4.0.8:
dependencies:
braces: 3.0.3
picomatch: 2.3.2
mimic-function@5.0.1: {}
minimatch@10.2.6:
@@ -2527,10 +2427,6 @@ snapshots:
picocolors@1.1.1: {}
picomatch@2.3.2: {}
picomatch@4.0.5: {}
picomatch@4.0.7: {}
postcss@8.5.29:
@@ -2547,15 +2443,11 @@ snapshots:
dependencies:
hookified: 2.2.0
queue-microtask@1.2.3: {}
restore-cursor@5.1.0:
dependencies:
onetime: 7.0.0
signal-exit: 4.1.0
reusify@1.1.0: {}
rollup@4.64.3:
dependencies:
'@types/estree': 1.0.9
@@ -2588,10 +2480,6 @@ snapshots:
'@rollup/rollup-win32-x64-msvc': 4.64.3
fsevents: 2.3.3
run-parallel@1.2.0:
dependencies:
queue-microtask: 1.2.3
safer-buffer@2.1.2: {}
semver@7.8.5: {}
@@ -2643,15 +2531,11 @@ snapshots:
tinyglobby@0.2.17:
dependencies:
fdir: 6.5.0(picomatch@4.0.5)
picomatch: 4.0.5
fdir: 6.5.0(picomatch@4.0.7)
picomatch: 4.0.7
tinyrainbow@3.1.1: {}
to-regex-range@5.0.1:
dependencies:
is-number: 7.0.0
totalist@3.0.1: {}
ts-api-utils@2.5.0(typescript@6.0.3):
@@ -2708,7 +2592,7 @@ snapshots:
magic-string: 0.30.21
obug: 2.2.1
pathe: 2.0.3
picomatch: 4.0.5
picomatch: 4.0.7
std-env: 4.2.0
tinybench: 2.9.0
tinyexec: 1.3.0
+4 -13
View File
@@ -10,7 +10,10 @@ allowBuilds:
# plain-name entries in package.json when it bumps the same package, so a mirrored
# copy there both drifts and silently takes precedence over these advisory pins.
overrides:
brace-expansion@<=5.0.8: '>=5.0.9 <6'
# GHSA-6j4f-fj2g-mc7p, GHSA-qhr7-859c-m2p7, GHSA-q2hr-2g5m-vwhr — brace-expansion
# below 5.0.12. Direct dependency, and minimatch. Remove once every copy is
# >=5.0.12 (check: pnpm why brace-expansion).
brace-expansion@<5.0.12: '>=5.0.12 <6'
postcss@<8.5.23: '>=8.5.23 <9'
# GHSA-5p4m-2wfm-xmqj — js-yaml quadratic-CPU !!omap DoS. Dev-only (pulled by
# @changesets: read-yaml-file for 3.x, @changesets/parse for 4.x); never in the
@@ -31,15 +34,3 @@ overrides:
# @changesets/cli > launch-editor); never in the published CLI. Remove once
# transitive shell-quote is >=1.11.0 (check: pnpm why shell-quote).
shell-quote@>=1.8.4 <1.11.0: '>=1.11.0 <2'
auditConfig:
ignoreGhsas:
# GHSA-vfj7-8cjw-p6xm — braces <=3.0.3 stack exhaustion on deeply nested
# patterns. Runtime (fast-glob > micromatch > braces), and no patched braces
# exists, so no override can fix it. Schema artifact `generates` and
# `apply.tracks` patterns reach fast-glob only through resolveArtifactOutputs,
# which rejects brace or parenthesis nesting deeper than 16 levels before globbing. Accepted
# residual risk: braces itself stays unpatched, so any new caller that passes
# it unchecked patterns is exposed. Remove this exception
# once braces ships a patched release (check: pnpm why braces; pnpm audit --prod).
- GHSA-vfj7-8cjw-p6xm
+10 -7
View File
@@ -60,8 +60,8 @@ In both branches, never create the root as a side effect: do not run `openspec i
**Before anything else, finish a partly written artifact**:
- An artifact whose output is a glob pattern (e.g. `specs/**/*.md`) reads `done` as soon as one matching file exists, so a run interrupted while writing its files leaves it `done` with some still missing
- For each such `done` artifact, first get its instructions and read any completed dependencies needed to determine the expected files. Then compare those files with `artifactPaths.<id>.existingOutputPaths`. For spec-driven's `specs`, read the proposal and check for one `specs/<capability-path>/spec.md` per capability it lists; resolve each expected path against `changeRoot` before comparing path identity
- If any are missing, write only the missing files, and STOP - that counts as this invocation's ONE artifact. If you cannot tell whether a file was left out on purpose, ask the user
- For each such `done` artifact, first get its instructions and read any completed dependencies needed to determine the expected files. Then compare those files with `artifactPaths.<id>.existingOutputPaths`. For spec-driven's `specs`, read the proposal and check for one `specs/<capability-path>/spec.md` per capability it lists; resolve each expected path against `changeRoot` before comparing path identity, and verify each one remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists
- Never write an expected path that fails that check; stop and tell the user. If any are missing, write only the missing files, each at its checked path - when the `instruction` field delegates creation to a skill or command, invoke it only if it can write each file to its checked path; otherwise stop and tell the user - and STOP - that counts as this invocation's ONE artifact. If you cannot tell whether a file was left out on purpose, ask the user
---
@@ -89,10 +89,13 @@ In both branches, never create the root as a side effect: do not run `openspec i
- `skipped`/`warning`: present when the change declares skip_specs and this artifact must NOT be created - pick another artifact
- **Create the artifact file**:
- Read any completed dependency files for context - always re-read them from disk, even if you saw them earlier in the conversation (the user may have edited them)
- If the `instruction` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at `resolvedOutputPath`
- Otherwise use `template` as the structure - fill in its sections
- Apply `context` and `rules` as constraints when writing - but do NOT copy them into the file
- Write to the `resolvedOutputPath` specified in instructions. If it is a glob pattern, choose the concrete file path using the schema instruction and the change's context
- Choose the output path before creating anything. If the `resolvedOutputPath` specified in instructions is a glob pattern, choose a concrete path inside `changeRoot` using the schema instruction and the change's context, and verify it remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is `resolvedOutputPath`
- If the `instruction` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user
- Otherwise, write it yourself:
- Use `template` as the structure - fill in its sections
- Apply `context` and `rules` as constraints when writing - but do NOT copy them into the file
- Write the file to that output path
- Either way, verify the artifact file exists at that output path
- Show what was created and what's now unlocked
- STOP after creating ONE artifact
@@ -120,7 +123,7 @@ After each invocation, show:
The artifact types and their purpose depend on the schema. The `instruction` field from the instructions output is the authoritative guidance for each artifact - follow it even when the artifact has a familiar name (proposal.md, tasks.md, etc.), since custom schemas may define different content or a different process for the same file names.
If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly.
If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there.
**Guardrails**
- Create ONE artifact per invocation
+1 -1
View File
@@ -156,7 +156,7 @@ If the user asks you to capture the exploration as a new change, that request is
1. Run `openspec new change "<name>"` (with `--store <id>` when applicable) before creating any artifacts. Never create a new change directory under `openspec/changes/` by hand; the CLI scaffold creates required metadata such as `.openspec.yaml`. Keep the selected `--store <id>` on every applicable follow-up `status` and `instructions` command.
2. Run `openspec status --change "<name>" --json` (append the confirmed `--store "<id>"` only for a registered standalone store), then process the requested artifacts in dependency order. For each requested artifact that is `ready`, run `openspec instructions "<artifact-id>" --change "<name>" --json` (append the confirmed `--store "<id>"` only for a registered standalone store). Before creating a requested artifact, evaluate any condition in its own `instruction` against the explored change; record a deliberate skip instead when the condition does not apply. If a requested artifact is blocked by a direct prerequisite the user did not request, run `openspec instructions "<prerequisite-id>" --change "<name>" --json` (append the confirmed `--store "<id>"` only for a registered standalone store) for that prerequisite whether it is `ready` or `blocked`. If its own `instruction` states a condition, evaluate that condition against the explored change and record a deliberate skip only when the condition does not apply. If the condition applies, or the prerequisite is not conditional, treat it as a normal prerequisite and ask before expanding the capture. Do not create an unrequested prerequisite unless the user approves.
3. Follow the returned `template` and `instruction` fields. Read completed dependency files listed in `dependencies`, and apply `context` and `rules` as constraints without copying them into the artifact. If the instruction delegates creation to a specific skill or command, invoke it; otherwise write the artifact to `resolvedOutputPath`, using the instruction to choose a concrete path when it is a glob. Verify that the selected concrete output exists.
3. Follow the returned `template` and `instruction` fields. Read completed dependency files listed in `dependencies`, and apply `context` and `rules` as constraints without copying them into the artifact. Choose the output path before creating anything: when `resolvedOutputPath` is a glob, use the instruction to choose a concrete path inside `changeRoot` and verify it remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is `resolvedOutputPath`. If the instruction delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Otherwise write the artifact to that output path. Verify that the selected concrete output exists at that output path.
4. After creating each artifact, re-run `openspec status --change "<name>" --json` (append the confirmed `--store "<id>"` only for a registered standalone store) and continue until every requested artifact is `done`, `skipped`, or was deliberately skipped because its own `instruction` stated a condition that did not apply. Tell the user about a deliberate conditional skip, remember it, and do not reconsider it. Dependencies are enablers, not gates: if a requested artifact is still `blocked` only because you deliberately skipped a conditional prerequisite, run `openspec instructions "<artifact-id>" --change "<name>" --json` (append the confirmed `--store "<id>"` only for a registered standalone store) despite the blocked status, then create it using step 3 only when those recorded conditional skips are its sole missing dependencies. If a requested artifact is blocked by a prerequisite the user did not ask to capture and cannot be conditionally skipped, explain that dependency and ask before expanding the capture.
Capture the artifact(s) the user requested without asking them to invoke another workflow command. If they asked only to start a change, stop after scaffolding and show its status. When the requested capture is done, stop there and name where the work continues: `/openspec-propose` writes the remaining planning artifacts, and `/openspec-apply-change` implements the change once tasks exist. Capturing artifacts never starts implementing them.
+4 -3
View File
@@ -76,8 +76,9 @@ In both branches, never create the root as a side effect: do not run `openspec i
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the `instruction` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at `resolvedOutputPath`
- Otherwise create the artifact file using `template` as the structure and write it to `resolvedOutputPath`. If `resolvedOutputPath` is a glob, follow `instruction` to choose the concrete file path
- Choose the output path before creating anything. If `resolvedOutputPath` is a glob, follow `instruction` to choose a concrete path inside `changeRoot` that matches the glob, and verify it remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is `resolvedOutputPath`
- If the `instruction` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using `template` as the structure and write it to that output path
- Apply `context` and `rules` as constraints - but do NOT copy them into the file
- Show brief progress: "✓ Created <artifact-id>"
@@ -111,7 +112,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the `instruction` field from `openspec instructions` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use `template` as the structure for your output file - fill in its sections
+4 -3
View File
@@ -121,8 +121,9 @@ In both branches, never create the root as a side effect: do not run `openspec i
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the `instruction` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at `resolvedOutputPath`
- Otherwise create the artifact file using `template` as the structure and write it to `resolvedOutputPath`. If `resolvedOutputPath` is a glob, follow `instruction` to choose the concrete file path
- Choose the output path before creating anything. If `resolvedOutputPath` is a glob, follow `instruction` to choose a concrete path inside `changeRoot` that matches the glob, and verify it remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is `resolvedOutputPath`
- If the `instruction` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using `template` as the structure and write it to that output path
- Apply `context` and `rules` as constraints - but do NOT copy them into the file
- Show brief progress: "Created <artifact-id>"
@@ -156,7 +157,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the `instruction` field from `openspec instructions` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use `template` as the structure for your output file - fill in its sections
+2 -1
View File
@@ -15,6 +15,7 @@ import {
} from '../utils/nested-change.js';
import { getTaskProgressForChange } from '../utils/task-progress.js';
import { FileSystemUtils } from '../utils/file-system.js';
import { assertDiscoveredSpecFile } from '../utils/spec-path-containment.js';
import { discoverSpecFiles } from '../utils/spec-discovery.js';
import {
foldRequirementName,
@@ -217,7 +218,7 @@ export class ChangeCommand {
const mainSpecPath = path.join(mainSpecsDir, ...capability.split('/'), 'spec.md');
let mainContent: string | null = null;
try {
FileSystemUtils.assertPathWithin(mainSpecsDir, mainSpecPath);
assertDiscoveredSpecFile(mainSpecsDir, mainSpecPath);
mainContent = await fs.readFile(mainSpecPath, 'utf-8');
} catch (error) {
if ((error as NodeJS.ErrnoException)?.code !== 'ENOENT') throw error;
+2 -9
View File
@@ -7,7 +7,7 @@ import type { RootOutput } from '../core/root-selection.js';
import { isInteractive } from '../utils/interactive.js';
import { getSpecIds } from '../utils/item-discovery.js';
import { discoverSpecFiles } from '../utils/spec-discovery.js';
import { FileSystemUtils } from '../utils/file-system.js';
import { assertDiscoveredSpecFile } from '../utils/spec-path-containment.js';
const SPECS_DIR = 'openspec/specs';
@@ -21,14 +21,7 @@ function assertSpecPath(specsDir: string, specPath: string): void {
throw new Error(`Path is outside the allowed directory: ${specPath}`);
}
try {
// Preserve confined spec.md links, including links to a sibling capability.
FileSystemUtils.assertPathWithin(specsDir, specPath);
} catch {
// A capability directory may intentionally be a monorepo symlink. Treat it
// as the trust root while still rejecting a link outside that capability.
FileSystemUtils.assertPathWithin(path.dirname(specPath), specPath);
}
assertDiscoveredSpecFile(specsDir, specPath);
}
export interface ShowOptions {
+3 -3
View File
@@ -15,7 +15,7 @@ import {
type ResolvedOpenSpecRoot,
isStoreSelectedRoot,
} from '../core/root-selection.js';
import { isInteractive, resolveNoInteractive } from '../utils/interactive.js';
import { canRenderSpinner, isInteractive, resolveNoInteractive } from '../utils/interactive.js';
import { getSpecIds } from '../utils/item-discovery.js';
import { getAvailableChanges } from './workflow/shared.js';
import { nearestMatches } from '../utils/match.js';
@@ -441,7 +441,7 @@ export class ValidateCommand {
}
private async runBulkValidation(root: ResolvedOpenSpecRoot, scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string; noInteractive?: boolean; findingsScope?: BulkScope }): Promise<void> {
const spinner = !opts.json && !opts.noInteractive ? ora('Validating...').start() : undefined;
const spinner = !opts.json && isInteractive(opts) && canRenderSpinner() ? ora('Validating...').start() : undefined;
const [changeIds, specIds] = await Promise.all([
scope.changes ? this.listChangeIds(root) : Promise.resolve<string[]>([]),
scope.specs ? getSpecIds(root.path) : Promise.resolve<string[]>([]),
@@ -615,7 +615,7 @@ export class ValidateCommand {
// List first (may throw on a real archive-read failure), then start the
// spinner so a thrown error never leaves a spinner spinning.
const ids = await this.listArchivedChangeIds(root);
const spinner = !opts.json && !opts.noInteractive ? ora('Validating archived changes...').start() : undefined;
const spinner = !opts.json && isInteractive(opts) && canRenderSpinner() ? ora('Validating archived changes...').start() : undefined;
// The archive is append-only and can hold thousands of changes; a single
// run resolves them all under one constant projectRoot (root.path), so
+211 -32
View File
@@ -1,13 +1,31 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import fg from 'fast-glob';
import { expand } from 'brace-expansion';
import picomatch from 'picomatch';
import * as globby from 'tinyglobby';
import { FileSystemUtils } from '../../utils/file-system.js';
const EXTGLOB_RE = /[!*+?@]\([^(]*\)/u;
const BRACE_EXPANSION_SEPARATORS_RE = /,|\.\./u;
// braces <=3.0.3 (fast-glob > micromatch) has no depth guard (GHSA-vfj7-8cjw-p6xm),
// so reject nesting no real artifact pattern needs before it reaches the parser.
const DUPLICATE_SLASH_RE = /(?!^)\/{2,}/gu;
// Deeply nested braces can overflow a recursive expander. Reject nesting no real
// artifact pattern needs before brace-expansion or tinyglobby see the pattern.
const MAX_BRACE_NESTING = 16;
// Nesting depth alone does not bound the work: `{a,b}` repeated 20 times is
// only one level deep and expands to 1,048,576 patterns, which exhausts the
// heap. fast-glob never expanded that far because micromatch compiled the
// braces to a regex. No real artifact pattern needs more than a few hundred.
const MAX_BRACE_EXPANSIONS = 1024;
const ARTIFACT_GLOB_OPTIONS = {
absolute: true,
// Braces are expanded first so `{draft}` left behind stays a literal name.
braceExpansion: false,
// fast-glob migration: directory patterns must not grow a trailing `/**`.
expandDirectories: false,
followSymbolicLinks: true,
onlyFiles: true,
} as const;
function assertBraceNesting(pattern: string): void {
// Mirror the braces parser: `[...]` and quoted runs are literal, `(` nests like
@@ -39,6 +57,160 @@ function assertBraceNesting(pattern: string): void {
}
}
function isDynamicSegment(segment: string): boolean {
return segment.includes('*') || segment.includes('?') || segment.includes('[')
|| EXTGLOB_RE.test(segment);
}
/**
* Static directory prefix of an expanded pattern, equivalent to the `base`
* fast-glob's generateTasks used to reject `../` before walking.
*/
function globBase(pattern: string): string {
const segments = pattern.split('/');
if (segments.length <= 1) return '.';
const base: string[] = [];
for (const segment of segments.slice(0, -1)) {
if (segment === '') continue;
if (isDynamicSegment(segment)) break;
base.push(segment);
}
// An absolute pattern must stay absolute, or `path.resolve(changeDir, base)`
// rebases `/etc/*` inside the change directory and the containment check
// passes for a pattern that globs the filesystem root.
if (pattern.startsWith('/')) return `/${base.join('/')}`;
return base.length > 0 ? base.join('/') : '.';
}
// brace-expansion 5 leaves the rest of a pattern literal when a non-expanding
// group such as `{draft}` comes before a range. Expand one real group at a time.
// NUL-prefixed so no `generates` value can forge them: `resolveArtifactOutputPath`
// runs `path.join` first, which rejects a NUL byte. A single-character sentinel
// such as U+0001 is a legal filename character, and a pattern containing one
// would come back out of the expander as a brace.
const LITERAL_BRACE_OPEN = '\u0000<';
const LITERAL_BRACE_CLOSE = '\u0000>';
function hasTopLevelExpansion(body: string): boolean {
let depth = 0;
for (let index = 0; index < body.length; index += 1) {
const char = body[index];
if (char === '{') depth += 1;
else if (char === '}') depth = Math.max(0, depth - 1);
else if (depth === 0) {
if (char === ',') return true;
if (char === '.' && body[index + 1] === '.') return true;
}
}
return false;
}
function findTopLevelExpansion(pattern: string): { pre: string; body: string; post: string } | null {
const stack: number[] = [];
for (let index = 0; index < pattern.length; index += 1) {
const char = pattern[index];
if (char === '{') {
stack.push(index);
} else if (char === '}' && stack.length > 0) {
const open = stack.pop();
if (open === undefined) continue;
// Only the outermost group that actually expands. brace-expansion expands
// its nested groups in one linear pass. Peeling the innermost `{a,b}` first
// turns `{a,{a,{a,b}}}` into a binary tree, so a 16-deep pattern does not
// finish before the test timeout.
if (stack.length > 0) continue;
const body = pattern.slice(open + 1, index);
if (!hasTopLevelExpansion(body)) {
// `{prefix{a,b}}` has no separator of its own, so the outer braces stay
// literal. Keep scanning inside them; a nested group may still expand.
const nested = findTopLevelExpansion(body);
if (!nested) continue;
return {
pre: `${pattern.slice(0, open + 1)}${nested.pre}`,
body: nested.body,
post: `${nested.post}${pattern.slice(index)}`,
};
}
return { pre: pattern.slice(0, open), body, post: pattern.slice(index + 1) };
}
}
return null;
}
function assertExpansionBudget(pattern: string, count: number): void {
if (count <= MAX_BRACE_EXPANSIONS) return;
// A pattern this wide can run to thousands of characters; quote only its start.
const shown = pattern.length > 80 ? `${pattern.slice(0, 80)}...` : pattern;
throw new Error(
`Artifact output pattern expands to more than ${MAX_BRACE_EXPANSIONS} patterns: ${shown}`
);
}
function expandArtifactBraces(pattern: string): string[] {
if (pattern.includes('\u0000')) {
throw new Error('Artifact output pattern contains a NUL byte');
}
const pending = [pattern];
const finished: string[] = [];
while (pending.length > 0) {
const current = pending.pop();
if (current === undefined) break;
const group = findTopLevelExpansion(current);
if (!group) {
finished.push(current.replaceAll(LITERAL_BRACE_OPEN, '{').replaceAll(LITERAL_BRACE_CLOSE, '}'));
continue;
}
const wrapped = `{${group.body}}`;
// `max` makes this budget the binding one. Without it `{1..1000000000}`
// allocates brace-expansion's own 100,000-result default before the check
// below rejects it. `max` truncates silently, so ask for one result more
// than the budget allows: a result that long means the pattern is over
// budget rather than fully expanded. A truncated group never looks literal,
// because its first alternative is not the group itself.
const remaining = Math.max(MAX_BRACE_EXPANSIONS - (pending.length + finished.length), 0);
const alternatives = expand(wrapped, { max: remaining + 1 });
if (alternatives.length === 1 && alternatives[0] === wrapped) {
pending.push(`${group.pre}${LITERAL_BRACE_OPEN}${group.body}${LITERAL_BRACE_CLOSE}${group.post}`);
continue;
}
// Checked before the push so the budget bounds memory, not just the result.
assertExpansionBudget(pattern, pending.length + finished.length + alternatives.length);
for (const alternative of alternatives) {
pending.push(`${group.pre}${alternative}${group.post}`);
}
}
const normalized = finished
.map((value) => value.replace(DUPLICATE_SLASH_RE, '/'))
.filter((value) => value.length > 0);
return normalized.length > 0 ? normalized : [pattern];
}
/**
* One directory level. picomatch matches the segment; readdir keeps symlink
* entries tinyglobby would drop when followSymbolicLinks is false.
*/
function listDirectoryMatches(directory: string, segment: string): string[] {
let entries: fs.Dirent[];
try {
entries = fs.readdirSync(directory, { withFileTypes: true });
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return [];
throw error;
}
const isMatch = picomatch(segment, {
dot: false,
nobrace: true,
noglobstar: true,
posix: true,
});
const names: string[] = [];
for (const entry of entries) {
if (isMatch(entry.name)) names.push(entry.name);
}
return names;
}
function hasBraceExpansion(pattern: string): boolean {
const openings: number[] = [];
for (let index = 0; index < pattern.length; index += 1) {
@@ -112,12 +284,25 @@ function assertGlobDirectoryTraversal(
);
}
const matches = fg.sync(segment === '**' ? '*' : segment, {
cwd: canonicalDir,
onlyFiles: false,
followSymbolicLinks: false,
deep: 1,
});
// `.` and `..` are not directory entries. Resolve them directly so a
// `**/../` pattern is confined before any glob runs.
if (segment === '' || segment === '.' || segment === '..') {
const candidate = path.resolve(canonicalDir, segment === '' ? '.' : segment);
const canonicalCandidate = FileSystemUtils.canonicalizeExistingPath(candidate);
FileSystemUtils.assertPathWithin(canonicalChangeDir, canonicalCandidate);
assertGlobDirectoryTraversal(
changeDir,
canonicalCandidate,
directorySegments,
segmentIndex + 1,
visited,
canonicalChangeDir,
ancestors
);
return;
}
const matches = listDirectoryMatches(canonicalDir, segment === '**' ? '*' : segment);
for (const match of matches) {
const candidate = path.join(canonicalDir, match);
try {
@@ -162,31 +347,25 @@ export function resolveArtifactOutputs(changeDir: string, generates: string): st
}
}
const normalizedPattern = FileSystemUtils.toPosixPath(generates);
const globOptions = {
cwd: changeDir,
onlyFiles: true,
absolute: true,
// Preserve linked artifact directories; confine traversal and concrete matches.
followSymbolicLinks: true,
};
// Task generation expands braces without accessing the filesystem. Validate
// every task base before globbing, including paths introduced by expansion.
const tasks = fg.generateTasks(normalizedPattern, globOptions);
for (const task of tasks) {
FileSystemUtils.assertPathWithin(changeDir, path.resolve(changeDir, task.base));
const normalizedPattern = FileSystemUtils.toPosixPath(generates).replace(DUPLICATE_SLASH_RE, '/');
// Expand braces before any filesystem access. tinyglobby has no generateTasks,
// and a `../` alternative is invisible until the braces are expanded.
const patterns = expandArtifactBraces(normalizedPattern);
for (const pattern of patterns) {
FileSystemUtils.assertPathWithin(changeDir, path.resolve(changeDir, globBase(pattern)));
}
for (const task of tasks) {
for (const positivePattern of task.positive) {
assertGlobDirectoryTraversal(
changeDir,
changeDir,
positivePattern.split('/').slice(0, -1)
);
}
for (const pattern of patterns) {
assertGlobDirectoryTraversal(
changeDir,
changeDir,
pattern.split('/').slice(0, -1)
);
}
const matches = fg
.sync(normalizedPattern, globOptions)
const matches = globby
.globSync(patterns, {
cwd: changeDir,
...ARTIFACT_GLOB_OPTIONS,
})
.map((match) => {
const normalizedMatch = path.normalize(match);
FileSystemUtils.assertPathWithin(changeDir, normalizedMatch);
+1 -1
View File
@@ -97,7 +97,7 @@ export class CompletionProvider {
// Fetch fresh data. The resolver (and the yaml and zod it parses schemas
// with) loads only here: completing changes or specs never needs it, and
// the artifact-graph barrel would also pull in fast-glob.
// the artifact-graph barrel would also pull in tinyglobby.
const { listSchemas } = await import('../artifact-graph/resolver.js');
const schemaNames = listSchemas(this.projectRoot);
+25 -5
View File
@@ -30,6 +30,7 @@ import {
} from './validation/constants.js';
import { discoverSpecFiles } from '../utils/spec-discovery.js';
import { FileSystemUtils } from '../utils/file-system.js';
import { assertSpecFileWithin } from '../utils/spec-path-containment.js';
import { matchLineEnding } from '../utils/line-endings.js';
// -----------------------------------------------------------------------------
@@ -72,7 +73,7 @@ function resolveTrustedSpecPath(
try {
// Preserve spec.md links that remain inside the overall specs tree.
FileSystemUtils.assertPathWithin(specsRoot, specPath);
assertSpecFileWithin(specsRoot, specPath);
const root = FileSystemUtils.canonicalizeExistingPath(specsRoot);
return {
root,
@@ -97,7 +98,7 @@ function resolveTrustedSpecPath(
);
}
const file = path.join(root, path.basename(specPath));
FileSystemUtils.assertPathWithin(root, file);
assertSpecFileWithin(root, file);
return { root, file };
}
}
@@ -106,7 +107,7 @@ function assertTrustedSpecPath(root: string, specPath: string): void {
if (FileSystemUtils.canonicalizeExistingPath(root) !== path.resolve(root)) {
throw new Error(`Path is outside the allowed directory: ${specPath}`);
}
FileSystemUtils.assertPathWithin(root, specPath);
assertSpecFileWithin(root, specPath);
}
// -----------------------------------------------------------------------------
@@ -1247,6 +1248,14 @@ export async function writeUpdatedSpec(
} = {}
): Promise<void> {
assertTrustedSpecPath(update.targetRoot, update.target);
// Pin what the assertion just checked. A capability's spec.md may be a link
// to the file the project keeps elsewhere, so the write follows a link by
// design - and resolving it once, here, is what keeps that from becoming a
// redirect: a link repointed after this line, including from inside
// `beforeMutate`, cannot send the write to a file nothing verified. The
// bytes still land in the same place a path-based write would have put them,
// which keeps the in-place semantics below intact.
const verifiedTarget = FileSystemUtils.canonicalizeExistingPath(update.target);
// Create target directory if needed
const targetDir = path.dirname(update.target);
@@ -1258,17 +1267,28 @@ export async function writeUpdatedSpec(
// requirement moved. A spec that does not exist yet stays LF.
// Only a missing file means "no convention to match". Swallowing every error
// would read an existing but unreadable spec as absent and rewrite it as LF.
const previous = await fs.readFile(update.target, 'utf-8').catch((error) => {
const previous = await fs.readFile(verifiedTarget, 'utf-8').catch((error) => {
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined;
throw error;
});
const toWrite = previous === undefined ? rebuilt : matchLineEnding(rebuilt, previous);
await options.beforeMutate?.();
// `beforeMutate` awaits arbitrary work, so it is the widest gap between the
// check above and the write below. Re-check there, and refuse unless the
// target still resolves to the very file that was verified: pinning a
// pathname is not enough on its own, because the pinned path can itself be
// replaced by a link to somewhere else.
assertTrustedSpecPath(update.targetRoot, update.target);
if (FileSystemUtils.canonicalizeExistingPath(update.target) !== verifiedTarget) {
throw new Error(
`Spec target for '${update.id}' changed before it could be written: ${update.target}`
);
}
// Preserve the established in-place write semantics: symlink referents,
// hard-linked specs, ACLs, extended attributes, and filesystems without hard
// links must continue to behave as they did before capability retirement.
await fs.writeFile(update.target, toWrite);
await fs.writeFile(verifiedTarget, toWrite);
if (options.silent) return;
const specName = update.id;
+18 -12
View File
@@ -31,8 +31,8 @@ const PLANNING_COMPLETE_HANDOFF = optionalWorkflow(
*/
const PARTIAL_ARTIFACT_CHECK = `**Before anything else, finish a partly written artifact**:
- An artifact whose output is a glob pattern (e.g. \`specs/**/*.md\`) reads \`done\` as soon as one matching file exists, so a run interrupted while writing its files leaves it \`done\` with some still missing
- For each such \`done\` artifact, first get its instructions and read any completed dependencies needed to determine the expected files. Then compare those files with \`artifactPaths.<id>.existingOutputPaths\`. For spec-driven's \`specs\`, read the proposal and check for one \`specs/<capability-path>/spec.md\` per capability it lists; resolve each expected path against \`changeRoot\` before comparing path identity
- If any are missing, write only the missing files, and STOP - that counts as this invocation's ONE artifact. If you cannot tell whether a file was left out on purpose, ask the user`;
- For each such \`done\` artifact, first get its instructions and read any completed dependencies needed to determine the expected files. Then compare those files with \`artifactPaths.<id>.existingOutputPaths\`. For spec-driven's \`specs\`, read the proposal and check for one \`specs/<capability-path>/spec.md\` per capability it lists; resolve each expected path against \`changeRoot\` before comparing path identity, and verify each one remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists
- Never write an expected path that fails that check; stop and tell the user. If any are missing, write only the missing files, each at its checked path - when the \`instruction\` field delegates creation to a skill or command, invoke it only if it can write each file to its checked path; otherwise stop and tell the user - and STOP - that counts as this invocation's ONE artifact. If you cannot tell whether a file was left out on purpose, ask the user`;
export function getContinueChangeSkillTemplate(): SkillTemplate {
return {
@@ -106,10 +106,13 @@ ${PROJECT_ROOT_GUARD}
- \`skipped\`/\`warning\`: present when the change declares skip_specs and this artifact must NOT be created - pick another artifact
- **Create the artifact file**:
- Read any completed dependency files for context - always re-read them from disk, even if you saw them earlier in the conversation (the user may have edited them)
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise use \`template\` as the structure - fill in its sections
- Apply \`context\` and \`rules\` as constraints when writing - but do NOT copy them into the file
- Write to the \`resolvedOutputPath\` specified in instructions. If it is a glob pattern, choose the concrete file path using the schema instruction and the change's context
- Choose the output path before creating anything. If the \`resolvedOutputPath\` specified in instructions is a glob pattern, choose a concrete path inside \`changeRoot\` using the schema instruction and the change's context, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user
- Otherwise, write it yourself:
- Use \`template\` as the structure - fill in its sections
- Apply \`context\` and \`rules\` as constraints when writing - but do NOT copy them into the file
- Write the file to that output path
- Either way, verify the artifact file exists at that output path
- Show what was created and what's now unlocked
- STOP after creating ONE artifact
@@ -137,7 +140,7 @@ After each invocation, show:
The artifact types and their purpose depend on the schema. The \`instruction\` field from the instructions output is the authoritative guidance for each artifact - follow it even when the artifact has a familiar name (proposal.md, tasks.md, etc.), since custom schemas may define different content or a different process for the same file names.
If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly.
If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there.
**Guardrails**
- Create ONE artifact per invocation
@@ -229,10 +232,13 @@ ${PROJECT_ROOT_GUARD}
- \`skipped\`/\`warning\`: present when the change declares skip_specs and this artifact must NOT be created - pick another artifact
- **Create the artifact file**:
- Read any completed dependency files for context - always re-read them from disk, even if you saw them earlier in the conversation (the user may have edited them)
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise use \`template\` as the structure - fill in its sections
- Apply \`context\` and \`rules\` as constraints when writing - but do NOT copy them into the file
- Write to the \`resolvedOutputPath\` specified in instructions. If it is a glob pattern, choose the concrete file path using the schema instruction and the change's context
- Choose the output path before creating anything. If the \`resolvedOutputPath\` specified in instructions is a glob pattern, choose a concrete path inside \`changeRoot\` using the schema instruction and the change's context, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user
- Otherwise, write it yourself:
- Use \`template\` as the structure - fill in its sections
- Apply \`context\` and \`rules\` as constraints when writing - but do NOT copy them into the file
- Write the file to that output path
- Either way, verify the artifact file exists at that output path
- Show what was created and what's now unlocked
- STOP after creating ONE artifact
@@ -260,7 +266,7 @@ After each invocation, show:
The artifact types and their purpose depend on the schema. The \`instruction\` field from the instructions output is the authoritative guidance for each artifact - follow it even when the artifact has a familiar name (proposal.md, tasks.md, etc.), since custom schemas may define different content or a different process for the same file names.
If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly.
If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there.
**Guardrails**
- Create ONE artifact per invocation
+2 -2
View File
@@ -195,7 +195,7 @@ If the user asks you to capture the exploration as a new change, that request is
1. Run \`openspec new change "<name>"\` (with \`--store <id>\` when applicable) before creating any artifacts. Never create a new change directory under \`openspec/changes/\` by hand; the CLI scaffold creates required metadata such as \`.openspec.yaml\`. Keep the selected \`--store <id>\` on every applicable follow-up \`status\` and \`instructions\` command.
2. Run \`openspec status --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store), then process the requested artifacts in dependency order. For each requested artifact that is \`ready\`, run \`openspec instructions "<artifact-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store). Before creating a requested artifact, evaluate any condition in its own \`instruction\` against the explored change; record a deliberate skip instead when the condition does not apply. If a requested artifact is blocked by a direct prerequisite the user did not request, run \`openspec instructions "<prerequisite-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) for that prerequisite whether it is \`ready\` or \`blocked\`. If its own \`instruction\` states a condition, evaluate that condition against the explored change and record a deliberate skip only when the condition does not apply. If the condition applies, or the prerequisite is not conditional, treat it as a normal prerequisite and ask before expanding the capture. Do not create an unrequested prerequisite unless the user approves.
3. Follow the returned \`template\` and \`instruction\` fields. Read completed dependency files listed in \`dependencies\`, and apply \`context\` and \`rules\` as constraints without copying them into the artifact. If the instruction delegates creation to a specific skill or command, invoke it; otherwise write the artifact to \`resolvedOutputPath\`, using the instruction to choose a concrete path when it is a glob. Verify that the selected concrete output exists.
3. Follow the returned \`template\` and \`instruction\` fields. Read completed dependency files listed in \`dependencies\`, and apply \`context\` and \`rules\` as constraints without copying them into the artifact. Choose the output path before creating anything: when \`resolvedOutputPath\` is a glob, use the instruction to choose a concrete path inside \`changeRoot\` and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`. If the instruction delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Otherwise write the artifact to that output path. Verify that the selected concrete output exists at that output path.
4. After creating each artifact, re-run \`openspec status --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) and continue until every requested artifact is \`done\`, \`skipped\`, or was deliberately skipped because its own \`instruction\` stated a condition that did not apply. Tell the user about a deliberate conditional skip, remember it, and do not reconsider it. Dependencies are enablers, not gates: if a requested artifact is still \`blocked\` only because you deliberately skipped a conditional prerequisite, run \`openspec instructions "<artifact-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) despite the blocked status, then create it using step 3 only when those recorded conditional skips are its sole missing dependencies. If a requested artifact is blocked by a prerequisite the user did not ask to capture and cannot be conditionally skipped, explain that dependency and ask before expanding the capture.
Capture the artifact(s) the user requested without asking them to invoke another workflow command. If they asked only to start a change, stop after scaffolding and show its status. When the requested capture is done, stop there and name where the work continues: ${CAPTURE_PLANNING_HANDOFF}, and ${CAPTURE_APPLY_HANDOFF}. Capturing artifacts never starts implementing them.
@@ -531,7 +531,7 @@ If the user asks you to capture the exploration as a new change, that request is
1. Run \`openspec new change "<name>"\` (with \`--store <id>\` when applicable) before creating any artifacts. Never create a new change directory under \`openspec/changes/\` by hand; the CLI scaffold creates required metadata such as \`.openspec.yaml\`. Keep the selected \`--store <id>\` on every applicable follow-up \`status\` and \`instructions\` command.
2. Run \`openspec status --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store), then process the requested artifacts in dependency order. For each requested artifact that is \`ready\`, run \`openspec instructions "<artifact-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store). Before creating a requested artifact, evaluate any condition in its own \`instruction\` against the explored change; record a deliberate skip instead when the condition does not apply. If a requested artifact is blocked by a direct prerequisite the user did not request, run \`openspec instructions "<prerequisite-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) for that prerequisite whether it is \`ready\` or \`blocked\`. If its own \`instruction\` states a condition, evaluate that condition against the explored change and record a deliberate skip only when the condition does not apply. If the condition applies, or the prerequisite is not conditional, treat it as a normal prerequisite and ask before expanding the capture. Do not create an unrequested prerequisite unless the user approves.
3. Follow the returned \`template\` and \`instruction\` fields. Read completed dependency files listed in \`dependencies\`, and apply \`context\` and \`rules\` as constraints without copying them into the artifact. If the instruction delegates creation to a specific skill or command, invoke it; otherwise write the artifact to \`resolvedOutputPath\`, using the instruction to choose a concrete path when it is a glob. Verify that the selected concrete output exists.
3. Follow the returned \`template\` and \`instruction\` fields. Read completed dependency files listed in \`dependencies\`, and apply \`context\` and \`rules\` as constraints without copying them into the artifact. Choose the output path before creating anything: when \`resolvedOutputPath\` is a glob, use the instruction to choose a concrete path inside \`changeRoot\` and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`. If the instruction delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Otherwise write the artifact to that output path. Verify that the selected concrete output exists at that output path.
4. After creating each artifact, re-run \`openspec status --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) and continue until every requested artifact is \`done\`, \`skipped\`, or was deliberately skipped because its own \`instruction\` stated a condition that did not apply. Tell the user about a deliberate conditional skip, remember it, and do not reconsider it. Dependencies are enablers, not gates: if a requested artifact is still \`blocked\` only because you deliberately skipped a conditional prerequisite, run \`openspec instructions "<artifact-id>" --change "<name>" --json\` (append the confirmed \`--store "<id>"\` only for a registered standalone store) despite the blocked status, then create it using step 3 only when those recorded conditional skips are its sole missing dependencies. If a requested artifact is blocked by a prerequisite the user did not ask to capture and cannot be conditionally skipped, explain that dependency and ask before expanding the capture.
Capture the artifact(s) the user requested without asking them to invoke another workflow command. If they asked only to start a change, stop after scaffolding and show its status. When the requested capture is done, stop there and name where the work continues: ${CAPTURE_PLANNING_HANDOFF}, and ${CAPTURE_APPLY_HANDOFF}. Capturing artifacts never starts implementing them.
+8 -6
View File
@@ -91,8 +91,9 @@ ${PROJECT_ROOT_GUARD}
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise create the artifact file using \`template\` as the structure and write it to \`resolvedOutputPath\`. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose the concrete file path
- Choose the output path before creating anything. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose a concrete path inside \`changeRoot\` that matches the glob, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using \`template\` as the structure and write it to that output path
- Apply \`context\` and \`rules\` as constraints - but do NOT copy them into the file
- Show brief progress: "✓ Created <artifact-id>"
@@ -126,7 +127,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the \`instruction\` field from \`openspec instructions\` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use \`template\` as the structure for your output file - fill in its sections
@@ -210,8 +211,9 @@ ${PROJECT_ROOT_GUARD}
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise create the artifact file using \`template\` as the structure and write it to \`resolvedOutputPath\`. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose the concrete file path
- Choose the output path before creating anything. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose a concrete path inside \`changeRoot\` that matches the glob, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using \`template\` as the structure and write it to that output path
- Apply \`context\` and \`rules\` as constraints - but do NOT copy them into the file
- Show brief progress: "✓ Created <artifact-id>"
@@ -245,7 +247,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the \`instruction\` field from \`openspec instructions\` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use \`template\` as the structure for your output file - fill in its sections
+8 -6
View File
@@ -137,8 +137,9 @@ ${PROJECT_ROOT_GUARD}
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise create the artifact file using \`template\` as the structure and write it to \`resolvedOutputPath\`. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose the concrete file path
- Choose the output path before creating anything. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose a concrete path inside \`changeRoot\` that matches the glob, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using \`template\` as the structure and write it to that output path
- Apply \`context\` and \`rules\` as constraints - but do NOT copy them into the file
- Show brief progress: "Created <artifact-id>"
@@ -172,7 +173,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the \`instruction\` field from \`openspec instructions\` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use \`template\` as the structure for your output file - fill in its sections
@@ -302,8 +303,9 @@ ${PROJECT_ROOT_GUARD}
- Identify the target project from the request and project context; the planning home may be separate from the code. If the target is unclear, ask. For greenfield or non-code changes, inspect the available structure and relevant documents. If source is unavailable, state the limitation and ask when it materially affects the plan.
- Ground scope, approach, and tasks in what you find. Distinguish observed behavior from assumptions and proposed additions; surface conflicts with existing specs instead of silently deciding which is correct.
- Do this discovery now, rather than leaving generic "explore the codebase" or "make a plan" tasks for implementation. Keep any necessary follow-up investigation specific to an unresolved question.
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at \`resolvedOutputPath\`
- Otherwise create the artifact file using \`template\` as the structure and write it to \`resolvedOutputPath\`. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose the concrete file path
- Choose the output path before creating anything. If \`resolvedOutputPath\` is a glob, follow \`instruction\` to choose a concrete path inside \`changeRoot\` that matches the glob, and verify it remains inside \`changeRoot\` after resolving symlinks, both in its parent directories and in the file itself if it already exists; if it does not, stop and tell the user. Otherwise the output path is \`resolvedOutputPath\`
- If the \`instruction\` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user. Then verify the artifact file exists at that output path
- Otherwise create the artifact file using \`template\` as the structure and write it to that output path
- Apply \`context\` and \`rules\` as constraints - but do NOT copy them into the file
- Show brief progress: "Created <artifact-id>"
@@ -337,7 +339,7 @@ After completing all artifacts, summarize:
**Artifact Creation Guidelines**
- Follow the \`instruction\` field from \`openspec instructions\` for each artifact type - it is the authoritative guidance, even for familiar artifact names
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly
- If the \`instruction\` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there
- The schema defines what each artifact should contain - follow it
- Read dependency artifacts for context before creating new ones
- Use \`template\` as the structure for your output file - fill in its sections
+3 -3
View File
@@ -29,6 +29,7 @@ import {
} from '../parsers/requirement-text.js';
import { findMainSpecStructureIssues } from '../parsers/spec-structure.js';
import { FileSystemUtils } from '../../utils/file-system.js';
import { assertDiscoveredSpecFile } from '../../utils/spec-path-containment.js';
import { discoverSpecFiles, findUnreadDeltaFiles, hasAnyFileUnder } from '../../utils/spec-discovery.js';
import {
METADATA_FILENAME,
@@ -376,14 +377,13 @@ export class Validator {
...specId.split('/'),
'spec.md'
);
FileSystemUtils.assertPathWithin(path.dirname(mainSpecFile), mainSpecFile);
issues.push(
...(await this.findScenarioLossIssues(
plan.modified,
plan.renamed,
mainSpecFile,
entryPath,
path.dirname(mainSpecFile)
options.mainSpecsDir
))
);
}
@@ -710,7 +710,7 @@ export class Validator {
mainSpecRoot: string
): Promise<ValidationIssue[]> {
let mainContent: string;
FileSystemUtils.assertPathWithin(mainSpecRoot, mainSpecFile);
assertDiscoveredSpecFile(mainSpecRoot, mainSpecFile);
try {
mainContent = await fs.readFile(mainSpecFile, 'utf-8');
} catch (error) {
+1 -1
View File
@@ -82,7 +82,7 @@ function findMarkerIndex(
export class FileSystemUtils {
/**
* Converts a path to use forward slashes (POSIX style).
* Essential for cross-platform compatibility with glob libraries like fast-glob.
* Essential for cross-platform compatibility with glob libraries like tinyglobby.
*/
static toPosixPath(p: string): string {
return p.replace(/\\/g, '/');
+19
View File
@@ -176,3 +176,22 @@ function readYesNo(
});
}
/**
* Whether a spinner can safely render to `stream`.
*
* ora decides how many lines to erase with `ceil(width / stream.columns)`, so a
* TTY that reports zero columns makes that count `Infinity` and `clear()` never
* ends: the spinner emits cursor-up/clear-line escapes as fast as it can and the
* command never finishes (#2106). Zero-width TTYs are real — a pty inherited
* from a pre-commit runner such as lefthook, or `script -qec` — and nothing can
* be drawn in zero columns anyway, so refuse the spinner and let the caller fall
* back to its plain output.
*/
export function canRenderSpinner(
stream: { isTTY?: boolean; columns?: number } = process.stderr
): boolean {
// A non-TTY stream never enters the clear loop: ora disables itself and
// prints one plain line instead.
if (!stream.isTTY) return true;
return (stream.columns ?? 0) > 0;
}
+7 -17
View File
@@ -1,6 +1,6 @@
import { promises as fs } from 'fs';
import path from 'path';
import { FileSystemUtils } from './file-system.js';
import { assertDiscoveredSpecFile } from './spec-path-containment.js';
import { parseDeltaSpec } from '../core/parsers/requirement-blocks.js';
export interface DiscoveredSpec {
@@ -10,26 +10,16 @@ export interface DiscoveredSpec {
specFile: string;
}
function assertDiscoveredSpecPath(specsRoot: string, capabilityDir: string, specFile: string): void {
try {
FileSystemUtils.assertPathWithin(specsRoot, specFile);
} catch {
// Direct capability directories may intentionally be external monorepo
// links. In that case, confine the file to the capability itself.
FileSystemUtils.assertPathWithin(capabilityDir, specFile);
}
}
/**
* Recursively discover every `spec.md` under a specs root, so both the flat
* `specs/<id>/spec.md` layout and nested `specs/<area>/<id>/spec.md` layouts
* are found (#1353). A `spec.md` sitting directly in the root is ignored,
* matching the historical requirement that specs live in a capability folder.
* Dot-directories are skipped and symlinked directories are not followed.
* An in-capability symlinked `spec.md` IS resolved: `hasAnyFileUnder` and the
* artifact graph's globs both count it as content, so dropping it here would
* silently lose the delta on archive. A link outside its capability is
* rejected and a dangling link is skipped. Results are sorted by id.
* A symlinked `spec.md` IS resolved: `hasAnyFileUnder` and the artifact
* graph's globs both count it as content, so dropping it here would silently
* lose the delta on archive. `assertSpecFilePathAllowed` decides how far such
* a link may point; a dangling link is skipped. Results are sorted by id.
*
* A missing root (ENOENT) yields an empty list, but any other read failure
* (EACCES, EIO, ...) is thrown rather than swallowed: since this feeds the
@@ -53,12 +43,12 @@ export async function discoverSpecFiles(specsRoot: string): Promise<DiscoveredSp
} else if (entry.name === 'spec.md' && segments.length > 0) {
const specFile = path.join(dir, entry.name);
if (entry.isFile()) {
assertDiscoveredSpecPath(specsRoot, dir, specFile);
assertDiscoveredSpecFile(specsRoot, specFile);
results.push({ id: segments.join('/'), specFile });
} else if (entry.isSymbolicLink()) {
try {
if ((await fs.stat(specFile)).isFile()) {
assertDiscoveredSpecPath(specsRoot, dir, specFile);
assertDiscoveredSpecFile(specsRoot, specFile);
results.push({ id: segments.join('/'), specFile });
}
} catch (err: any) {
+132
View File
@@ -0,0 +1,132 @@
import * as nodeFs from 'fs';
import path from 'path';
import { OPENSPEC_DIR_NAME } from '../core/config.js';
import { FileSystemUtils } from './file-system.js';
/**
* The project a specs root belongs to, or null when it cannot be named.
*
* Every specs root OpenSpec reads is addressed through the project's
* `openspec` directory - `<project>/openspec/specs` for main specs and
* `<project>/openspec/changes/<change>/specs` (or `.../changes/archive/...`)
* for a change's deltas - so the project is the parent of that `openspec`
* directory.
*
* The directory below the match has to be `specs` or `changes`, which is what
* makes this a planning root rather than a directory that merely shares the
* name: a store kept at `~/openspec/my-store` would otherwise hand out `~` as
* its boundary. A specs root reached some other way (a store given by path)
* has no project to name, and containment stays at the specs root.
*/
function findProjectRoot(specsRoot: string): string | null {
let current = path.resolve(specsRoot);
let below: string | null = null;
while (true) {
const parent = path.dirname(current);
if (parent === current) {
return null;
}
if (
path.basename(current) === OPENSPEC_DIR_NAME &&
(below === 'specs' || below === 'changes')
) {
return parent;
}
below = path.basename(current);
current = parent;
}
}
/** Path-segment containment on the paths as written, without resolving links. */
function isLexicallyWithin(allowedDirectory: string, targetPath: string): boolean {
const relative = path.relative(path.resolve(allowedDirectory), path.resolve(targetPath));
return (
relative === '' ||
(relative !== '..' &&
!relative.startsWith(`..${path.sep}`) &&
!path.isAbsolute(relative))
);
}
/**
* A project may keep a capability's spec file elsewhere - beside the product
* documentation it belongs to - and leave a relative symlink at
* `openspec/specs/<capability>/spec.md`, so OpenSpec and the docs read and
* write the same bytes. Accept such a link, and refuse one that leaves the
* project, so a link swapped underneath a repo cannot turn a spec read or an
* archive write into an arbitrary file access.
*
* `outsideError` is rethrown when the file is not a link at all, so a path
* that simply escapes its root keeps reporting that.
*/
function assertLinkedSpecFileInProject(
root: string,
specFile: string,
outsideError: unknown
): void {
const projectRoot = findProjectRoot(root);
if (projectRoot === null) {
throw outsideError;
}
const resolvedFile = path.resolve(specFile);
let isLink = false;
try {
isLink = nodeFs.lstatSync(resolvedFile).isSymbolicLink();
} catch {
isLink = false;
}
if (!isLink) {
throw outsideError;
}
// Only where the link points is the project's choice. The directory holding
// it must still be reached from inside the root.
FileSystemUtils.assertPathWithin(root, path.dirname(resolvedFile));
// Canonicalizes through the link, so a target outside the project is refused
// and a dangling link - which cannot be proven contained - fails loudly.
FileSystemUtils.assertPathWithin(projectRoot, resolvedFile);
}
/**
* Containment for a `spec.md` under a trust root already established for it:
* inside the root, or a link out of the root that stays inside the project.
*
* Deliberately does not re-derive a trust root from the file's own capability
* directory. Callers use this to recheck a path immediately before reading or
* writing it, and a capability directory swapped for a link since the path was
* resolved has to be refused rather than trusted afresh.
*/
export function assertSpecFileWithin(root: string, specFile: string): void {
try {
FileSystemUtils.assertPathWithin(root, specFile);
} catch (error) {
assertLinkedSpecFileInProject(root, specFile, error);
}
}
/**
* Containment for a `spec.md` as it is discovered, where the capability
* directory itself may deliberately be a link into another repository and is
* then the file's own trust root.
*/
export function assertDiscoveredSpecFile(specsRoot: string, specFile: string): void {
const capabilityDir = path.dirname(path.resolve(specFile));
try {
FileSystemUtils.assertPathWithin(specsRoot, specFile);
return;
} catch (error) {
// The capability directory is the file's own trust root only while it is
// addressed from inside the specs root: the link it may be is deliberate,
// a `..` out of the root is not.
if (isLexicallyWithin(specsRoot, capabilityDir)) {
try {
FileSystemUtils.assertPathWithin(capabilityDir, specFile);
return;
} catch {
// Not a file of its own capability either; the link rule decides.
}
}
assertLinkedSpecFileInProject(specsRoot, specFile, error);
}
}
@@ -0,0 +1,157 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { promises as fs } from 'fs';
import path from 'path';
import { tmpdir } from 'os';
import { runCLI } from '../helpers/run-cli.js';
/**
* The layout reported in #2100: a project keeps its main specs beside the
* product documentation they belong to and links
* `openspec/specs/<capability>/spec.md` to them, so OpenSpec and the docs read
* and write the same bytes.
*
* Driven through the real CLI on purpose. The regression this guards against
* was introduced by a containment check several layers down, and the unit
* tests of the day could not see it because their fixtures placed "outside the
* specs root" and "outside the project" at the same path. Exercising the
* commands a user actually runs does not care which layer the boundary moves
* in: it fails if any of them stops accepting the link, or starts accepting a
* link that leaves the project.
*/
const describeWithSymlinks = describe.skipIf(process.platform === 'win32');
describeWithSymlinks('a main spec that is a symlink into the project (#2100)', () => {
let base: string;
let projectDir: string;
let docsSpec: string;
let linkedSpec: string;
const MAIN_SPEC = [
'# login Specification',
'',
'## Purpose',
'Users sign in with their credentials to reach their account.',
'',
'## Requirements',
'',
'### Requirement: Sign in',
'The system SHALL let a user sign in with valid credentials.',
'',
'#### Scenario: Valid credentials',
'- **WHEN** the user submits valid credentials',
'- **THEN** the user is signed in',
'',
].join('\n');
const DELTA = [
'## ADDED Requirements',
'',
'### Requirement: Second factor',
'The system SHALL require a second factor.',
'',
'#### Scenario: Prompted',
'- **WHEN** a user signs in',
'- **THEN** a second factor is requested',
'',
].join('\n');
const write = async (relative: string, content: string) => {
const file = path.join(projectDir, relative);
await fs.mkdir(path.dirname(file), { recursive: true });
await fs.writeFile(file, content);
};
beforeEach(async () => {
// The project sits one level below the temp root, so a link written to the
// temp root genuinely leaves the project.
base = await fs.mkdtemp(path.join(tmpdir(), 'openspec-spec-symlink-e2e-'));
projectDir = path.join(base, 'repo');
docsSpec = path.join(projectDir, 'docs', 'auth', 'specs', 'login.md');
linkedSpec = path.join(projectDir, 'openspec', 'specs', 'login', 'spec.md');
await write(path.join('docs', 'auth', 'specs', 'login.md'), MAIN_SPEC);
await write(path.join('openspec', 'changes', 'add-mfa', 'proposal.md'),
'# Add MFA\n\n## Why\nAccounts need a second factor.\n\n## What Changes\n- Require a second factor\n');
await write(path.join('openspec', 'changes', 'add-mfa', 'tasks.md'), '## 1. Work\n- [x] 1.1 Ship it\n');
await write(path.join('openspec', 'changes', 'add-mfa', 'specs', 'login', 'spec.md'), DELTA);
await fs.mkdir(path.dirname(linkedSpec), { recursive: true });
});
afterEach(async () => {
await fs.rm(base, { recursive: true, force: true });
});
const linkInto = (target: string) =>
fs.symlink(path.relative(path.dirname(linkedSpec), target), linkedSpec);
describe('when the link stays inside the project', () => {
beforeEach(async () => {
await linkInto(docsSpec);
});
it('lists the capability', async () => {
const result = await runCLI(['list', '--specs'], { cwd: projectDir });
expect(result.exitCode).toBe(0);
expect(result.stdout).toContain('login');
});
it('shows the linked file as the spec', async () => {
const result = await runCLI(['show', 'login', '--type', 'spec'], { cwd: projectDir });
expect(result.exitCode).toBe(0);
expect(result.stdout).toContain('Users sign in with their credentials');
});
it('validates the capability, by id and as part of --specs', async () => {
const byId = await runCLI(['validate', 'login', '--type', 'spec'], { cwd: projectDir });
expect(byId.exitCode).toBe(0);
const all = await runCLI(['validate', '--specs'], { cwd: projectDir });
expect(all.exitCode).toBe(0);
expect(`${all.stdout}${all.stderr}`).not.toContain('outside the allowed directory');
});
it('archives through the link, leaving the link and the docs file in place', async () => {
const result = await runCLI(['archive', 'add-mfa', '--yes'], { cwd: projectDir });
expect(result.exitCode).toBe(0);
// The merge landed in the file the docs own, and kept what was there.
const merged = await fs.readFile(docsSpec, 'utf-8');
expect(merged).toContain('Second factor');
expect(merged).toContain('Sign in');
// ...by writing through the link rather than replacing it.
expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true);
});
});
describe('when the link leaves the project', () => {
let outside: string;
beforeEach(async () => {
outside = path.join(base, 'outside.md');
await fs.writeFile(outside, MAIN_SPEC);
await linkInto(outside);
});
it.each([
['list', ['list', '--specs']],
['show', ['show', 'login', '--type', 'spec']],
['validate by id', ['validate', 'login', '--type', 'spec']],
['validate --specs', ['validate', '--specs']],
])('refuses it in %s', async (_label, args) => {
const result = await runCLI(args, { cwd: projectDir });
expect(result.exitCode).not.toBe(0);
expect(`${result.stdout}${result.stderr}`).toContain('outside the allowed directory');
});
it('refuses to archive, and writes nothing', async () => {
const result = await runCLI(['archive', 'add-mfa', '--yes'], { cwd: projectDir });
expect(result.exitCode).not.toBe(0);
expect(`${result.stdout}${result.stderr}`).toContain('outside the allowed directory');
await expect(fs.readFile(outside, 'utf-8')).resolves.toBe(MAIN_SPEC);
// The change is still there to retry once the link is corrected.
await expect(
fs.access(path.join(projectDir, 'openspec', 'changes', 'add-mfa'))
).resolves.toBeUndefined();
});
});
});
+4 -2
View File
@@ -134,17 +134,19 @@ describe('CLI startup loads only what the command needs', () => {
}
);
it('`openspec __complete schemas` loads the schema reader but not fast-glob or ora', async () => {
it('`openspec __complete schemas` loads the schema reader but not tinyglobby or ora', async () => {
await ensureCliBuilt();
const { own, packages, status } = loadedModules(['__complete', 'schemas']);
expect(status).toBe(0);
expect([...packages]).not.toContain('fast-glob');
expect([...packages]).not.toContain('tinyglobby');
expect([...packages]).not.toContain('brace-expansion');
expect([...packages]).not.toContain('ora');
expect(own).not.toContain('telemetry/index.js');
// The name promises a schema reader, so pin it: the resolver directly, not
// the artifact-graph barrel, which re-exports it but also pulls outputs.js
// and with it fast-glob.
// and with it tinyglobby.
expect(own).toContain('core/artifact-graph/resolver.js');
expect(own).not.toContain('core/artifact-graph/index.js');
});
+36 -11
View File
@@ -6,7 +6,7 @@ import { MarkdownParser } from '../../src/core/parsers/markdown-parser.js';
import { findMainSpecStructureIssues } from '../../src/core/parsers/spec-structure.js';
import { VALIDATION_MESSAGES } from '../../src/core/validation/constants.js';
import { formatLocalDate } from '../../src/utils/date.js';
import { promises as fs, realpathSync } from 'fs';
import { constants as fsConstants, promises as fs, realpathSync } from 'fs';
import path from 'path';
import os from 'os';
@@ -7863,12 +7863,15 @@ The system SHALL provide a new behavior.
});
await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow(
/Path is outside the allowed directory/
/changed while archive was securing it for retirement/
);
expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true);
expect(await fs.readlink(linkedSpec)).toBe(shared);
await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(mainSpec('a-layer'));
// The link is back, and the update that landed on its target while the
// retirement was in flight is still there - restoring the link must not
// write through it.
await expect(fs.readFile(shared, 'utf-8')).resolves.toBe('concurrent update\n');
await expect(fs.access(changeDir)).resolves.not.toThrow();
}
);
@@ -7912,11 +7915,31 @@ The system SHALL provide a new behavior.
});
await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow(
/Path is outside the allowed directory/
/A concurrent file appeared at/
);
expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true);
expect(await fs.readlink(linkedSpec)).toBe(shared);
// The occupant that appeared at the link's path is left exactly as it
// was found; the displaced link is retained beside it instead.
// O_NOFOLLOW makes the open itself fail if the path is a link again, so
// one handle both proves the occupant is a regular file and carries its
// bytes - no second lookup that could land on a different file.
const occupant = await fs.open(
linkedSpec,
fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW
);
try {
expect((await occupant.stat()).isFile()).toBe(true);
await expect(occupant.readFile('utf-8')).resolves.toBe('concurrent occupant\n');
} finally {
await occupant.close();
}
const retained = (await fs.readdir(path.dirname(linkedSpec))).find((name) =>
name.includes('.openspec-retire-')
);
expect(retained).toBeDefined();
expect(
(await fs.lstat(path.join(path.dirname(linkedSpec), retained!))).isSymbolicLink()
).toBe(true);
await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(mainSpec('a-layer'));
await expect(fs.access(changeDir)).resolves.not.toThrow();
}
@@ -7973,7 +7996,7 @@ The system SHALL provide a new behavior.
});
await expect(archiveCommand.execute(changeName, { yes: true })).rejects.toThrow(
/Path is outside the allowed directory/
/permission denied/
);
await expect(fs.readFile(shared, 'utf-8')).resolves.toBe(original);
@@ -8372,10 +8395,12 @@ The system SHALL provide a new behavior.
await archiveCommand.execute(changeName, { yes: true, json: true });
const payload = JSON.parse(lastJsonPayload());
expect(payload.archive).toBeNull();
expect(payload.status[0].message).toContain('Path is outside the allowed directory');
expect((await fs.lstat(path.join(mainSpecDir, 'spec.md'))).isSymbolicLink()).toBe(true);
// The shared file really is still there.
expect(payload.archive.specsUpdated).toBe(true);
const notes = payload.archive.warnings.join('\n');
expect(notes).toContain('openspec/specs/legacy-layer/spec.md');
expect(notes).not.toContain('shared-legacy.md');
// The link is gone, and the shared file it pointed at really is still there.
await expect(fs.lstat(path.join(mainSpecDir, 'spec.md'))).rejects.toThrow();
await expect(fs.readFile(shared, 'utf-8')).resolves.toContain('### Requirement:');
}
);
+120 -5
View File
@@ -1,5 +1,13 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import fg from 'fast-glob';
import * as globby from 'tinyglobby';
vi.mock('tinyglobby', async () => {
const actual = await vi.importActual<typeof import('tinyglobby')>('tinyglobby');
return {
...actual,
globSync: vi.fn((...args: Parameters<typeof actual.globSync>) => actual.globSync(...args)),
};
});
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
@@ -18,6 +26,7 @@ describe('artifact-graph/outputs', () => {
beforeEach(() => {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-outputs-test-'));
vi.mocked(globby.globSync).mockClear();
});
afterEach(() => {
@@ -138,7 +147,7 @@ describe('artifact-graph/outputs', () => {
const outsideDir = fs.mkdtempSync(path.join(path.dirname(tempDir), 'openspec-outside-'));
const pattern = `{safe,../${path.basename(outsideDir)}}/review.md`;
if (exists) fs.writeFileSync(path.join(outsideDir, 'review.md'), 'private');
const glob = vi.spyOn(fg, 'sync');
const glob = vi.mocked(globby.globSync);
try {
expect(() => resolveArtifactOutputs(tempDir, pattern)).toThrow(
/outside the allowed directory/u
@@ -160,6 +169,39 @@ describe('artifact-graph/outputs', () => {
expect(resolveArtifactOutputs(tempDir, pattern)).toEqual([canonical(filePath)]);
});
it.each([
['{prefix{a,b}}.md', ['{prefixa}.md', '{prefixb}.md']],
['{prefix{1..2}}.md', ['{prefix1}.md', '{prefix2}.md']],
['{pre{a,b}post}.md', ['{preapost}.md', '{prebpost}.md']],
['{{a,b}}.md', ['{a}.md', '{b}.md']],
['{draft{nope}{a,b}}.md', ['{draft{nope}a}.md', '{draft{nope}b}.md']],
])('expands a nested group inside non-expanding braces in %s', (pattern, filenames) => {
for (const filename of filenames) {
fs.writeFileSync(path.join(tempDir, filename), 'content');
}
fs.writeFileSync(path.join(tempDir, pattern), 'unexpanded');
expect(resolveArtifactOutputs(tempDir, pattern)).toEqual(
filenames.map((filename) => canonical(path.join(tempDir, filename)))
);
});
it('keeps a brace group literal when neither it nor its nested groups expand', () => {
const literalPath = path.join(tempDir, 'notes-{prefix{draft}}-a.md');
fs.writeFileSync(literalPath, 'content');
fs.writeFileSync(path.join(tempDir, 'notes-{prefix{draft}}-b.md'), 'content');
fs.writeFileSync(path.join(tempDir, 'notes-{prefixdraft}-a.md'), 'stripped');
fs.writeFileSync(path.join(tempDir, '{prefix{draft}}.md'), 'content');
expect(resolveArtifactOutputs(tempDir, 'notes-{prefix{draft}}-{a,b}.md')).toEqual([
canonical(literalPath),
canonical(path.join(tempDir, 'notes-{prefix{draft}}-b.md')),
]);
expect(resolveArtifactOutputs(tempDir, '{prefix{draft}}.md')).toEqual([
canonical(path.join(tempDir, '{prefix{draft}}.md')),
]);
});
it('resolves a brace range after a literal brace group', () => {
const filenames = [1, 2, 3, 4].map((index) => `report-{draft}-${index}.md`);
for (const filename of filenames) {
@@ -272,11 +314,11 @@ describe('artifact-graph/outputs', () => {
expect(artifactOutputExists(tempDir, 'file-{1..3}.md')).toBe(true);
});
it('rejects brace nesting deeper than any artifact pattern needs', () => {
it('rejects brace nesting deeper than any artifact pattern needs before it can overflow', () => {
const fileA = path.join(tempDir, 'a.md');
fs.writeFileSync(fileA, 'content');
const nested = (depth: number) => `${'{a,'.repeat(depth)}b${'}'.repeat(depth)}.md`;
const globSpy = vi.spyOn(fg, 'generateTasks');
const globSpy = vi.mocked(globby.globSync);
expect(resolveArtifactOutputs(tempDir, nested(16))).toEqual([canonical(fileA)]);
globSpy.mockClear();
@@ -293,17 +335,90 @@ describe('artifact-graph/outputs', () => {
`16 levels deep: ${nested(4000).slice(0, 80)}...`
);
expect(() => resolveArtifactOutputs(tempDir, nested(4000))).not.toThrow(nested(4000));
expect(() => resolveArtifactOutputs(tempDir, nested(4000))).not.toThrow(
/Maximum call stack size exceeded/u
);
expect(globSpy).not.toHaveBeenCalled();
globSpy.mockRestore();
});
it('rejects a brace pattern that is wide rather than deep before it exhausts the heap', () => {
// Nesting depth is 1 here, so the depth guard never fires. `{a,b}` repeated
// 20 times is 1,048,576 patterns and crashed the expander with
// "JavaScript heap out of memory"; fast-glob compiled it to a regex instead.
const wide = (groups: number) => `${'{a,b}'.repeat(groups)}.md`;
const globSpy = vi.mocked(globby.globSync);
expect(() => resolveArtifactOutputs(tempDir, wide(20))).toThrow(
'Artifact output pattern expands to more than 1024 patterns'
);
// The error quotes only the start of a pattern that long.
expect(() => resolveArtifactOutputs(tempDir, wide(20))).toThrow(
`1024 patterns: ${wide(20).slice(0, 80)}...`
);
// A range is bounded by the same budget. brace-expansion caps itself at
// 100,000 results, so the budget is passed down as its `max` and the
// allocation stops just past 1024 instead.
expect(() => resolveArtifactOutputs(tempDir, 'file-{1..200000}.md')).toThrow(
'Artifact output pattern expands to more than 1024 patterns'
);
// Far past brace-expansion's own cap, so only our `max` can stop it early.
expect(() => resolveArtifactOutputs(tempDir, 'file-{1..1000000000}.md')).toThrow(
'Artifact output pattern expands to more than 1024 patterns'
);
expect(globSpy).not.toHaveBeenCalled();
globSpy.mockRestore();
});
it('still resolves the widest pattern inside the expansion budget', () => {
// 2^10 = 1024 alternatives: at the budget, not over it.
const file = path.join(tempDir, `${'a'.repeat(10)}.md`);
fs.writeFileSync(file, 'content');
expect(resolveArtifactOutputs(tempDir, `${'{a,b}'.repeat(10)}.md`)).toEqual([canonical(file)]);
});
it('rejects an absolute pattern before globbing, however little it matches', () => {
// fast-glob's generateTasks reported `/etc` as the task base, so the
// containment check rejected the pattern. A base that silently drops the
// leading slash resolves inside the change directory and lets the glob read
// a directory the change does not own.
const globSpy = vi.mocked(globby.globSync);
expect(() => resolveArtifactOutputs(tempDir, '/etc/openspec-nope-*.md')).toThrow(
/outside the allowed directory/u
);
expect(() => resolveArtifactOutputs(tempDir, '/openspec-nope-*.md')).toThrow(
/outside the allowed directory/u
);
expect(globSpy).not.toHaveBeenCalled();
globSpy.mockRestore();
});
it('keeps a control character in a filename literal instead of reading it as a brace', () => {
// The expander parks a non-expanding brace group on a sentinel. A
// single-character sentinel is a legal filename character on POSIX, so a
// pattern containing one came back out as `{` or `}` and matched the wrong
// file. The decoy is what the bug returned, and it must never match on any
// platform; Windows forbids U+0001 in a filename, so there the real file
// cannot exist and the pattern has to come back empty.
fs.writeFileSync(path.join(tempDir, 'x}y.md'), 'wrong');
const literal = path.join(tempDir, 'x\u0001y.md');
const literalIsCreatable = process.platform !== 'win32';
if (literalIsCreatable) fs.writeFileSync(literal, 'content');
expect(resolveArtifactOutputs(tempDir, 'x\u0001y*.md')).toEqual(
literalIsCreatable ? [canonical(literal)] : []
);
});
it.each([
['a bracket', (depth: number) => `*${'{a,[}]'.repeat(depth)}${'}'.repeat(depth)}`],
['a quote', (depth: number) => `*${'{a,"}"'.repeat(depth)}${'}'.repeat(depth)}`],
['an open parenthesis', (depth: number) => `*${'{a,(}'.repeat(depth)}${')}'.repeat(depth)}`],
['parentheses alone', (depth: number) => `*{a,b}${'('.repeat(depth)}${')'.repeat(depth)}`],
])('counts nesting the braces parser sees past %s', (_label, nested) => {
const globSpy = vi.spyOn(fg, 'generateTasks');
const globSpy = vi.mocked(globby.globSync);
expect(() => resolveArtifactOutputs(tempDir, nested(17))).toThrow(
'Artifact output pattern nests braces more than 16 levels deep'
@@ -6,13 +6,17 @@ import { SpecCommand } from '../../../src/commands/spec.js';
describe('SpecCommand path boundaries', () => {
let tempDir: string;
let projectDir: string;
let originalCwd: string;
beforeEach(async () => {
originalCwd = process.cwd();
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-spec-command-security-'));
await fs.mkdir(path.join(tempDir, 'openspec', 'specs'), { recursive: true });
process.chdir(tempDir);
// A spec file may link to another file in the same project, so the escape
// these cases guard against is one that leaves the project entirely.
projectDir = path.join(tempDir, 'repo');
await fs.mkdir(path.join(projectDir, 'openspec', 'specs'), { recursive: true });
process.chdir(projectDir);
});
afterEach(async () => {
@@ -31,10 +35,10 @@ describe('SpecCommand path boundaries', () => {
});
it.skipIf(process.platform === 'win32')(
'rejects a spec file symlink that leaves the specs root',
'rejects a spec file symlink that leaves the project',
async () => {
const outsideSpec = path.join(tempDir, 'outside.md');
const linkedSpec = path.join(tempDir, 'openspec', 'specs', 'linked', 'spec.md');
const linkedSpec = path.join(projectDir, 'openspec', 'specs', 'linked', 'spec.md');
await fs.writeFile(outsideSpec, '# Outside sentinel');
await fs.mkdir(path.dirname(linkedSpec), { recursive: true });
await fs.symlink(outsideSpec, linkedSpec);
@@ -45,6 +49,28 @@ describe('SpecCommand path boundaries', () => {
}
);
it.skipIf(process.platform === 'win32')(
'shows a spec.md linked to a file elsewhere in the project',
async () => {
// The reported layout (#2100): the spec lives beside the product docs it
// belongs to and openspec/specs/<capability>/spec.md links to it.
const docsSpec = path.join(projectDir, 'docs', 'auth', 'specs', 'login.md');
await fs.mkdir(path.dirname(docsSpec), { recursive: true });
await fs.writeFile(
docsSpec,
'# login\n\n## Purpose\n\nUsers sign in.\n\n## Requirements\n'
);
const linkedSpec = path.join(projectDir, 'openspec', 'specs', 'login', 'spec.md');
await fs.mkdir(path.dirname(linkedSpec), { recursive: true });
await fs.symlink(
path.relative(path.dirname(linkedSpec), docsSpec),
linkedSpec
);
await expect(new SpecCommand().show('login')).resolves.toBeUndefined();
}
);
it.skipIf(process.platform === 'win32')(
'allows a linked capability directory as its own trust root',
async () => {
@@ -56,7 +82,7 @@ describe('SpecCommand path boundaries', () => {
);
await fs.symlink(
sharedCapability,
path.join(tempDir, 'openspec', 'specs', 'shared')
path.join(projectDir, 'openspec', 'specs', 'shared')
);
await expect(new SpecCommand().show('shared')).resolves.toBeUndefined();
@@ -66,7 +92,7 @@ describe('SpecCommand path boundaries', () => {
it.skipIf(process.platform === 'win32')(
'allows a spec file symlink elsewhere in the specs root',
async () => {
const specsDir = path.join(tempDir, 'openspec', 'specs');
const specsDir = path.join(projectDir, 'openspec', 'specs');
const sharedSpec = path.join(specsDir, 'shared.md');
const linkedSpec = path.join(specsDir, 'linked', 'spec.md');
await fs.writeFile(
@@ -0,0 +1,232 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { promises as fs } from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {
buildUpdatedSpec,
findSpecUpdates,
writeUpdatedSpec,
} from '../../src/core/specs-apply.js';
const itWithSymlinks = it.skipIf(process.platform === 'win32');
/**
* A project may keep its main specs beside the product documentation they
* belong to and link `openspec/specs/<capability>/spec.md` to them, so the
* docs and OpenSpec read and write the same bytes (#2100). The merge has to
* write through such a link, and still refuse one that leaves the project.
*/
describe('a spec.md linked to a file elsewhere in the project', () => {
let tempDir: string;
let projectDir: string;
let changeDir: string;
let mainSpecsDir: string;
let docsSpec: string;
let linkedSpec: string;
const DELTA = [
'## ADDED Requirements',
'',
'### Requirement: Second factor',
'The system SHALL require a second factor.',
'',
'#### Scenario: Prompted',
'- **WHEN** a user signs in',
'- **THEN** a second factor is requested',
'',
].join('\n');
const MAIN_SPEC = [
'# login Specification',
'',
'## Purpose',
'Users sign in with their credentials to reach their account.',
'',
'## Requirements',
'',
'### Requirement: Sign in',
'The system SHALL let a user sign in with valid credentials.',
'',
'#### Scenario: Valid credentials',
'- **WHEN** the user submits valid credentials',
'- **THEN** the user is signed in',
'',
].join('\n');
beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-project-linked-spec-'));
projectDir = path.join(tempDir, 'repo');
changeDir = path.join(projectDir, 'openspec', 'changes', 'add-mfa');
mainSpecsDir = path.join(projectDir, 'openspec', 'specs');
docsSpec = path.join(projectDir, 'docs', 'auth', 'specs', 'login.md');
linkedSpec = path.join(mainSpecsDir, 'login', 'spec.md');
await fs.mkdir(path.join(changeDir, 'specs', 'login'), { recursive: true });
await fs.writeFile(path.join(changeDir, 'specs', 'login', 'spec.md'), DELTA);
await fs.mkdir(path.dirname(docsSpec), { recursive: true });
await fs.writeFile(docsSpec, MAIN_SPEC);
await fs.mkdir(path.dirname(linkedSpec), { recursive: true });
});
afterEach(async () => {
await fs.rm(tempDir, { recursive: true, force: true });
});
itWithSymlinks('merges through the link and leaves the link in place', async () => {
await fs.symlink(path.relative(path.dirname(linkedSpec), docsSpec), linkedSpec);
const warnings: string[] = [];
const onWarning = (warning: Error) => warnings.push(warning.message);
process.on('warning', onWarning);
let updates;
try {
updates = await findSpecUpdates(changeDir, mainSpecsDir);
// process.emitWarning dispatches on the next tick.
await new Promise((resolve) => setImmediate(resolve));
} finally {
process.off('warning', onWarning);
}
expect(updates).toHaveLength(1);
const [update] = updates;
// The link path is what the CLI reports, and the write goes through it.
// Rebased onto the canonical specs root, so /var and /private/var agree.
const canonicalLinkedSpec = path.join(
await fs.realpath(path.dirname(linkedSpec)),
'spec.md'
);
expect(update.target).toBe(canonicalLinkedSpec);
expect(update.exists).toBe(true);
// Nothing leaves the project, so there is nothing to announce.
expect(warnings).toEqual([]);
const built = await buildUpdatedSpec(update, 'add-mfa', { silent: true });
await writeUpdatedSpec(update, built.rebuilt, built.counts, { silent: true });
// The file the docs own received the merge, and kept what it already had.
const merged = await fs.readFile(docsSpec, 'utf-8');
expect(merged).toContain('Second factor');
expect(merged).toContain('Sign in');
expect((await fs.lstat(linkedSpec)).isSymbolicLink()).toBe(true);
});
itWithSymlinks('refuses a link that leaves the project', async () => {
const outsideSpec = path.join(tempDir, 'outside.md');
await fs.writeFile(outsideSpec, MAIN_SPEC);
await fs.symlink(outsideSpec, linkedSpec);
await expect(findSpecUpdates(changeDir, mainSpecsDir)).rejects.toThrow(
'Path is outside the allowed directory'
);
await expect(fs.readFile(outsideSpec, 'utf-8')).resolves.toBe(MAIN_SPEC);
});
itWithSymlinks(
'refuses a link repointed out of the project between resolve and write',
async () => {
await fs.symlink(path.relative(path.dirname(linkedSpec), docsSpec), linkedSpec);
const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
const built = await buildUpdatedSpec(update, 'add-mfa', { silent: true });
// The link now points out of the project, after the path was resolved and
// the merge was built. The recheck before the write has to catch it.
const outsideSpec = path.join(tempDir, 'outside.md');
await fs.writeFile(outsideSpec, 'outside sentinel');
await fs.rm(linkedSpec);
await fs.symlink(outsideSpec, linkedSpec);
await expect(
writeUpdatedSpec(update, built.rebuilt, built.counts, { silent: true })
).rejects.toThrow('Path is outside the allowed directory');
await expect(fs.readFile(outsideSpec, 'utf-8')).resolves.toBe('outside sentinel');
}
);
itWithSymlinks(
'refuses a link repointed out of the project between resolve and read',
async () => {
await fs.symlink(path.relative(path.dirname(linkedSpec), docsSpec), linkedSpec);
const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
const outsideSpec = path.join(tempDir, 'outside.md');
await fs.writeFile(outsideSpec, 'outside sentinel');
await fs.rm(linkedSpec);
await fs.symlink(outsideSpec, linkedSpec);
await expect(buildUpdatedSpec(update, 'add-mfa', { silent: true })).rejects.toThrow(
'Path is outside the allowed directory'
);
}
);
itWithSymlinks(
'cannot be redirected by a link repointed while the write is in flight',
async () => {
await fs.symlink(path.relative(path.dirname(linkedSpec), docsSpec), linkedSpec);
const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
const built = await buildUpdatedSpec(update, 'add-mfa', { silent: true });
const outsideSpec = path.join(tempDir, 'outside.md');
await fs.writeFile(outsideSpec, 'outside sentinel');
// `beforeMutate` runs after the containment check, so it is the one
// window a swap could still use.
await expect(
writeUpdatedSpec(update, built.rebuilt, built.counts, {
silent: true,
beforeMutate: async () => {
await fs.rm(linkedSpec);
await fs.symlink(outsideSpec, linkedSpec);
},
})
).rejects.toThrow();
// Nothing was written: not to where the link now points, and not to the
// file that was verified either, since the target is no longer it.
await expect(fs.readFile(outsideSpec, 'utf-8')).resolves.toBe('outside sentinel');
await expect(fs.readFile(docsSpec, 'utf-8')).resolves.not.toContain('Second factor');
}
);
itWithSymlinks(
'refuses an ordinary target replaced by a link while the write is in flight',
async () => {
// Not specific to the linked layout: the canonical file at the end of the
// chain can be swapped for a link too, so the re-check has to catch that
// for a plain `openspec/specs/<capability>/spec.md` as well.
await fs.writeFile(linkedSpec, MAIN_SPEC);
const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
const built = await buildUpdatedSpec(update, 'add-mfa', { silent: true });
const outsideSpec = path.join(tempDir, 'outside.md');
await fs.writeFile(outsideSpec, 'outside sentinel');
await expect(
writeUpdatedSpec(update, built.rebuilt, built.counts, {
silent: true,
beforeMutate: async () => {
await fs.rm(linkedSpec);
await fs.symlink(outsideSpec, linkedSpec);
},
})
).rejects.toThrow();
await expect(fs.readFile(outsideSpec, 'utf-8')).resolves.toBe('outside sentinel');
}
);
itWithSymlinks('merges through a linked delta spec inside the project', async () => {
await fs.symlink(path.relative(path.dirname(linkedSpec), docsSpec), linkedSpec);
const sharedDelta = path.join(projectDir, 'docs', 'auth', 'deltas', 'login.md');
await fs.mkdir(path.dirname(sharedDelta), { recursive: true });
await fs.writeFile(sharedDelta, DELTA);
const deltaPath = path.join(changeDir, 'specs', 'login', 'spec.md');
await fs.rm(deltaPath);
await fs.symlink(path.relative(path.dirname(deltaPath), sharedDelta), deltaPath);
const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
const built = await buildUpdatedSpec(update, 'add-mfa', { silent: true });
expect(built.rebuilt).toContain('Second factor');
});
});
+7 -3
View File
@@ -12,6 +12,7 @@ const itWithSymlinks = it.skipIf(process.platform === 'win32');
describe('spec apply path boundaries', () => {
let tempDir: string;
let projectDir: string;
let changeDir: string;
let changeSpecsDir: string;
let mainSpecsDir: string;
@@ -19,9 +20,12 @@ describe('spec apply path boundaries', () => {
beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-spec-apply-security-'));
changeDir = path.join(tempDir, 'openspec', 'changes', 'test-change');
// A spec file may link to another file in the same project, so the escape
// these cases guard against is one that leaves the project entirely.
projectDir = path.join(tempDir, 'repo');
changeDir = path.join(projectDir, 'openspec', 'changes', 'test-change');
changeSpecsDir = path.join(changeDir, 'specs');
mainSpecsDir = path.join(tempDir, 'openspec', 'specs');
mainSpecsDir = path.join(projectDir, 'openspec', 'specs');
outsideDir = path.join(tempDir, 'outside');
await fs.mkdir(changeSpecsDir, { recursive: true });
await fs.mkdir(mainSpecsDir, { recursive: true });
@@ -52,7 +56,7 @@ describe('spec apply path boundaries', () => {
return deltaPath;
}
itWithSymlinks('rejects a delta spec symlink that leaves the change specs root', async () => {
itWithSymlinks('rejects a delta spec symlink that leaves the project', async () => {
const outsideSpec = path.join(outsideDir, 'spec.md');
await fs.writeFile(outsideSpec, 'outside sentinel');
const linkedSpec = path.join(changeSpecsDir, 'widgets', 'spec.md');
+3 -2
View File
@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
import { promises as fs } from 'fs';
import path from 'path';
import { fileURLToPath } from 'url';
import fg from 'fast-glob';
import { globSync } from 'tinyglobby';
import { findMissingTaskCheckboxIssues } from '../../src/core/validation/task-checkboxes.js';
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
@@ -189,9 +189,10 @@ describe('findMissingTaskCheckboxIssues', () => {
it('accepts every packaged tasks template', async () => {
// An agent writing a task file follows these. If one ever loses its
// checkboxes, every change built from it starts life counting zero tasks.
const templates = await fg('schemas/*/templates/tasks.md', {
const templates = globSync('schemas/*/templates/tasks.md', {
cwd: repoRoot,
absolute: true,
expandDirectories: false,
});
expect(templates.length).toBeGreaterThan(0);
@@ -0,0 +1,213 @@
// Every workflow that turns a glob output into a concrete write path must keep
// that path inside `changeRoot` (#2099). update-change always did; propose,
// ff-change and continue-change chose a path with no containment rule, so a
// capability path such as `../../x`, or a symlinked directory under `specs/`,
// would be written outside the change.
import { describe, expect, it } from 'vitest';
import {
getContinueChangeSkillTemplate,
getExploreSkillTemplate,
getFfChangeSkillTemplate,
getOpsxContinueCommandTemplate,
getOpsxExploreCommandTemplate,
getOpsxFfCommandTemplate,
getOpsxProposeCommandTemplate,
getOpsxProposeSkillTemplate,
getOpsxUpdateCommandTemplate,
getUpdateChangeSkillTemplate,
} from '../../../src/core/templates/skill-templates.js';
import { resolveOptionalWorkflows } from '../../../src/core/templates/optional-workflow.js';
import { ALL_WORKFLOWS } from '../../../src/core/profiles.js';
const withAllWorkflows = (body: string) =>
resolveOptionalWorkflows(body, new Set<string>(ALL_WORKFLOWS));
/** Lines that tell the agent to pick a concrete path for a glob output. */
const PATH_CHOICE = /choose a concrete path/i;
/**
* The containment rule for a write that may land on an existing file. A
* destination that is itself a symlink to a file outside `changeRoot` passes a
* parent-only check, so the file is resolved too.
*/
const BOUND =
'remains inside `changeRoot` after resolving symlinks, both in its parent directories and in the file itself if it already exists';
/**
* update-change creates its file with an operation that fails if the target
* exists - a symlink included - so resolving the parents is enough there.
*/
const CREATE_ONLY_BOUND =
'remains inside `changeRoot` after resolving any symlinked parent directories';
const boundFor = (label: string) => (label.startsWith('update-change') ? CREATE_ONLY_BOUND : BOUND);
/** Every surface that writes glob artifacts, skill and slash-command alike. */
const SURFACES: Array<[label: string, body: string]> = [
['propose skill', withAllWorkflows(getOpsxProposeSkillTemplate().instructions)],
['propose command', withAllWorkflows(getOpsxProposeCommandTemplate().content)],
['ff-change skill', withAllWorkflows(getFfChangeSkillTemplate().instructions)],
['ff-change command', withAllWorkflows(getOpsxFfCommandTemplate().content)],
['continue-change skill', withAllWorkflows(getContinueChangeSkillTemplate().instructions)],
['continue-change command', withAllWorkflows(getOpsxContinueCommandTemplate().content)],
['update-change skill', withAllWorkflows(getUpdateChangeSkillTemplate().instructions)],
['update-change command', withAllWorkflows(getOpsxUpdateCommandTemplate().content)],
['explore skill', withAllWorkflows(getExploreSkillTemplate().instructions)],
['explore command', withAllWorkflows(getOpsxExploreCommandTemplate().content)],
];
/** Where a line tells the agent to write. The bound must come before it. */
const WRITE = /\b(?:write|Write)\b(?! anything)(?! an expected path)(?! only the missing)/;
/**
* The bound has to be checked before the write, or an escaping path is already
* written by the time it is verified. A line with no write (update-change
* writes in a later step) only needs the bound.
*/
function expectBoundBeforeWrite(label: string, line: string): void {
const boundAt = line.indexOf(boundFor(label));
expect(boundAt, label).toBeGreaterThanOrEqual(0);
const writeAt = line.search(WRITE);
if (writeAt !== -1) {
expect(writeAt, `${label}: writes before checking the bound`).toBeGreaterThan(boundAt);
expect(line, `${label}: no instruction to refuse an escaping path`).toContain(
'stop and tell the user'
);
}
}
/** Every place a body hands artifact creation to another skill or command. */
const DELEGATION = /delegates creation/g;
/**
* A delegated writer is a write too. It may run only after the path it writes
* to has been chosen and checked, and only if it can write exactly there.
* Returns each delegation as [offset, the sentence that states it].
*/
function delegations(body: string): Array<[number, string]> {
return [...body.matchAll(DELEGATION)].map((match) => {
const start = match.index ?? 0;
const end = body.slice(start).search(/[.\n]/);
return [start, body.slice(start, end === -1 ? undefined : start + end)];
});
}
describe('glob output paths stay inside changeRoot (#2099)', () => {
it.each(SURFACES)('%s bounds every concrete-path choice', (label, body) => {
const choices = body.split('\n').filter((line) => PATH_CHOICE.test(line));
expect(choices, `${label}: no concrete-path line found`).toHaveLength(1);
expectBoundBeforeWrite(label, choices[0]);
});
it.each(SURFACES)('%s chooses and checks the path before any delegated writer runs', (label, body) => {
const choiceAt = body.search(PATH_CHOICE);
const found = delegations(body);
expect(found.length, `${label}: no delegated-creation rule found`).toBeGreaterThan(0);
for (const [at, sentence] of found) {
// Bound to one path, and refuses rather than letting it write elsewhere.
expect(sentence, label).toMatch(/only if it can/);
expect(sentence, label).toMatch(/stop/);
// The creation step's delegation comes after the choice it depends on.
// The only one allowed earlier is continue-change's backfill, which
// names the path its own preceding bullet checked.
if (at < choiceAt) {
expect(sentence, `${label}: delegates before choosing the path`).toContain(
'its checked path'
);
}
}
expect(
found.some(([at]) => at > choiceAt),
`${label}: the creation step does not bound its delegated writer`
).toBe(true);
});
// A delegated writer has already created the artifact; writing the template
// over it afterwards would replace what it made.
it.each(SURFACES.filter(([label]) => !label.startsWith('update-change')))(
'%s writes the template only when creation was not delegated',
(label, body) => {
const templateWrite = /\b[Ww]rite (?:it|the file|the artifact) to that output path/u;
const lines = body.split('\n');
let writes = 0;
lines.forEach((line, index) => {
// Judged sentence by sentence: explore states both branches in one
// paragraph, and the delegation sentence names the same path for the
// delegated writer.
for (const sentence of line.split(/(?<=\.)\s+(?=[A-Z])/u)) {
if (!templateWrite.test(sentence) || sentence.includes('delegates creation')) continue;
writes++;
if (/otherwise/iu.test(sentence.slice(0, sentence.search(templateWrite)))) continue;
// Otherwise nested under a bullet that opens the Otherwise branch.
const indent = line.search(/\S/u);
const parent = lines
.slice(0, index)
.reverse()
.find((candidate) => candidate.trim() !== '' && candidate.search(/\S/u) < indent);
expect(parent, `${label}: template write outside the Otherwise branch`).toMatch(
/^\s*- Otherwise/u
);
}
});
expect(writes, `${label}: no template write found`).toBeGreaterThan(0);
}
);
it.each([
['continue-change skill', withAllWorkflows(getContinueChangeSkillTemplate().instructions)],
['continue-change command', withAllWorkflows(getOpsxContinueCommandTemplate().content)],
])('%s verifies the output after either creation branch', (label, body) => {
const lines = body.split('\n');
const delegation = lines.findIndex((line) => line.includes('delegates creation to a specific skill'));
const verify = lines.findIndex((line) =>
line.includes('Either way, verify the artifact file exists at that output path')
);
expect(verify, label).toBeGreaterThan(delegation);
// Same depth as the two branches, so it runs after whichever was taken.
expect(lines[verify].search(/\S/u), label).toBe(lines[delegation].search(/\S/u));
});
it.each(SURFACES)('%s never targets the glob itself', (label, body) => {
// A glob is a pattern, not a file: nothing may be written to, or verified
// at, `resolvedOutputPath` unless it is a concrete path.
expect(body, label).not.toContain('exists at `resolvedOutputPath`');
expect(body, label).not.toContain('write it to `resolvedOutputPath`');
});
it.each(SURFACES)('%s restates delegation only with the same bound', (label, body) => {
// The artifact-creation guidelines repeat the delegation rule where agents
// skim; a bare "invoke it instead" there would undo the ordering above.
for (const line of body
.split('\n')
.filter((candidate) => candidate.includes('directs you to use a specific skill or command'))) {
expect(line, label).toContain('only once you have chosen and checked the output path');
}
});
// continue-change alone backfills a half-written glob artifact: it compares
// expected paths against what exists instead of choosing a new one, so the
// bound has to sit on that bullet too.
it.each([
['continue-change skill', withAllWorkflows(getContinueChangeSkillTemplate().instructions)],
['continue-change command', withAllWorkflows(getOpsxContinueCommandTemplate().content)],
])('%s bounds the partly-written-artifact check', (label, body) => {
const bullets = body
.split('\n')
.filter((line) => line.includes('resolve each expected path against `changeRoot`'));
expect(bullets, `${label}: partial-artifact bullet missing`).toHaveLength(1);
expect(bullets[0], label).toContain(BOUND);
// The write is the next bullet, so the refusal has to sit there.
const lines = body.split('\n');
const next = lines[lines.indexOf(bullets[0]) + 1];
expect(next, label).toContain('Never write an expected path that fails that check');
expect(next.indexOf('Never write'), label).toBeLessThan(next.indexOf('write only the missing'));
// A delegated writer filling the gap is held to the same checked paths.
expect(next, label).toContain('invoke it only if it can write each file to its checked path');
});
});
+1 -1
View File
@@ -678,7 +678,7 @@ describe('artifact loop guards (propose and ff)', () => {
it('tells the agent how to resolve a glob output path', () => {
for (const [label, body] of loopBodies) {
expect(body, label).toContain(
'is a glob, follow `instruction` to choose the concrete file path'
'is a glob, follow `instruction` to choose a concrete path'
);
}
});
@@ -76,18 +76,18 @@ function specDrivenTitles(): Record<string, string> {
}
const EXPECTED_FUNCTION_HASHES: Record<string, string> = {
getExploreSkillTemplate: 'efeb143c50472bf0db0f031f7fc9e32f3c2662b569101f6473b2cbbb99425367',
getExploreSkillTemplate: '80246765b685ec1e5530e8348bde84ed2ba0a6c418c9fbcd7e9e79a638d289dd',
getNewChangeSkillTemplate: '0e5035b7b42198afc430206a1dbc9579096650ef0813d85e837d5a6cd0b98a85',
getContinueChangeSkillTemplate: '32ec5965a2e90c033c9285f57a73cd6f39a31b58029d7546dba3cb6ba4c091f7',
getContinueChangeSkillTemplate: '2da5a24eb949f0717ffdb23c28c021ab6f73dc4beeb92ab709b8dbdac9db7355',
getApplyChangeSkillTemplate: 'c4f02c29137e19b34bf1dc59146941b8c920214aa2cf3ce80ae7f1b982e976fc',
getFfChangeSkillTemplate: 'd091600476a815ba99f69b446bcd46af5bf73d1c2810215a0c6196937d019cf6',
getFfChangeSkillTemplate: '0eb8ddc6da710bc9ed6cd142a2eec1bec883f730125d76dc38e532bd5395bdf2',
getSyncSpecsSkillTemplate: 'bc80fe9b07eaa289e5eb8a3ce65eb7df722a16d864e37283c678220712e4f230',
getOnboardSkillTemplate: '84258a06c0ca88de708a23dd74e9a17efe11eff63a071b3864c781dcd5a0a4b7',
getOpsxExploreCommandTemplate: '477c1a97cf272778953d1ad032113e3f20afaefa332a5c0882e536d919c176e2',
getOpsxExploreCommandTemplate: 'c0af210bc070b7ac5168523b381cb9e213e4e102205bfc26bdae15677d954ae1',
getOpsxNewCommandTemplate: '6d504fef1e0d4ced7c423f4cc9d9d2cee11b1a6224edf685e06a3f0757e0ebff',
getOpsxContinueCommandTemplate: '659596cef4859162707d8acbd411afb863110f34a7b90ad037bae5eb84dd858d',
getOpsxContinueCommandTemplate: '9e36b758c2760fcab4dcef4677ee678eb29eac00f31de8cbc99f812a440afaaf',
getOpsxApplyCommandTemplate: '9b733946aa7216b45d084bb5affd7071086db6f91282c3666731a17543825ff1',
getOpsxFfCommandTemplate: '743a7304c7efc84aa87f556154c034e1e0e561c276c51870a30ada58f33eb9af',
getOpsxFfCommandTemplate: '625d6a4a52adeaa16dbceff378f1d7cb07556e79b19d9af63feca5125aa4da13',
getArchiveChangeSkillTemplate: '04a029782fc4137971fad6f54cfda3685d7e0b835b06565f9a509b4878093099',
getBulkArchiveChangeSkillTemplate: '44dbd3c7a347e5f8339b2141393f2ac36017527cce251483fe70f2059c1e286e',
getOpsxSyncCommandTemplate: '60550b7bb9829421656d6324a9e4c951bc912f48f88882d1a07ce7f78397a5e7',
@@ -96,25 +96,25 @@ const EXPECTED_FUNCTION_HASHES: Record<string, string> = {
getOpsxOnboardCommandTemplate: '0cf66e164c0e14c916c6d1ebb5d80ded07d7fb8e55d4eb34eba43e8ca9c28558',
getOpsxBulkArchiveCommandTemplate: 'cb1d55d6ce53686bfe94be5e081c7a4d06a8e4d10b63019132df5bb3db7144cb',
getOpsxVerifyCommandTemplate: '9f3b4551a3cce3f1d1aa6c14fc595424ba22cd049326412460b30bd32e4cb0fc',
getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8',
getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c',
getOpsxProposeSkillTemplate: 'd5e0e2d675cedaffbd771892f15edd8f7f55b464ee42362b1572aa59c4c1d492',
getOpsxProposeCommandTemplate: 'f30af4ce5b7677bd664da71dd6d616a93278335e8828a7c3442fab59c339a11b',
getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133',
getUpdateChangeSkillTemplate: '14c209b8a18d1b100d372b0ef96707b70a7c64117a16309ee463f66d39180b3e',
getOpsxUpdateCommandTemplate: '33528a80c9436c9dad74084a37d2fce5e78e34abfb71185fa21495a4f2a8b4d3',
};
const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record<string, string> = {
'openspec-explore': '92055ecb0f60011b76369defb441883a985fb798f4cb90397f956f73383deda2',
'openspec-explore': '90a87227a7d3dca576448bb16936cede31b4e4666d35a6f0744e0b9bc628f1ac',
'openspec-new-change': '27e09d43785953827efc9a98bb9d6cf06db48fe6abe7e1c049409fe5b5061323',
'openspec-continue-change': '4edf6d8b220f09146029e7671f140b7ef94a666d8e29fe7ad78ce40bc7ec9017',
'openspec-continue-change': 'd8859091316ad43a58cd015091454a95bf49378ec9d9117b24ab9b2396dc8303',
'openspec-apply-change': '991a1d4687f1c8c7147d8c3b68cf14e9c515c6c53bb026356dac3b80e909b827',
'openspec-ff-change': 'a7ab656d46f04d45dff0c8888df4a126a2e62288b7336f7445bce4d1715055f5',
'openspec-ff-change': '224ff2ed2315eb23876fc4b83d796b60cf3cd05d853f2612e5b056ee7a6e93d2',
'openspec-sync-specs': '3909936a236a21a9a6d5bf495f90b396b3b68fc9220d7b2c1894668653beb2e4',
'openspec-archive-change': '5f0d131a885dcdcd9ba2172ea9a42bc6748125e24b8c4eecb7c86f1a4aea83af',
'openspec-bulk-archive-change': 'd2a258055ab2f0d8086c4348d37212ebc95a5adef2d5f524db959fb93490d5c8',
'openspec-verify-change': '9422e0afeec1dbd27c425258e67d52d73686c07765a54edeba62043c9640c8b5',
'openspec-onboard': '6993eff867d97d485e080078f9dfb80e968e242f3b17a924eeb077715fd548fa',
'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b',
'openspec-propose': '2bbfc169f483cbacc481bf5160aece61d5237e1bca80955f48f7e2a4814e6143',
'openspec-update-change': 'a6fc9d6972bfb024b91f0a01e14d8a7666ae641bcad83cfae937e41dcccc545a',
};
@@ -1212,15 +1212,19 @@ describe('skill templates split parity', () => {
expect(content, variant).toContain('the authoritative guidance');
expect(content, variant).not.toContain('Common artifact patterns');
// Delegated creation is honored at the creation step itself, and the
// delegated skill's output is verified rather than assumed.
// Delegated creation is honored at the creation step itself, held to the
// output path already chosen and checked (#2099), and the delegated
// skill's output is verified rather than assumed.
expect(content, variant).toContain(
'If the `instruction` field delegates creation to a specific skill or command, invoke it to produce the artifact instead of writing the file yourself, then verify the artifact file exists at `resolvedOutputPath`'
'If the `instruction` field delegates creation to a specific skill or command, invoke it only if it can write the artifact to that output path; otherwise stop and tell the user'
);
expect(content, variant).toMatch(
/(?:Then|Either way,) verify the artifact file exists at that output path/u
);
// ...and restated in the artifact-creation guidelines.
expect(content, variant).toContain(
'If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly'
'If the `instruction` field directs you to use a specific skill or command to create the artifact, invoke it instead of writing the artifact directly - but only once you have chosen and checked the output path, and only if it can write there'
);
}
});
@@ -59,8 +59,10 @@ describe('validate: deltas archive would refuse (#1112)', () => {
beforeEach(async () => {
testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-preflight-'));
changesDir = path.join(testDir, 'openspec', 'changes');
mainSpecsDir = path.join(testDir, 'openspec', 'specs');
// The project sits one level down, so a link written to testDir leaves it:
// a spec link that stays inside the project is allowed, one that leaves is not.
changesDir = path.join(testDir, 'repo', 'openspec', 'changes');
mainSpecsDir = path.join(testDir, 'repo', 'openspec', 'specs');
await fs.mkdir(changesDir, { recursive: true });
await fs.mkdir(mainSpecsDir, { recursive: true });
});
+18
View File
@@ -54,6 +54,24 @@ describe('pnpm workspace configuration', () => {
expect(workspace.overrides).toEqual(lockfile.overrides);
});
it('previews the built site without a server dependency', () => {
const packageJson = readJson('website/package.json');
const lockfile = readYaml('website/pnpm-lock.yaml');
// `pnpm start` ran `serve`, whose dev-only tree kept publishing advisories
// into the site's graph — four of them (brace-expansion x3, fast-uri) turned
// the Security workflow red on `main` and had to be pinned by hand, for code
// that never reached the published site. A first-party preview script has no
// tree to audit. Re-adding a server dependency means taking that back on
// purpose, so it should update this test too.
expect(packageJson.scripts.start).toBe('node scripts/preview.mjs');
expect(fs.existsSync(path.join(projectRoot, 'website/scripts/preview.mjs'))).toBe(true);
const serverPackages = Object.keys(lockfile.packages).filter((key) =>
/^(serve|serve-handler)@/.test(key)
);
expect(serverPackages).toEqual([]);
});
it('includes install policy changes in Nix and security validation', () => {
const flake = fs.readFileSync(path.join(projectRoot, 'flake.nix'), 'utf8');
const ci = fs.readFileSync(path.join(projectRoot, '.github/workflows/ci.yml'), 'utf8');
+46
View File
@@ -5,6 +5,7 @@ import {
isInteractive,
isNonInteractivePromptError,
resolveNoInteractive,
canRenderSpinner,
InteractiveOptions,
} from '../../src/utils/interactive.js';
@@ -382,4 +383,49 @@ describe('interactive utilities', () => {
).rejects.toThrow('stdin exploded');
});
});
describe('canRenderSpinner', () => {
it('refuses a TTY that reports zero columns', () => {
expect(canRenderSpinner({ isTTY: true, columns: 0 })).toBe(false);
});
it('refuses a TTY that reports no width at all', () => {
expect(canRenderSpinner({ isTTY: true })).toBe(false);
});
it('allows a TTY with a real width', () => {
expect(canRenderSpinner({ isTTY: true, columns: 80 })).toBe(true);
});
it('allows a non-TTY stream, which ora prints as one plain line', () => {
expect(canRenderSpinner({ isTTY: false })).toBe(true);
});
it('covers the case where ora clears without bound (#2106)', async () => {
// The reason the guard exists: with zero columns ora's line count is
// Infinity, so clear() keeps moving the cursor up forever. The fake stream
// is capped so the unbounded loop fails fast instead of hanging the suite.
// If a future ora bounds that loop this test goes red, and the guard can
// be revisited.
const ora = (await import('ora')).default;
let moves = 0;
const stream = {
isTTY: true,
columns: 0,
rows: 0,
write: () => true,
cursorTo: () => {},
clearLine: () => {},
moveCursor: () => {
if (++moves > 1000) throw new Error('unbounded clear');
},
once: () => {},
} as unknown as NodeJS.WriteStream;
const spinner = ora({ text: 'Validating...', stream, isEnabled: true });
spinner.render();
expect(() => spinner.render()).toThrow('unbounded clear');
expect(canRenderSpinner(stream)).toBe(false);
});
});
});
+38
View File
@@ -149,6 +149,44 @@ describe('discoverSpecFiles', () => {
});
});
it.skipIf(process.platform === 'win32')(
'discovers a spec.md linked to a file elsewhere in the project (#2100)',
async () => {
await withTempDir(async (dir) => {
// The project keeps its specs beside the product documentation and
// links openspec/specs/<capability>/spec.md to them.
const specsRoot = path.join(dir, 'repo', 'openspec', 'specs');
const docsSpec = path.join(dir, 'repo', 'docs', 'auth', 'specs', 'login.md');
await fs.mkdir(path.join(specsRoot, 'login'), { recursive: true });
await fs.mkdir(path.dirname(docsSpec), { recursive: true });
await fs.writeFile(docsSpec, '# login\n', 'utf8');
const link = path.join(specsRoot, 'login', 'spec.md');
await fs.symlink(path.relative(path.dirname(link), docsSpec), link, 'file');
const found = await discoverSpecFiles(specsRoot);
expect(found.map((s) => s.id)).toEqual(['login']);
expect(found[0].specFile).toBe(link);
});
}
);
it.skipIf(process.platform === 'win32')(
'rejects a spec.md symlink that leaves the project (#2100)',
async () => {
await withTempDir(async (dir) => {
const specsRoot = path.join(dir, 'repo', 'openspec', 'specs');
const outside = path.join(dir, 'outside.md');
await fs.mkdir(path.join(specsRoot, 'login'), { recursive: true });
await fs.writeFile(outside, '# Outside\n', 'utf8');
await fs.symlink(outside, path.join(specsRoot, 'login', 'spec.md'), 'file');
await expect(discoverSpecFiles(specsRoot)).rejects.toThrow(
'Path is outside the allowed directory'
);
});
}
);
it.skipIf(process.platform === 'win32')('skips a dangling spec.md symlink', async () => {
await withTempDir(async (dir) => {
await writeSpec(dir, 'real');
+192
View File
@@ -0,0 +1,192 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { promises as fs } from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {
assertDiscoveredSpecFile,
assertSpecFileWithin,
} from '../../src/utils/spec-path-containment.js';
const itWithSymlinks = it.skipIf(process.platform === 'win32');
/**
* Coverage for the containment rule itself rather than for a copy of it in a
* command. The cases run against real directories because the rule
* canonicalizes before deciding, so a purely notional path would not exercise
* it.
*/
describe('spec.md containment', () => {
let tempDir: string;
let projectDir: string;
let specsRoot: string;
beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-spec-containment-'));
projectDir = path.join(tempDir, 'repo');
specsRoot = path.join(projectDir, 'openspec', 'specs');
await fs.mkdir(path.join(specsRoot, 'login'), { recursive: true });
});
afterEach(async () => {
await fs.rm(tempDir, { recursive: true, force: true });
});
async function link(target: string): Promise<string> {
const linkPath = path.join(specsRoot, 'login', 'spec.md');
await fs.symlink(target, linkPath);
return linkPath;
}
for (const [name, assert] of [
['assertDiscoveredSpecFile', assertDiscoveredSpecFile],
['assertSpecFileWithin', assertSpecFileWithin],
] as const) {
describe(name, () => {
it('accepts an ordinary spec file', async () => {
const specFile = path.join(specsRoot, 'login', 'spec.md');
await fs.writeFile(specFile, '# login\n');
expect(() => assert(specsRoot, specFile)).not.toThrow();
});
it('rejects a path that traverses out of the specs root', () => {
expect(() =>
assert(specsRoot, path.join(specsRoot, '..', '..', '..', 'outside.md'))
).toThrow('Path is outside the allowed directory');
});
itWithSymlinks('accepts a link to a file elsewhere in the project', async () => {
const docsSpec = path.join(projectDir, 'docs', 'auth', 'specs', 'login.md');
await fs.mkdir(path.dirname(docsSpec), { recursive: true });
await fs.writeFile(docsSpec, '# login\n');
const linkPath = await link(path.relative(path.join(specsRoot, 'login'), docsSpec));
expect(() => assert(specsRoot, linkPath)).not.toThrow();
});
itWithSymlinks('rejects a link that leaves the project', async () => {
const outside = path.join(tempDir, 'outside.md');
await fs.writeFile(outside, '# outside\n');
const linkPath = await link(outside);
expect(() => assert(specsRoot, linkPath)).toThrow(
'Path is outside the allowed directory'
);
});
itWithSymlinks('rejects a dangling link rather than assuming it is contained', async () => {
const linkPath = await link(path.join(projectDir, 'docs', 'missing.md'));
expect(() => assert(specsRoot, linkPath)).toThrow();
});
itWithSymlinks(
'keeps a specs root with no project to name confined to itself',
async () => {
// A specs root reached outside an `openspec` directory (a store given
// by path) has no project boundary to widen to.
const bareRoot = path.join(tempDir, 'bare-specs');
await fs.mkdir(path.join(bareRoot, 'login'), { recursive: true });
const outside = path.join(tempDir, 'bare-outside.md');
await fs.writeFile(outside, '# outside\n');
const linkPath = path.join(bareRoot, 'login', 'spec.md');
await fs.symlink(outside, linkPath);
expect(() => assert(bareRoot, linkPath)).toThrow(
'Path is outside the allowed directory'
);
}
);
});
}
describe('the project boundary a specs root resolves to', () => {
async function linkedSpecUnder(
specsRoot: string,
target: string
): Promise<string> {
await fs.mkdir(path.join(specsRoot, 'login'), { recursive: true });
await fs.mkdir(path.dirname(target), { recursive: true });
await fs.writeFile(target, '# login\n');
const linkPath = path.join(specsRoot, 'login', 'spec.md');
await fs.symlink(path.relative(path.join(specsRoot, 'login'), target), linkPath);
return linkPath;
}
itWithSymlinks('reaches the project from a change\'s delta specs', async () => {
const deltaSpecs = path.join(projectDir, 'openspec', 'changes', 'c1', 'specs');
const linkPath = await linkedSpecUnder(
deltaSpecs,
path.join(projectDir, 'docs', 'shared-delta.md')
);
expect(() => assertDiscoveredSpecFile(deltaSpecs, linkPath)).not.toThrow();
});
itWithSymlinks('reaches the project from an archived change\'s delta specs', async () => {
const deltaSpecs = path.join(
projectDir,
'openspec',
'changes',
'archive',
'2026-01-01-c1',
'specs'
);
const linkPath = await linkedSpecUnder(
deltaSpecs,
path.join(projectDir, 'docs', 'shared-delta.md')
);
expect(() => assertDiscoveredSpecFile(deltaSpecs, linkPath)).not.toThrow();
});
itWithSymlinks(
'does not widen for a directory that merely shares the name `openspec`',
async () => {
// A store kept at `<somewhere>/openspec/my-store` must not hand out
// `<somewhere>` as its boundary: only `openspec/specs` and
// `openspec/changes` are planning roots.
const storeSpecs = path.join(tempDir, 'openspec', 'my-store', 'specs');
const linkPath = await linkedSpecUnder(
storeSpecs,
path.join(tempDir, 'elsewhere', 'login.md')
);
expect(() => assertDiscoveredSpecFile(storeSpecs, linkPath)).toThrow(
'Path is outside the allowed directory'
);
}
);
});
itWithSymlinks(
'assertDiscoveredSpecFile treats a linked capability directory as its own trust root',
async () => {
const external = path.join(tempDir, 'other-repo', 'login');
await fs.mkdir(external, { recursive: true });
await fs.writeFile(path.join(external, 'spec.md'), '# login\n');
const capabilityLink = path.join(specsRoot, 'shared');
await fs.symlink(external, capabilityLink);
expect(() =>
assertDiscoveredSpecFile(specsRoot, path.join(capabilityLink, 'spec.md'))
).not.toThrow();
}
);
itWithSymlinks(
'assertSpecFileWithin refuses a capability directory swapped for a link',
async () => {
// The recheck before a read or a write must not trust a directory that
// became a link after the path was resolved.
const external = path.join(tempDir, 'other-repo', 'login');
await fs.mkdir(external, { recursive: true });
await fs.writeFile(path.join(external, 'spec.md'), '# login\n');
await fs.rm(path.join(specsRoot, 'login'), { recursive: true });
await fs.symlink(external, path.join(specsRoot, 'login'));
expect(() =>
assertSpecFileWithin(specsRoot, path.join(specsRoot, 'login', 'spec.md'))
).toThrow('Path is outside the allowed directory');
}
);
});
+207
View File
@@ -0,0 +1,207 @@
import fs from 'fs';
import os from 'os';
import path from 'path';
import type { AddressInfo } from 'net';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
type PreviewModule = {
resolveStaticFile(root: string, requestPath: string): string | null;
contentTypeFor(filePath: string): string;
createPreviewServer(root: string): import('http').Server;
};
// A literal relative specifier, so the runner resolves it the same way it
// resolves every other import in this suite. Building the path at runtime
// instead handed it a Windows absolute path, which is not a module specifier:
// the suite failed to load on windows-pwsh with `SyntaxError: Invalid or
// unexpected token`, and a `file://` URL did not fix it either.
// `test/pnpm-workspace-config.test.ts` pins the script's location.
const preview = (await import(
'../website/scripts/preview.mjs'
)) as unknown as PreviewModule;
/**
* Mirrors a Next static export: a route is written as `docs/cli.html`, and a
* directory of the same name holds its data files but no index.html.
*/
function buildFixture(): string {
// The export sits one level down, so files placed "outside" it still live
// inside this run's own temporary directory.
const parent = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-preview-'));
const root = path.join(parent, 'out');
fs.mkdirSync(root);
fs.writeFileSync(path.join(root, 'index.html'), '<html>home</html>');
fs.writeFileSync(path.join(root, '404.html'), '<html>missing</html>');
fs.writeFileSync(path.join(root, 'robots.txt'), 'User-agent: *\n');
fs.mkdirSync(path.join(root, 'docs', 'cli'), { recursive: true });
fs.writeFileSync(path.join(root, 'docs', 'cli.html'), '<html>cli</html>');
fs.writeFileSync(path.join(root, 'docs', 'cli', 'data.txt'), 'data');
fs.mkdirSync(path.join(root, 'guide'), { recursive: true });
fs.writeFileSync(path.join(root, 'guide', 'index.html'), '<html>guide</html>');
return root;
}
/** A file symlink, or false where the OS refuses one (Windows without the privilege). */
function linkFile(target: string, linkPath: string): boolean {
try {
fs.symlinkSync(target, linkPath, 'file');
return true;
} catch (error) {
if ((error as NodeJS.ErrnoException).code === 'EPERM') return false;
throw error;
}
}
describe('website preview server', () => {
let root: string;
beforeAll(() => {
root = buildFixture();
});
afterAll(() => {
fs.rmSync(path.dirname(root), { recursive: true, force: true });
});
describe('resolveStaticFile', () => {
it('serves the root index', () => {
expect(preview.resolveStaticFile(root, '/')).toBe(path.join(root, 'index.html'));
});
it('falls back to the exported .html file for a route', () => {
expect(preview.resolveStaticFile(root, '/docs/cli')).toBe(
path.join(root, 'docs', 'cli.html')
);
});
it('resolves a route the same way with a trailing slash', () => {
expect(preview.resolveStaticFile(root, '/docs/cli/')).toBe(
path.join(root, 'docs', 'cli.html')
);
});
it('prefers a directory index when one exists', () => {
expect(preview.resolveStaticFile(root, '/guide')).toBe(
path.join(root, 'guide', 'index.html')
);
});
it('serves a file by its exact path', () => {
expect(preview.resolveStaticFile(root, '/robots.txt')).toBe(path.join(root, 'robots.txt'));
});
it('returns null for a path with no match', () => {
expect(preview.resolveStaticFile(root, '/nope')).toBeNull();
});
it('refuses to escape the export directory', () => {
const outside = path.join(root, '..', 'escaped.txt');
fs.writeFileSync(outside, 'secret');
try {
expect(preview.resolveStaticFile(root, '/../escaped.txt')).toBeNull();
expect(preview.resolveStaticFile(root, '/%2e%2e/escaped.txt')).toBeNull();
expect(preview.resolveStaticFile(root, '/docs/../../escaped.txt')).toBeNull();
} finally {
fs.rmSync(outside, { force: true });
}
});
it('refuses a symlink under the export that points outside it', () => {
const outsideDir = path.join(path.dirname(root), 'private');
fs.mkdirSync(outsideDir, { recursive: true });
fs.writeFileSync(path.join(outsideDir, 'secret.txt'), 'secret');
// A junction needs no privilege on Windows and is a link to Node.
fs.symlinkSync(outsideDir, path.join(root, 'linked'), 'junction');
const fileLinked = linkFile(path.join(outsideDir, 'secret.txt'), path.join(root, 'secret.txt'));
try {
expect(preview.resolveStaticFile(root, '/linked/secret.txt')).toBeNull();
if (fileLinked) expect(preview.resolveStaticFile(root, '/secret.txt')).toBeNull();
} finally {
fs.rmSync(path.join(root, 'linked'), { force: true, recursive: true });
fs.rmSync(path.join(root, 'secret.txt'), { force: true });
fs.rmSync(outsideDir, { recursive: true, force: true });
}
});
it('follows a symlink that stays inside the export', (ctx) => {
if (!linkFile(path.join(root, 'robots.txt'), path.join(root, 'robots-link.txt'))) {
ctx.skip();
}
try {
expect(preview.resolveStaticFile(root, '/robots-link.txt')).toBe(
path.join(root, 'robots-link.txt')
);
} finally {
fs.rmSync(path.join(root, 'robots-link.txt'), { force: true });
}
});
it('rejects malformed encoding and null bytes', () => {
expect(preview.resolveStaticFile(root, '/%E0%A4%A')).toBeNull();
expect(preview.resolveStaticFile(root, '/index.html%00.txt')).toBeNull();
});
});
describe('contentTypeFor', () => {
it('names the common static types', () => {
expect(preview.contentTypeFor('a/b.html')).toBe('text/html; charset=utf-8');
expect(preview.contentTypeFor('a/b.css')).toBe('text/css; charset=utf-8');
expect(preview.contentTypeFor('a/b.svg')).toBe('image/svg+xml');
expect(preview.contentTypeFor('a/b.woff2')).toBe('font/woff2');
});
it('falls back to a byte stream for anything else', () => {
expect(preview.contentTypeFor('a/b.unknown')).toBe('application/octet-stream');
});
});
describe('over HTTP', () => {
it('serves pages and answers a miss with the 404 page', async () => {
const server = preview.createPreviewServer(root);
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const { port } = server.address() as AddressInfo;
try {
const page = await fetch(`http://127.0.0.1:${port}/docs/cli`);
expect(page.status).toBe(200);
expect(page.headers.get('content-type')).toBe('text/html; charset=utf-8');
expect(await page.text()).toBe('<html>cli</html>');
const missing = await fetch(`http://127.0.0.1:${port}/nope`);
expect(missing.status).toBe(404);
expect(await missing.text()).toBe('<html>missing</html>');
} finally {
await new Promise<void>((resolve, reject) =>
server.close((error) => (error ? reject(error) : resolve()))
);
}
});
it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)(
'answers an unreadable file with an error instead of crashing',
async () => {
const locked = path.join(root, 'locked.txt');
fs.writeFileSync(locked, 'locked');
fs.chmodSync(locked, 0o000);
const server = preview.createPreviewServer(root);
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const { port } = server.address() as AddressInfo;
try {
const failed = await fetch(`http://127.0.0.1:${port}/locked.txt`);
expect(failed.status).toBe(500);
// Still serving afterwards.
const page = await fetch(`http://127.0.0.1:${port}/docs/cli`);
expect(page.status).toBe(200);
} finally {
fs.chmodSync(locked, 0o644);
fs.rmSync(locked, { force: true });
server.closeAllConnections();
await new Promise<void>((resolve, reject) =>
server.close((error) => (error ? reject(error) : resolve()))
);
}
}
);
});
});
+1 -2
View File
@@ -8,7 +8,7 @@
"sync:docs:watch": "node --watch-path=../docs --watch-path=../docs-lab --watch-path=docs.sync.config.mjs --watch-preserve-output scripts/sync-docs.mjs",
"build": "pnpm run sync:docs && fumadocs-mdx && next build",
"dev": "pnpm run sync:docs && (pnpm run sync:docs:watch & next dev)",
"start": "serve out",
"start": "node scripts/preview.mjs",
"types:check": "pnpm run sync:docs && fumadocs-mdx && next typegen && tsc --noEmit"
},
"dependencies": {
@@ -29,7 +29,6 @@
"@types/react": "^19.3.0",
"@types/react-dom": "^19.3.0",
"postcss": "^8.5.29",
"serve": "^14.2.6",
"tailwindcss": "^4.3.1",
"typescript": "^6.0.3"
}
-627
View File
File diff suppressed because it is too large Load Diff
-10
View File
@@ -12,17 +12,7 @@ allowBuilds:
overrides:
postcss: ^8.5.28
sharp: ^0.35.3
# GHSA-6j4f-fj2g-mc7p, GHSA-qhr7-859c-m2p7, GHSA-q2hr-2g5m-vwhr — brace-expansion
# DoS via nested or comma-heavy brace groups. Dev-only (serve > serve-handler >
# minimatch); never in the static site.
brace-expansion@<5.0.12: '>=5.0.12 <6'
# GHSA-hrr3-gc8f-f4qj — fast-uri. Dev-only (serve > ajv).
fast-uri@<3.1.8: ^3.1.8
# GHSA-2v37-7h3g-55p8 / CVE-2026-67213 — nanoid infinite loop on size=0. Build-time
# only (transitive via postcss); this is a statically exported site with no server
# runtime. Remove once transitive nanoid is >=3.3.17 (check: pnpm why nanoid).
nanoid@<3.3.17: '>=3.3.17 <4'
# GHSA-vc2v-76pw-4v95 — compression memory-leak DoS on premature response close.
# Dev-only (serve > compression); never in the static site. Remove once
# transitive compression is >=1.8.2 (check: pnpm why compression).
compression@<1.8.2: '>=1.8.2 <2'
+169
View File
@@ -0,0 +1,169 @@
// Serve the built static site in `out/` for a local look at what Cloudflare
// Pages will publish. Backs `pnpm start`.
//
// Deliberately dependency-free. This replaced `serve`, whose transitive tree
// kept publishing advisories into a dev-only tool: four of them (brace-expansion
// x3, fast-uri) turned the Security workflow red on `main` and had to be pinned
// by hand, and none of that code ever reached the published site. A preview
// server for a static export is small enough to own outright.
//
// Routing approximates Cloudflare Pages for a Next static export:
// / -> out/index.html
// /docs/cli -> out/docs/cli.html (or out/docs/cli/index.html)
// /docs/cli/ -> the same, so links work with or without a trailing slash
// anything else -> out/404.html with status 404
//
// `out/_redirects` is a Cloudflare feature and is not applied here, same as
// before: the preview serves the exported files as they are.
import { createServer } from 'node:http';
import { createReadStream, existsSync, realpathSync, statSync } from 'node:fs';
import { dirname, extname, join, normalize, resolve, sep } from 'node:path';
import { fileURLToPath } from 'node:url';
const websiteRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..');
const outDir = join(websiteRoot, 'out');
const CONTENT_TYPES = {
'.html': 'text/html; charset=utf-8',
'.js': 'text/javascript; charset=utf-8',
'.mjs': 'text/javascript; charset=utf-8',
'.css': 'text/css; charset=utf-8',
'.json': 'application/json; charset=utf-8',
'.map': 'application/json; charset=utf-8',
'.txt': 'text/plain; charset=utf-8',
'.xml': 'application/xml; charset=utf-8',
'.svg': 'image/svg+xml',
'.png': 'image/png',
'.jpg': 'image/jpeg',
'.jpeg': 'image/jpeg',
'.gif': 'image/gif',
'.webp': 'image/webp',
'.avif': 'image/avif',
'.ico': 'image/x-icon',
'.woff': 'font/woff',
'.woff2': 'font/woff2',
'.ttf': 'font/ttf',
'.otf': 'font/otf',
};
export function contentTypeFor(filePath) {
return CONTENT_TYPES[extname(filePath).toLowerCase()] ?? 'application/octet-stream';
}
/**
* Resolves a request path to a file inside `root`, or null when nothing matches.
*
* Escaping `root` is never a match: the decoded path is normalized and the
* result is checked to still sit under `root`, so `..` segments and an
* encoded `%2e%2e` both fall through to the 404 page. A candidate is then
* checked again after resolving symlinks, so a link under `root` that points
* outside it is not served either.
*/
export function resolveStaticFile(root, requestPath) {
let decoded;
try {
decoded = decodeURIComponent(requestPath);
} catch {
return null; // Malformed percent-encoding.
}
if (decoded.includes('\0')) return null;
// Normalize in POSIX terms first: a request path is URL-shaped, so its
// separator is `/` on every platform.
// Trailing separators are dropped so a directory request and the `.html`
// fallback for it build the same base path.
const relativePath = normalize(decoded.replace(/^\/+/, '').split('/').join(sep)).replace(
new RegExp(`\\${sep}+$`),
''
);
const base = relativePath === '' || relativePath === '.' ? root : join(root, relativePath);
if (base !== root && !base.startsWith(root + sep)) return null;
// Next's export writes a route as `docs/cli.html`, not `docs/cli/index.html`,
// so both spellings have to resolve either way the link was written.
const candidates = decoded.endsWith('/')
? [join(base, 'index.html'), `${base}.html`]
: [base, join(base, 'index.html'), `${base}.html`];
let realRoot;
try {
realRoot = realpathSync(root);
} catch {
return null;
}
for (const candidate of candidates) {
if (candidate !== root && !candidate.startsWith(root + sep)) continue;
try {
// The lexical check above does not see symlinks; the real path does.
const real = realpathSync(candidate);
if (real !== realRoot && !real.startsWith(realRoot + sep)) continue;
if (statSync(real).isFile()) return candidate;
} catch {
// Missing, or removed since the request arrived: not a match.
}
}
return null;
}
/**
* Streams a file, sending headers only once it is open. A file that is gone or
* unreadable by then gets a plain error response instead of crashing the
* server with an unhandled stream error.
*/
function sendFile(response, status, filePath) {
const stream = createReadStream(filePath);
stream.once('open', () => {
response.writeHead(status, { 'content-type': contentTypeFor(filePath) });
stream.pipe(response);
});
stream.once('error', () => {
if (response.headersSent) {
response.destroy();
return;
}
const failed = status === 404 ? [404, 'Not found\n'] : [500, 'Could not read file\n'];
response.writeHead(failed[0], { 'content-type': 'text/plain; charset=utf-8' });
response.end(failed[1]);
});
}
/** The preview server for a built export directory. Exported for tests. */
export function createPreviewServer(root) {
return createServer((request, response) => {
const requestPath = new URL(request.url ?? '/', 'http://localhost').pathname;
const filePath = resolveStaticFile(root, requestPath);
if (!filePath) {
const notFound = resolveStaticFile(root, '/404.html');
if (notFound) {
sendFile(response, 404, notFound);
return;
}
response.writeHead(404, { 'content-type': 'text/plain; charset=utf-8' });
response.end('Not found\n');
return;
}
sendFile(response, 200, filePath);
});
}
function main() {
if (!existsSync(outDir)) {
console.error(`No build found at ${outDir}. Run "pnpm build" first.`);
process.exit(1);
}
const port = Number(process.env.PORT ?? 3000);
const host = process.env.HOST ?? 'localhost';
createPreviewServer(outDir).listen(port, host, () => {
console.log(`Serving ${outDir} at http://${host}:${port}`);
});
}
// Importing this file for its helpers must not start a server.
if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
main();
}