diff --git a/.changeset/add-codearts-tool.md b/.changeset/add-codearts-tool.md new file mode 100644 index 0000000000..d18a465c28 --- /dev/null +++ b/.changeset/add-codearts-tool.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Add CodeArts Agent skills support: `openspec init --tools codeartsagent` installs the workflow skills. diff --git a/.changeset/add-hermes-tool.md b/.changeset/add-hermes-tool.md new file mode 100644 index 0000000000..eed0b2213f --- /dev/null +++ b/.changeset/add-hermes-tool.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Add Hermes Agent as a supported AI tool: `openspec init --tools hermes` installs the workflow skills (Hermes is skills-only and invokes them directly). diff --git a/.changeset/add-zcode-tool.md b/.changeset/add-zcode-tool.md new file mode 100644 index 0000000000..4a39b7703a --- /dev/null +++ b/.changeset/add-zcode-tool.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Add ZCode as a supported AI tool: `openspec init --tools zcode` generates its skills and `/opsx:*` commands. diff --git a/.changeset/avoid-npx-profile-changes.md b/.changeset/avoid-npx-profile-changes.md new file mode 100644 index 0000000000..4e250e4bd6 --- /dev/null +++ b/.changeset/avoid-npx-profile-changes.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Apply profile changes with the installed CLI instead of shelling out to `npx`, which could run a different version. diff --git a/.changeset/bom-delta-parsing.md b/.changeset/bom-delta-parsing.md new file mode 100644 index 0000000000..ee6e7307a9 --- /dev/null +++ b/.changeset/bom-delta-parsing.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Delta and main-spec parsers strip a UTF-8 BOM, so files saved by Windows editors or PowerShell redirects no longer fail with "No delta sections found". diff --git a/.changeset/change-name-length.md b/.changeset/change-name-length.md new file mode 100644 index 0000000000..cd053ba251 --- /dev/null +++ b/.changeset/change-name-length.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec new change` rejects names over 200 characters with a validation message instead of surfacing a raw ENAMETOOLONG filesystem error. diff --git a/.changeset/codex-skills-only.md b/.changeset/codex-skills-only.md new file mode 100644 index 0000000000..5866f48d40 --- /dev/null +++ b/.changeset/codex-skills-only.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Codex is now skills-only: workflows install as `$openspec-*` skills and previously managed custom prompts are retired (existing ones are cleaned up on update). diff --git a/.changeset/doctor-store-drift.md b/.changeset/doctor-store-drift.md new file mode 100644 index 0000000000..d106b5d86e --- /dev/null +++ b/.changeset/doctor-store-drift.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec doctor` now notes when a store checkout is behind its upstream ref. diff --git a/.changeset/drift-check-multiplicity.md b/.changeset/drift-check-multiplicity.md new file mode 100644 index 0000000000..5510ec28ec --- /dev/null +++ b/.changeset/drift-check-multiplicity.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Make the archive scenario-drift check multiplicity-aware: a MODIFIED block that keeps only one of two same-named scenarios no longer silently drops the other. diff --git a/.changeset/feedback-manual-fallback.md b/.changeset/feedback-manual-fallback.md new file mode 100644 index 0000000000..1ef59f8004 --- /dev/null +++ b/.changeset/feedback-manual-fallback.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec feedback` shows the formatted text and a pre-filled submission URL on any gh failure (issues disabled, network, rate limit), not only when gh is missing or unauthenticated. diff --git a/.changeset/fence-aware-drift-check.md b/.changeset/fence-aware-drift-check.md new file mode 100644 index 0000000000..5e85c7a6dd --- /dev/null +++ b/.changeset/fence-aware-drift-check.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +The archive scenario-drift check now ignores `#### Scenario:` lines inside fenced code blocks, matching validate: a fenced example no longer false-aborts an archive, and a fenced name no longer masks a genuinely dropped scenario. diff --git a/.changeset/gemini-toml-escaping.md b/.changeset/gemini-toml-escaping.md new file mode 100644 index 0000000000..4df52a851a --- /dev/null +++ b/.changeset/gemini-toml-escaping.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Gemini command files escape TOML-active characters (quotes, backslashes, control characters) in the description and prompt, so a template value containing them can no longer produce an invalid `.toml` file. diff --git a/.changeset/kimi-cli-to-kimi-code.md b/.changeset/kimi-cli-to-kimi-code.md new file mode 100644 index 0000000000..8daaf5b268 --- /dev/null +++ b/.changeset/kimi-cli-to-kimi-code.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Follow the Kimi CLI rename to Kimi Code: new install paths with automatic migration of existing `.kimi` setups. diff --git a/.changeset/local-dates-cli.md b/.changeset/local-dates-cli.md new file mode 100644 index 0000000000..b26946abef --- /dev/null +++ b/.changeset/local-dates-cli.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Use local dates for CLI date-only values (archive names, timestamps) instead of UTC, so late-evening archives no longer get tomorrow's date. diff --git a/.changeset/missing-core-workflows-warning.md b/.changeset/missing-core-workflows-warning.md new file mode 100644 index 0000000000..959c93269c --- /dev/null +++ b/.changeset/missing-core-workflows-warning.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec update` warns when a custom profile is missing core workflows instead of silently generating a partial install. diff --git a/.changeset/modified-noop-counting.md b/.changeset/modified-noop-counting.md new file mode 100644 index 0000000000..7072974d0e --- /dev/null +++ b/.changeset/modified-noop-counting.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Archive treats a MODIFIED delta whose content already matches the main spec as a no-op: a fully early-synced change now reports "Specs already in sync" instead of rewriting the file and claiming modifications. diff --git a/.changeset/multiselect-checkbox-markers.md b/.changeset/multiselect-checkbox-markers.md new file mode 100644 index 0000000000..caada04e08 --- /dev/null +++ b/.changeset/multiselect-checkbox-markers.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Render multi-select prompts with `[x]`/`[ ]` checkbox markers instead of radio-button icons. diff --git a/.changeset/nested-spec-discovery.md b/.changeset/nested-spec-discovery.md new file mode 100644 index 0000000000..7a6881a164 --- /dev/null +++ b/.changeset/nested-spec-discovery.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Discover nested spec paths like `specs///spec.md` recursively and consistently across parse, apply, and archive. diff --git a/.changeset/renamed-near-miss-guard.md b/.changeset/renamed-near-miss-guard.md new file mode 100644 index 0000000000..b852a8ef15 --- /dev/null +++ b/.changeset/renamed-near-miss-guard.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +An already-synced RENAMED delta aborts when a case/whitespace variant of the source requirement still exists — the same typo guard REMOVED deltas have. diff --git a/.changeset/resolve-open-questions.md b/.changeset/resolve-open-questions.md new file mode 100644 index 0000000000..731e1bc285 --- /dev/null +++ b/.changeset/resolve-open-questions.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Proposal guidance now resolves blocking open questions with the user instead of deferring them to design.md. diff --git a/.changeset/skills-sh-distribution.md b/.changeset/skills-sh-distribution.md new file mode 100644 index 0000000000..84ab95a6f9 --- /dev/null +++ b/.changeset/skills-sh-distribution.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Publish the workflow skills as static `skills//SKILL.md` files so `npx skills add Fission-AI/OpenSpec` works. diff --git a/.changeset/spec-content-guidance.md b/.changeset/spec-content-guidance.md new file mode 100644 index 0000000000..d5fca0ad94 --- /dev/null +++ b/.changeset/spec-content-guidance.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Specs instructions include the spec content guidance from the concepts docs, so generated specs follow the requirement/scenario format. diff --git a/.changeset/static-welcome-waits.md b/.changeset/static-welcome-waits.md new file mode 100644 index 0000000000..25dfe55356 --- /dev/null +++ b/.changeset/static-welcome-waits.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +The static welcome screen (reduced motion, `--no-animation`, narrow terminals) now waits for the Enter it asks for instead of letting the keystroke submit the tool picker unseen. diff --git a/.changeset/store-aware-main-specs.md b/.changeset/store-aware-main-specs.md new file mode 100644 index 0000000000..4de9d453dd --- /dev/null +++ b/.changeset/store-aware-main-specs.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Sync and archive workflows resolve main specs through the store-aware root instead of assuming `openspec/specs` in the repo. diff --git a/.changeset/symlinked-schema-dirs.md b/.changeset/symlinked-schema-dirs.md new file mode 100644 index 0000000000..1b1c80bf6b --- /dev/null +++ b/.changeset/symlinked-schema-dirs.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Resolve symlinked schema directories so schemas shared via symlink (e.g. from a dotfiles repo) are discovered. diff --git a/.changeset/update-check-detection-hardening.md b/.changeset/update-check-detection-hardening.md new file mode 100644 index 0000000000..fa54f8cef0 --- /dev/null +++ b/.changeset/update-check-detection-hardening.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +The stale-CLI check hardens its install detection: a directory merely named `volta` no longer changes the upgrade hint, the Windows npm-ownership check corroborates against the `openspec.cmd` shim npm actually writes, and a registry redirect from https to plain http is no longer followed. diff --git a/.changeset/update-check-redirect-teardown.md b/.changeset/update-check-redirect-teardown.md new file mode 100644 index 0000000000..069f445f14 --- /dev/null +++ b/.changeset/update-check-redirect-teardown.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +The stale-CLI check tears down a redirected registry connection when its time budget expires instead of leaving the socket open. diff --git a/.changeset/update-zero-artifact-notice.md b/.changeset/update-zero-artifact-notice.md new file mode 100644 index 0000000000..e84c1a6bc5 --- /dev/null +++ b/.changeset/update-zero-artifact-notice.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec update` with `delivery: commands` prints the same configuration correction as init when it removes the skills of a tool that supports only skills, instead of deleting them silently. diff --git a/.changeset/validator-unreadable-specs.md b/.changeset/validator-unreadable-specs.md new file mode 100644 index 0000000000..0f584da9ce --- /dev/null +++ b/.changeset/validator-unreadable-specs.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec validate` reports an unreadable specs/ directory as the error it is instead of misdiagnosing it as "no deltas found". diff --git a/.changeset/windows-welcome-input.md b/.changeset/windows-welcome-input.md new file mode 100644 index 0000000000..423362ed35 --- /dev/null +++ b/.changeset/windows-welcome-input.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Preserve keyboard input on Windows after the welcome screen instead of dropping the first keystrokes. diff --git a/.changeset/zsh-completions-custom-omz.md b/.changeset/zsh-completions-custom-omz.md new file mode 100644 index 0000000000..12bcc08f11 --- /dev/null +++ b/.changeset/zsh-completions-custom-omz.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +zsh completion install honors `$ZSH` and `$ZSH_CUSTOM`, so Oh My Zsh setups at custom locations get the completion where their shell actually loads it. diff --git a/flake.nix b/flake.nix index dab23fe8ef..dc02ee814a 100644 --- a/flake.nix +++ b/flake.nix @@ -51,7 +51,7 @@ inherit (finalAttrs) pname version src; pnpm = pkgs.pnpm_9; fetcherVersion = 3; - hash = "sha256-OUK3rXD0xjw2PPGQSzEeRnzN06SSKFRIkT0XHSIgDBU="; + hash = "sha256-z9NIWAY1KODgALBML1bBFpM2K9N7Z4L9jFBJC/t+Mww="; }; nativeBuildInputs = with pkgs; [ diff --git a/package.json b/package.json index 4f7522c8e8..fc89329e13 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "@types/node": "^20.19.43", "@vitest/ui": "^3.2.6", "eslint": "^10.5.0", + "smol-toml": "^1.7.1", "typescript": "^6.0.3", "typescript-eslint": "^8.65.0", "vitest": "^3.2.6" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3706b240d6..83f71fa159 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -57,6 +57,9 @@ importers: eslint: specifier: ^10.5.0 version: 10.7.0 + smol-toml: + specifier: ^1.7.1 + version: 1.7.1 typescript: specifier: ^6.0.3 version: 6.0.3 @@ -1382,6 +1385,10 @@ packages: resolution: {integrity: sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==} engines: {node: '>=8'} + smol-toml@1.7.1: + resolution: {integrity: sha512-PPlsspAZ4jbMBu5DMFhfUGDQLu/vrL4SyBROVS37x8ynnVmFIs1VPBz1Co8Xks3TvpIaZXmU85y4DrQ+UyVFoQ==} + engines: {node: '>= 18'} + source-map-js@1.2.1: resolution: {integrity: sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==} engines: {node: '>=0.10.0'} @@ -2896,6 +2903,8 @@ snapshots: slash@3.0.0: {} + smol-toml@1.7.1: {} + source-map-js@1.2.1: {} spawndamnit@3.0.1: diff --git a/src/commands/feedback.ts b/src/commands/feedback.ts index 529260401a..86d25042bd 100644 --- a/src/commands/feedback.ts +++ b/src/commands/feedback.ts @@ -131,9 +131,13 @@ function isMissingLabelError(error: any): boolean { } /** - * Report a gh CLI failure and exit, preserving gh's exit code + * Report a gh CLI failure and exit, preserving gh's exit code. + * + * gh failed after the user already typed their feedback (issues disabled, + * network, rate limit, ...), so show the same manual-submission path the + * missing-gh and unauthenticated flows get instead of discarding the text. */ -function reportGhFailure(error: any): void { +function reportGhFailure(error: any, title: string, body: string): void { // Display the error output from gh CLI if (error.stderr) { console.error(error.stderr.toString()); @@ -141,6 +145,12 @@ function reportGhFailure(error: any): void { console.error(error.message); } + displayFormattedFeedback(title, body); + + const manualUrl = generateManualSubmissionUrl(title, body); + console.log('Please submit your feedback manually:'); + console.log(manualUrl); + // Exit with the same code as gh CLI process.exit(error.status ?? 1); } @@ -181,7 +191,7 @@ function submitViaGhCli(title: string, body: string): void { issueUrl = createIssue(title, body, ['feedback']); } catch (error: any) { if (!isMissingLabelError(error)) { - reportGhFailure(error); + reportGhFailure(error, title, body); return; } @@ -191,7 +201,7 @@ function submitViaGhCli(title: string, body: string): void { issueUrl = createIssue(title, body, []); labelApplied = false; } catch (retryError: any) { - reportGhFailure(retryError); + reportGhFailure(retryError, title, body); return; } } diff --git a/src/core/command-generation/adapters/gemini.ts b/src/core/command-generation/adapters/gemini.ts index 2c08656f43..d3a4030513 100644 --- a/src/core/command-generation/adapters/gemini.ts +++ b/src/core/command-generation/adapters/gemini.ts @@ -7,6 +7,44 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; +/** + * Control characters (C0 except tab/newline/carriage return, plus DEL) are + * invalid inside TOML strings and must be written as escapes. + */ +const TOML_CONTROL_CHARS = new RegExp('[\\u0000-\\u0008\\u000b\\u000c\\u000e-\\u001f\\u007f]', 'g'); + +/** + * TOML basic strings are escape-active: a backslash or double quote in the + * value breaks the file if written raw. Newlines cannot appear in a + * single-line basic string at all, so they are escaped too. + */ +function escapeTomlBasicString(value: string): string { + return value + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r') + .replace(/\t/g, '\\t') + .replace(TOML_CONTROL_CHARS, (c) => `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`); +} + +/** + * Multiline basic strings keep raw newlines and tabs, but backslashes are + * still escape-active, any run of three quotes would end the string, and the + * same control characters are invalid as in single-line basic strings — a + * lone carriage return included (only LF and CRLF may appear raw; CRLF is + * normalized away so the emitted file is single-convention). Escapes are + * introduced after backslash-doubling so they are not re-doubled. + */ +function escapeTomlMultilineBasicString(value: string): string { + return value + .replace(/\r\n/g, '\n') + .replace(/\\/g, '\\\\') + .replace(/"""/g, '""\\"') + .replace(/\r/g, '\\r') + .replace(TOML_CONTROL_CHARS, (c) => `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`); +} + /** * Gemini adapter for command generation. * File path: .gemini/commands/opsx/.toml @@ -20,10 +58,10 @@ export const geminiAdapter: ToolCommandAdapter = { }, formatFile(content: CommandContent): string { - return `description = "${content.description}" + return `description = "${escapeTomlBasicString(content.description)}" prompt = """ -${content.body} +${escapeTomlMultilineBasicString(content.body)} """ `; }, diff --git a/src/core/command-generation/adapters/index.ts b/src/core/command-generation/adapters/index.ts index 358bc82767..43c2e36e65 100644 --- a/src/core/command-generation/adapters/index.ts +++ b/src/core/command-generation/adapters/index.ts @@ -31,3 +31,4 @@ export { lingmaAdapter } from './lingma.js'; export { qwenAdapter } from './qwen.js'; export { roocodeAdapter } from './roocode.js'; export { traeAdapter } from './trae.js'; +export { zcodeAdapter } from './zcode.js'; diff --git a/src/core/completions/command-registry.ts b/src/core/completions/command-registry.ts index 15bca61206..33db57e874 100644 --- a/src/core/completions/command-registry.ts +++ b/src/core/completions/command-registry.ts @@ -184,7 +184,7 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ }, { name: 'instructions', - description: 'Output enriched instructions for creating an artifact or applying tasks', + description: 'Output enriched instructions for artifacts, apply, or archive', acceptsPositional: true, positionals: [{ name: 'artifact', optional: true }], flags: [ diff --git a/src/core/completions/installers/zsh-installer.ts b/src/core/completions/installers/zsh-installer.ts index a405af1331..3b6d87d67a 100644 --- a/src/core/completions/installers/zsh-installer.ts +++ b/src/core/completions/installers/zsh-installer.ts @@ -35,16 +35,29 @@ export class ZshInstaller { } // Fall back to checking for ~/.oh-my-zsh directory - const ohMyZshPath = path.join(this.homeDir, '.oh-my-zsh'); - try { - const stat = await fs.stat(ohMyZshPath); + const stat = await fs.stat(this.ohMyZshRoot()); return stat.isDirectory(); } catch { return false; } } + /** + * Oh My Zsh exports its root as $ZSH; honor a custom location, or the + * completion lands in a ~/.oh-my-zsh tree that nothing ever loads. + */ + private ohMyZshRoot(): string { + return process.env.ZSH || path.join(this.homeDir, '.oh-my-zsh'); + } + + /** + * The custom dir is separately relocatable via $ZSH_CUSTOM. + */ + private ohMyZshCustomDir(): string { + return process.env.ZSH_CUSTOM || path.join(this.ohMyZshRoot(), 'custom'); + } + /** * Get the appropriate installation path for the completion script * @@ -56,7 +69,7 @@ export class ZshInstaller { if (isOhMyZsh) { // Oh My Zsh custom completions directory return { - path: path.join(this.homeDir, '.oh-my-zsh', 'custom', 'completions', '_openspec'), + path: path.join(this.ohMyZshCustomDir(), 'completions', '_openspec'), isOhMyZsh: true, }; } else { @@ -327,10 +340,14 @@ export class ZshInstaller { * @returns Array of guidance strings, or undefined if not needed */ private generateOhMyZshFpathGuidance(completionsDir: string): string[] | undefined { + // One fpath entry per line, matched as a literal: a relocated $ZSH_CUSTOM + // need not contain "custom/completions", and the path may hold characters + // grep would otherwise read as a pattern. Single-quoted for the shell. + const quotedDir = `'${completionsDir.replace(/'/g, `'\\''`)}'`; return [ 'Note: Oh My Zsh typically auto-loads completions from custom/completions.', `Verify that ${completionsDir} is in your fpath by running:`, - ' echo $fpath | grep "custom/completions"', + ` printf '%s\\n' $fpath | grep -F ${quotedDir}`, '', 'If not found, completions may not work. Restart your shell to ensure changes take effect.', ]; diff --git a/src/core/parsers/markdown-parser.ts b/src/core/parsers/markdown-parser.ts index 8dca1ef64f..4834f4795e 100644 --- a/src/core/parsers/markdown-parser.ts +++ b/src/core/parsers/markdown-parser.ts @@ -21,7 +21,8 @@ export class MarkdownParser { } protected static normalizeContent(content: string): string { - return content.replace(/\r\n?/g, '\n'); + // Strip a UTF-8 BOM so a header on the first line still matches. + return content.replace(/^/, '').replace(/\r\n?/g, '\n'); } parseSpec(name: string): Spec { diff --git a/src/core/parsers/requirement-blocks.ts b/src/core/parsers/requirement-blocks.ts index b47b9ecd45..cb0e79b75b 100644 --- a/src/core/parsers/requirement-blocks.ts +++ b/src/core/parsers/requirement-blocks.ts @@ -141,7 +141,9 @@ export interface DeltaPlan { } function normalizeLineEndings(content: string): string { - return content.replace(/\r\n?/g, '\n'); + // Strip a UTF-8 BOM: Windows editors and PowerShell redirects prepend one, + // and it would keep the first line's `## ADDED Requirements` from matching. + return content.replace(/^/, '').replace(/\r\n?/g, '\n'); } /** diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index db80ae8bf5..e8d2f3910f 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -294,6 +294,17 @@ export async function buildUpdatedSpec( // to the baseline (early-sync pattern) — re-applying it is a no-op, // not a failure. Only a missing source AND target is a genuine error. if (nameToBlock.has(to)) { + // Unless a case/whitespace variant of the source still exists (and is + // not the target itself, as in a case-only rename): that is a typo'd + // header, not an early-synced rename — same guard REMOVED applies. + const nearMiss = [...nameToBlock.keys()].find( + (k) => k !== to && foldRequirementName(k) === foldRequirementName(from) + ); + if (nearMiss !== undefined) { + throw new Error( + `${specName} RENAMED failed for header "### Requirement: ${r.from}" - source not found, but "### Requirement: ${nameToBlock.get(nearMiss)!.name}" exists; fix the header to match it exactly` + ); + } continue; } throw new Error(`${specName} RENAMED failed for header "### Requirement: ${r.from}" - source not found`); @@ -344,6 +355,7 @@ export async function buildUpdatedSpec( } // MODIFIED + let modifiedApplied = 0; for (const mod of plan.modified) { const key = normalizeRequirementName(mod.name); const currentBlock = nameToBlock.get(key); @@ -363,6 +375,13 @@ export async function buildUpdatedSpec( `${specName} MODIFIED failed for header "### Requirement: ${mod.name}" - current spec contains scenario(s) not present in the modified block: ${missingScenarios.map(name => `"${name}"`).join(', ')}. Refresh the change spec before archiving to avoid dropping scenarios.` ); } + // Identical content means the modification was already synced to the + // baseline (early-sync pattern) — count only real replacements, so a + // fully synced change still takes the "already in sync" write skip + // instead of churning normalization differences into the file. + if (normalizeBlockRaw(currentBlock.raw) !== normalizeBlockRaw(mod.raw)) { + modifiedApplied++; + } nameToBlock.set(key, mod); } @@ -419,7 +438,7 @@ export async function buildUpdatedSpec( rebuilt, counts: { added: addedApplied, - modified: plan.modified.length, + modified: modifiedApplied, removed: removedApplied, renamed: renamedApplied, }, @@ -579,11 +598,16 @@ function findMissingCurrentScenarios(current: RequirementBlock, incoming: Requir function parseScenarioBlocks(requirementRaw: string): ScenarioBlock[] { const lines = requirementRaw.replace(/\r\n?/g, '\n').split('\n'); + // A `#### Scenario:` inside a fenced example is not a real scenario. The + // validator's countScenarios already ignores fenced lines; the drift check + // must agree with it, or a fenced sample can false-abort an archive (or + // mask a genuinely dropped scenario). + const mask = buildCodeFenceMask(lines); const scenarios: ScenarioBlock[] = []; let index = 0; while (index < lines.length) { - const headerMatch = lines[index].match(/^####\s*Scenario:\s*(.+)\s*$/); + const headerMatch = mask[index] ? null : lines[index].match(/^####\s*Scenario:\s*(.+)\s*$/); if (!headerMatch) { index++; continue; @@ -592,7 +616,7 @@ function parseScenarioBlocks(requirementRaw: string): ScenarioBlock[] { const start = index; const name = headerMatch[1].trim(); index++; - while (index < lines.length && !/^####\s*Scenario:\s*(.+)\s*$/.test(lines[index])) { + while (index < lines.length && (mask[index] || !/^####\s*Scenario:\s*(.+)\s*$/.test(lines[index]))) { index++; } diff --git a/src/core/update.ts b/src/core/update.ts index 9d48f621ec..a39d42d7a3 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -247,6 +247,7 @@ export class UpdateCommand { const updatedTools: string[] = []; const failedTools: Array<{ name: string; error: string }> = []; const skillsInvocableCommandSkips: string[] = []; + const zeroArtifactTools: string[] = []; let removedCommandCount = 0; let removedSkillCount = 0; let removedDeselectedCommandCount = 0; @@ -288,6 +289,13 @@ export class UpdateCommand { // Delete skill directories if delivery is commands-only if (shouldRemoveSkillsForTool(tool.value, delivery)) { removedSkillCount += await this.removeSkillDirs(skillsDir); + // A tool with no command adapter now has zero OpenSpec artifacts; + // say so like init does, rather than deleting its skills silently + // and letting tool detection re-suggest an init that would also + // generate nothing under this delivery setting. + if (!shouldGenerateCommandsForTool(tool.value, delivery)) { + zeroArtifactTools.push(tool.name); + } } // Generate commands if delivery includes commands @@ -348,6 +356,16 @@ export class UpdateCommand { if (removedSkillCount > 0) { console.log(chalk.dim(`Removed: ${removedSkillCount} skill directories (delivery: commands)`)); } + if (zeroArtifactTools.length > 0) { + const names = zeroArtifactTools.join(', '); + console.log( + chalk.yellow( + `No skills or commands remain for ${names}: delivery is set to 'commands' but ` + + `${zeroArtifactTools.length === 1 ? 'it supports' : 'they support'} only skills. ` + + `Run 'openspec config set delivery both' to generate skills.` + ) + ); + } if (removedDeselectedCommandCount > 0) { console.log(chalk.dim(`Removed: ${removedDeselectedCommandCount} command files (deselected workflows)`)); } diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 25989f86e2..0086c12766 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -334,8 +334,16 @@ export class Validator { } } } - } catch { - // If no specs dir, treat as no deltas + } catch (error) { + // A missing specs dir (or a stray `specs` file) means no deltas; + // anything else (EACCES, EIO) must stay loud — discoverSpecFiles + // documents that silently dropping an unreadable capability recreates + // the data-loss class it prevents, and archive lets the same error + // propagate. + const code = (error as NodeJS.ErrnoException)?.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') { + throw error; + } } for (const { path: specPath, sections } of emptySectionSpecs) { diff --git a/src/core/version-check.ts b/src/core/version-check.ts index 1747e9d315..5033e3bb6e 100644 --- a/src/core/version-check.ts +++ b/src/core/version-check.ts @@ -158,6 +158,12 @@ function fetchLatestVersion(): Promise { // check would be permanently and silently dead for them. let redirectsLeft = MAX_REDIRECTS; + // The budget timer must tear down whichever request is open when it + // fires. Closing over the first hop's request would leave a redirected + // socket alive: a target that trickles bytes keeps resetting its idle + // timeout, and only the body-size cap would end it. + let activeRequest: http.ClientRequest | undefined; + const send = (target: URL): void => { const request = (target.protocol === 'http:' ? http : https).get( target, @@ -176,7 +182,10 @@ function fetchLatestVersion(): Promise { redirectsLeft -= 1; try { const next = new URL(location, target); - if (next.protocol === 'http:' || next.protocol === 'https:') { + // Never follow a downgrade to plain http: a MITM on the reply + // would control the "newer version" answer. + const downgrade = target.protocol === 'https:' && next.protocol === 'http:'; + if (!downgrade && (next.protocol === 'http:' || next.protocol === 'https:')) { send(next); return; } @@ -217,6 +226,8 @@ function fetchLatestVersion(): Promise { } ); + activeRequest = request; + request.on('timeout', () => { request.destroy(); finish(null); @@ -226,7 +237,7 @@ function fetchLatestVersion(): Promise { // One budget for the whole exchange, redirects included. if (!timer) { timer = setTimeout(() => { - request.destroy(); + activeRequest?.destroy(); finish(null); }, REQUEST_TIMEOUT_MS); } @@ -401,7 +412,13 @@ export function isNpmGlobalInstall( const prefix = npmPrefixFromInstallDir(installDir); if (!prefix) return false; try { - return fs.existsSync(process.platform === 'win32' ? prefix : path.join(prefix, 'bin')); + // Corroborate with something npm itself wrote: the bin dir on POSIX, the + // .cmd shim on Windows. The prefix alone proves nothing — it is just the + // parent of the node_modules dir the CLI resolved from, so a hand-copied + // portable tree would pass and be offered an npm upgrade it never had. + return fs.existsSync( + process.platform === 'win32' ? path.join(prefix, 'openspec.cmd') : path.join(prefix, 'bin') + ); } catch { return false; } @@ -432,7 +449,11 @@ export function detectPackageManager(installDir: string | null): PackageManager const segments = (installDir ?? '').split(/[\\/]/).map((segment) => segment.toLowerCase()); const has = (...names: string[]) => names.some((name) => segments.includes(name)); - if (has('.volta', 'volta')) return 'volta'; + // The undotted spelling exists for Windows (%LOCALAPPDATA%\Volta), whose + // layout nests tools\image; require both segments so a user or project + // directory merely named "volta" (even one with its own "tools" dir) does + // not steal the install. + if (has('.volta') || (has('volta') && has('tools') && has('image'))) return 'volta'; if (has('.bun')) return 'bun'; // These two need a corroborating segment: a directory merely named "pnpm" or // "yarn" (a user's home, a project) is not a global install of one. diff --git a/src/ui/welcome-screen.ts b/src/ui/welcome-screen.ts index e8e9fef2c6..9e33dcf4ab 100644 --- a/src/ui/welcome-screen.ts +++ b/src/ui/welcome-screen.ts @@ -184,9 +184,16 @@ export async function showWelcomeScreen( const textLines = getWelcomeText(workflows); if (options.animate === false || !canAnimate()) { - // Fallback: show static welcome + // Fallback: show static welcome. The "Press Enter" line is only honest + // when we actually wait; in a TTY, returning immediately would let the + // Enter it asks for fall through into the tool picker and submit the + // pre-selected tools sight-unseen. Without a TTY, drop the line instead. + const staticLines = process.stdin.isTTY + ? textLines + : textLines.filter((line) => !line.includes('Press Enter')); const frame = WELCOME_ANIMATION.frames[3]; // Peak frame - process.stdout.write('\n' + renderFrame(frame, textLines) + '\n\n'); + process.stdout.write('\n' + renderFrame(frame, staticLines) + '\n\n'); + await waitForEnter(); return; } diff --git a/src/utils/change-utils.ts b/src/utils/change-utils.ts index 11c678baa3..f73ba61bce 100644 --- a/src/utils/change-utils.ts +++ b/src/utils/change-utils.ts @@ -67,6 +67,13 @@ export function validateChangeName(name: string): ValidationResult { return { valid: false, error: 'Change name cannot be empty' }; } + // Filesystem directory components cap at 255 bytes and archive prepends a + // date prefix; bounding here turns the failure into a validation message + // instead of a raw ENAMETOOLONG from mkdir. + if (name.length > 200) { + return { valid: false, error: 'Change name is too long (200 characters max)' }; + } + if (!isKebabId(name)) { // Provide specific error messages for common mistakes if (/[A-Z]/.test(name)) { diff --git a/test/commands/feedback.test.ts b/test/commands/feedback.test.ts index 7545ecc52c..51fe40cd9d 100644 --- a/test/commands/feedback.test.ts +++ b/test/commands/feedback.test.ts @@ -344,6 +344,16 @@ describe('FeedbackCommand', () => { // A non-label failure must NOT be retried expect(mockExecFileSync).toHaveBeenCalledTimes(1); + + // ...and must not discard the typed feedback: the manual-submission + // fallback (formatted text + pre-filled URL) is shown like the + // missing-gh and unauthenticated flows. + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('Please submit your feedback manually:') + ); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('github.com/Fission-AI/OpenSpec/issues/new') + ); }); it('should not retry when the feedback text mentions the label error', async () => { diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index fd5a5d3132..8937eef399 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -488,6 +488,68 @@ Then expected result happens`; expect(process.exitCode).toBeUndefined(); }); + it('should archive when MODIFIED requirements were already synced to the baseline', async () => { + const changeName = 'early-synced-modify'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'mod-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + const block = `### Requirement: Session handling\nThe system SHALL keep sessions.\n\n#### Scenario: Session persists\n- **WHEN** a user returns\n- **THEN** the session is restored`; + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Mod Layer - Changes\n\n## MODIFIED Requirements\n\n${block}\n` + ); + + // Early-sync pattern: the modification is already applied to main. + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'mod-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# mod-layer Specification\n\n## Purpose\nSession layer behavior.\n\n## Requirements\n\n${block}\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // An identical MODIFIED block is a no-op: no churned rewrite, no + // claimed update, no "~ 1 modified" in the totals. + const updatedContent = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(updatedContent).toBe(mainSpecContent); + expect(console.log).toHaveBeenCalledWith('Specs already in sync; no files changed.'); + expect(console.log).not.toHaveBeenCalledWith('Specs updated successfully.'); + + const archives = await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive')); + expect(archives.some(a => a.includes(changeName))).toBe(true); + expect(process.exitCode).toBeUndefined(); + }); + + it('should abort an already-synced RENAMED when a case variant of the source still exists', async () => { + // FROM missing + TO present normally means the rename was early-synced, + // but a fold-variant of FROM still in the spec means the header is a + // typo - the same near-miss guard REMOVED applies. + const changeName = 'typo-rename'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'rename-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Rename Layer - Changes\n\n## RENAMED Requirements\n- FROM: \`### Requirement: cache policy\`\n- TO: \`### Requirement: Eviction policy\`\n` + ); + + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'rename-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# rename-layer Specification\n\n## Purpose\nCache behavior.\n\n## Requirements\n\n### Requirement: Cache Policy\nThe system SHALL cache.\n\n#### Scenario: Cached\n- **WHEN** data repeats\n- **THEN** it is served from cache\n\n### Requirement: Eviction policy\nThe system SHALL evict.\n\n#### Scenario: Evicted\n- **WHEN** the cache is full\n- **THEN** old entries are dropped\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('RENAMED failed for header "### Requirement: cache policy" - source not found, but "### Requirement: Cache Policy" exists') + ); + expect(process.exitCode).toBe(1); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + const untouched = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(untouched).toBe(mainSpecContent); + }); + it('should abort when a REMOVED header near-misses an existing requirement (case/whitespace typo)', async () => { // A fold-insensitive match in the current spec means the header is a // typo, not an early-synced removal - that case must stay a hard abort. @@ -2020,6 +2082,131 @@ The system SHALL authenticate. expect(archives.some(a => a.includes(changeName))).toBe(false); }); + it('should not treat a fenced scenario example in the current spec as real drift', async () => { + // The validator ignores fenced `#### Scenario:` lines (countScenarios is + // fence-aware); the drift check must agree, or a fenced sample in the + // current spec aborts an archive that validate said was fine. + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'fenced-current'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecPath = path.join(mainSpecDir, 'spec.md'); + await fs.writeFile( + mainSpecPath, + `# fenced-current Specification + +## Purpose +Fenced scenario samples in the current spec. + +## Requirements + +### Requirement: Reporting +The system SHALL report results using the scenario format: + +\`\`\`markdown +#### Scenario: Fenced sample +- **WHEN** shown as an example +- **THEN** it is not a real scenario +\`\`\` + +#### Scenario: Emit report +- **WHEN** a run finishes +- **THEN** a report is emitted` + ); + + const changeName = 'edit-fenced-current'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'fenced-current'); + await fs.mkdir(changeSpecDir, { recursive: true }); + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Edit Fenced Current - Change + +## MODIFIED Requirements + +### Requirement: Reporting +The system SHALL report results in JSON. + +#### Scenario: Emit report +- **WHEN** a run finishes +- **THEN** a JSON report is emitted` + ); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + const updated = await fs.readFile(mainSpecPath, 'utf-8'); + expect(updated).toContain('The system SHALL report results in JSON.'); + expect(updated).toContain('a JSON report is emitted'); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('current spec contains scenario(s) not present in the modified block') + ); + const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); + const archives = await fs.readdir(archiveDir); + expect(archives.some(a => a.includes(changeName))).toBe(true); + }); + + it('should abort when a MODIFIED block only keeps a dropped scenario inside a fence', async () => { + // The inverse hole: a fenced `#### Scenario: Audit` in the incoming block + // must not count as keeping the real Audit scenario the block dropped. + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'fenced-incoming'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecPath = path.join(mainSpecDir, 'spec.md'); + await fs.writeFile( + mainSpecPath, + `# fenced-incoming Specification + +## Purpose +Fenced scenario names in the incoming block. + +## Requirements + +### Requirement: Access log +The system SHALL log access. + +#### Scenario: Audit +- **WHEN** a user signs in +- **THEN** an audit row is written` + ); + + const changeName = 'drop-audit-behind-fence'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'fenced-incoming'); + await fs.mkdir(changeSpecDir, { recursive: true }); + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Drop Audit Behind Fence - Change + +## MODIFIED Requirements + +### Requirement: Access log +The system SHALL log access, for example: + +\`\`\`markdown +#### Scenario: Audit +- **WHEN** shown as an example +- **THEN** it is not a real scenario +\`\`\` + +#### Scenario: Trace +- **WHEN** a request is served +- **THEN** a trace row is written` + ); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + const updated = await fs.readFile(mainSpecPath, 'utf-8'); + // Spec must be untouched — the real Audit scenario preserved. + expect(updated).toContain('an audit row is written'); + expect(updated).not.toContain('Trace'); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + 'fenced-incoming MODIFIED failed for header "### Requirement: Access log" - current spec contains scenario(s) not present in the modified block: "Audit"' + ) + ); + expect(console.log).toHaveBeenCalledWith('Aborted. No files were changed.'); + const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); + const archives = await fs.readdir(archiveDir); + expect(archives.some(a => a.includes(changeName))).toBe(false); + }); + it('should abort with a structural error when target spec hides requirements outside ## Requirements', async () => { const changeName = 'hidden-requirement-target'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); diff --git a/test/core/command-generation/adapters.test.ts b/test/core/command-generation/adapters.test.ts index 43dbea5d4e..f4d946e565 100644 --- a/test/core/command-generation/adapters.test.ts +++ b/test/core/command-generation/adapters.test.ts @@ -35,6 +35,7 @@ import type { import { CommandAdapterRegistry } from '../../../src/core/command-generation/registry.js'; import { generateCommand } from '../../../src/core/command-generation/generator.js'; import { parse as parseYaml } from 'yaml'; +import { parse as parseToml } from 'smol-toml'; describe('command-generation/adapters', () => { const sampleContent: CommandContent = { @@ -414,6 +415,55 @@ describe('command-generation/adapters', () => { expect(output).toContain('This is the command body.'); expect(output).toContain('"""'); }); + + it('escapes TOML-active characters in the description', () => { + const output = geminiAdapter.formatFile({ + ...sampleContent, + description: 'Say "hi" to C:\\Users and\nmore', + }); + // Basic strings are escape-active: quotes, backslashes, and newlines + // must be written as escapes or the file stops parsing as TOML. + expect(output).toContain('description = "Say \\"hi\\" to C:\\\\Users and\\nmore"'); + expect((parseToml(output) as { description: string }).description).toBe( + 'Say "hi" to C:\\Users and\nmore' + ); + }); + + it('keeps the prompt a single multiline string when the body carries fences and backslashes', () => { + const body = 'Windows path C:\\temp and a quote run: """ done'; + const output = geminiAdapter.formatFile({ ...sampleContent, body }); + // Backslashes must be escaped and no unescaped quote-triple may remain, + // or the """ delimiter ends the prompt early. + expect(output).toContain('C:\\\\temp'); + expect(output).toContain('""\\" done'); + const delimiters = output.match(/(? = [ + ['control characters', 'null:\u0000 vt:\u000b ff:\u000c end', 'null:\u0000 vt:\u000b ff:\u000c end'], + // A lone CR is illegal raw in a multiline basic string (only LF and + // CRLF may appear); Python tomllib rejects it — so must never be + // emitted bare. + ['a lone carriage return', 'a\rb', 'a\rb'], + ['CRLF line endings (normalized to LF)', 'line one\r\nline two\r\n', 'line one\nline two\n'], + ['a CR before a quote run', 'x\r""" y', 'x\r""" y'], + ['a trailing backslash', 'ends with a backslash \\', 'ends with a backslash \\'], + ['quote runs of four and five', 'four """" five """""', 'four """" five """""'], + ]; + + for (const [label, body, expected] of HOSTILE_BODIES) { + it(`emits parseable TOML for a body with ${label}`, () => { + const output = geminiAdapter.formatFile({ ...sampleContent, body }); + const parsed = parseToml(output) as { description: string; prompt: string }; + expect(parsed.prompt).toBe(`${expected}\n`); + expect(parsed.description).toBe(sampleContent.description); + }); + } }); describe('githubCopilotAdapter', () => { diff --git a/test/core/completions/installers/zsh-installer.test.ts b/test/core/completions/installers/zsh-installer.test.ts index 91100d03e2..07348ee9bb 100644 --- a/test/core/completions/installers/zsh-installer.test.ts +++ b/test/core/completions/installers/zsh-installer.test.ts @@ -8,12 +8,16 @@ describe('ZshInstaller', () => { let testHomeDir: string; let installer: ZshInstaller; let originalZsh: string | undefined; + let originalZshCustom: string | undefined; beforeEach(async () => { - // Clear $ZSH (set by a real Oh My Zsh install) so isOhMyZshInstalled() - // falls through to the isolated test home directory + // Clear $ZSH and $ZSH_CUSTOM (set by a real Oh My Zsh install) so the + // installer resolves against the isolated test home directory instead of + // reading — or writing into — the developer's real OMZ tree originalZsh = process.env.ZSH; delete process.env.ZSH; + originalZshCustom = process.env.ZSH_CUSTOM; + delete process.env.ZSH_CUSTOM; // Create a temporary home directory for testing testHomeDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-zsh-test-')); @@ -27,6 +31,11 @@ describe('ZshInstaller', () => { } else { delete process.env.ZSH; } + if (originalZshCustom !== undefined) { + process.env.ZSH_CUSTOM = originalZshCustom; + } else { + delete process.env.ZSH_CUSTOM; + } // Clean up test directory await fs.rm(testHomeDir, { recursive: true, force: true }); @@ -83,6 +92,30 @@ describe('ZshInstaller', () => { expect(result.isOhMyZsh).toBe(false); expect(result.path).toBe(path.join(testHomeDir, '.zsh', 'completions', '_openspec')); }); + + it('should honor $ZSH for an Oh My Zsh install at a custom location', async () => { + // A relocated OMZ exports $ZSH; writing under ~/.oh-my-zsh instead + // would create a tree that no shell ever loads. + const customRoot = path.join(testHomeDir, 'dotfiles', 'omz'); + process.env.ZSH = customRoot; + + const result = await installer.getInstallationPath(); + + expect(result.isOhMyZsh).toBe(true); + expect(result.path).toBe(path.join(customRoot, 'custom', 'completions', '_openspec')); + }); + + it('should honor $ZSH_CUSTOM over the derived custom dir', async () => { + process.env.ZSH = path.join(testHomeDir, 'dotfiles', 'omz'); + process.env.ZSH_CUSTOM = path.join(testHomeDir, 'dotfiles', 'omz-custom'); + + const result = await installer.getInstallationPath(); + + expect(result.isOhMyZsh).toBe(true); + expect(result.path).toBe( + path.join(testHomeDir, 'dotfiles', 'omz-custom', 'completions', '_openspec') + ); + }); }); describe('backupExistingFile', () => { diff --git a/test/core/parsers/requirement-blocks.test.ts b/test/core/parsers/requirement-blocks.test.ts index 798d70c596..d0f9712cfe 100644 --- a/test/core/parsers/requirement-blocks.test.ts +++ b/test/core/parsers/requirement-blocks.test.ts @@ -37,6 +37,17 @@ describe('extractRequirementsSection', () => { }); describe('parseDeltaSpec', () => { + it('strips a UTF-8 BOM so a delta section on the first line still parses', () => { + // Windows editors and PowerShell redirects prepend a BOM; without + // stripping it the first line never matches "## ADDED Requirements" and + // validate reports "No delta sections found" for a well-formed file. + const content = `## ADDED Requirements\n### Requirement: BOM survivor\nThe system SHALL parse.\n\n#### Scenario: Parses\n- **WHEN** a BOM prefixes the file\n- **THEN** the delta is found\n`; + const result = parseDeltaSpec(content); + expect(result.sectionPresence.added).toBe(true); + expect(result.added.length).toBe(1); + expect(result.added[0].name).toBe('BOM survivor'); + }); + it('regression: parses ###Requirement: header with no space in delta ADDED section', () => { const content = `## ADDED Requirements\n###Requirement: NoSpace\nThe system SHALL foo.\n`; const result = parseDeltaSpec(content); diff --git a/test/core/update.test.ts b/test/core/update.test.ts index c6f6a6a3ea..52a669c094 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -2414,11 +2414,19 @@ More user content after markers. await fs.mkdir(path.join(skillsDir, 'openspec-explore'), { recursive: true }); await fs.writeFile(path.join(skillsDir, 'openspec-explore', 'SKILL.md'), 'old'); + const consoleSpy = vi.spyOn(console, 'log'); await expect(updateCommand.execute(testDir)).resolves.toBeUndefined(); expect(await FileSystemUtils.fileExists( path.join(skillsDir, 'openspec-explore', 'SKILL.md') )).toBe(false); + + // The tool now has zero OpenSpec artifacts; the removal must not be + // silent — update prints the same configuration correction init does. + const logCalls = consoleSpy.mock.calls.flat().map(String); + const correction = logCalls.find((entry) => entry.includes('No skills or commands remain')); + expect(correction).toBeTruthy(); + expect(correction).toContain("openspec config set delivery both"); }); it('should apply config sync when templates are up to date', async () => { diff --git a/test/core/version-check.test.ts b/test/core/version-check.test.ts index 00471d70b1..3f3231649e 100644 --- a/test/core/version-check.test.ts +++ b/test/core/version-check.test.ts @@ -215,6 +215,35 @@ describe('getAvailableCliUpdate', () => { await expect(getAvailableCliUpdate()).resolves.toBeNull(); }); + it('tears down a redirected connection when the overall budget expires', async () => { + // The redirect target trickles bytes forever: steady data keeps resetting + // the per-request idle timeout, so only the overall budget timer can end + // the exchange — and it must destroy the redirected request, not the + // already-dead first hop, or the socket outlives the check. + let hop = 0; + let trickleClosed = false; + respond = (res) => { + hop += 1; + if (hop === 1) { + res.writeHead(302, { location: '/mirror/@fission-ai/openspec/latest' }); + res.end(); + return; + } + res.writeHead(200, { 'content-type': 'application/json' }); + res.write('{"ver'); + const trickle = setInterval(() => res.write('x'), 200); + res.on('close', () => { + trickleClosed = true; + clearInterval(trickle); + }); + }; + + const startedAt = Date.now(); + await expect(getAvailableCliUpdate()).resolves.toBeNull(); + expect(Date.now() - startedAt).toBeLessThan(5000); + await vi.waitFor(() => expect(trickleClosed).toBe(true), { timeout: 2000 }); + }, 10000); + it('gives up rather than hanging when the registry stalls mid-response', async () => { respond = (res) => { res.writeHead(200, { 'content-type': 'application/json' }); @@ -411,7 +440,13 @@ describe('offerCliUpgrade', () => { ? path.join(prefix, 'node_modules', '@fission-ai', 'openspec') : path.join(prefix, 'lib', 'node_modules', '@fission-ai', 'openspec'); fs.mkdirSync(installed, { recursive: true }); - fs.mkdirSync(path.join(prefix, 'bin'), { recursive: true }); + if (isWindows) { + // npm writes the .cmd shim beside node_modules; it is what separates + // a real prefix from a hand-copied portable tree. + fs.writeFileSync(path.join(prefix, 'openspec.cmd'), '@echo off\n'); + } else { + fs.mkdirSync(path.join(prefix, 'bin'), { recursive: true }); + } expect(npmPrefixFromInstallDir(installed)).toBe(prefix); // Deliberately an unrelated root, standing in for the Cellar path. @@ -421,6 +456,21 @@ describe('offerCliUpgrade', () => { expect(npmPrefixFromInstallDir(path.join(HOME_ROOT, 'not', 'an', 'install'))).toBeNull(); expect(npmPrefixFromInstallDir(null)).toBeNull(); + + // The same shape with nothing npm wrote (no bin dir, no .cmd shim) is a + // hand-copied portable tree, not an npm install — no upgrade offer. + const portable = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-portable-')); + try { + const copied = isWindows + ? path.join(portable, 'node_modules', '@fission-ai', 'openspec') + : path.join(portable, 'lib', 'node_modules', '@fission-ai', 'openspec'); + fs.mkdirSync(copied, { recursive: true }); + expect( + isNpmGlobalInstall(copied, [path.join(GLOBAL_ROOT, 'lib', 'node_modules')]) + ).toBe(false); + } finally { + fs.rmSync(portable, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } } finally { fs.rmSync(prefix, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); } @@ -491,6 +541,16 @@ describe('offerCliUpgrade', () => { expect(detectPackageManager(null)).toBe('npm'); }); + it('does not let a user or project directory named after a manager steal the install', () => { + // A person named volta with a plain npm prefix in their home directory: + // the undotted segment alone must not turn the hint into `volta install`. + expect(detectPackageManager('/home/volta/.npm-global/lib/node_modules/pkg')).toBe('npm'); + expect(detectPackageManager('/srv/volta/apps/node_modules/pkg')).toBe('npm'); + // Even alongside a generic "tools" dir — only volta's full tools/image + // layout counts. + expect(detectPackageManager('/srv/volta/tools/apps/node_modules/pkg')).toBe('npm'); + }); + it('recognizes the Windows spellings of those install directories', () => { // %LOCALAPPDATA%\Volta, \Yarn\Data, \pnpm-cache — capitalized, undotted, // and nothing like their POSIX equivalents. diff --git a/test/ui/welcome-screen.test.ts b/test/ui/welcome-screen.test.ts index 0a4ce5f134..263bb6eea4 100644 --- a/test/ui/welcome-screen.test.ts +++ b/test/ui/welcome-screen.test.ts @@ -184,9 +184,12 @@ describe('welcome screen', () => { await showWelcomeScreen(CORE_WORKFLOWS); - expect(useKeypressMock).not.toHaveBeenCalled(); + // Static rendering still waits for the Enter the prompt line asks for; + // otherwise the keystroke falls through into the tool picker (#1462). + expect(useKeypressMock).toHaveBeenCalledOnce(); const output = writtenOutput(); expect(output).toContain('Welcome to OpenSpec'); + expect(output).toContain('Press Enter'); // No cursor-up repaints: the frame is drawn exactly once. expect(output).not.toMatch(/\x1b\[\d+A/); }); @@ -197,7 +200,7 @@ describe('welcome screen', () => { await showWelcomeScreen(CORE_WORKFLOWS); - expect(useKeypressMock).not.toHaveBeenCalled(); + expect(useKeypressMock).toHaveBeenCalledOnce(); expect(writtenOutput()).not.toMatch(/\x1b\[\d+A/); }); @@ -206,7 +209,7 @@ describe('welcome screen', () => { await showWelcomeScreen(CORE_WORKFLOWS, { animate: false }); - expect(useKeypressMock).not.toHaveBeenCalled(); + expect(useKeypressMock).toHaveBeenCalledOnce(); const output = writtenOutput(); expect(output).toContain('Welcome to OpenSpec'); expect(output).not.toMatch(/\x1b\[\d+A/); @@ -222,7 +225,7 @@ describe('welcome screen', () => { await showWelcomeScreen(CORE_WORKFLOWS); - expect(useKeypressMock).not.toHaveBeenCalled(); + expect(useKeypressMock).toHaveBeenCalledOnce(); expect(writtenOutput()).toContain('Welcome to OpenSpec'); } ); diff --git a/test/utils/change-utils.test.ts b/test/utils/change-utils.test.ts index d07edc396d..4f32914aa6 100644 --- a/test/utils/change-utils.test.ts +++ b/test/utils/change-utils.test.ts @@ -11,6 +11,15 @@ describe('validateChangeName', () => { expect(result).toEqual({ valid: true }); }); + it('should accept a long-but-bounded name and reject one past the cap', () => { + // Past the cap the failure must be a validation message, not a raw + // ENAMETOOLONG once mkdir hits the 255-byte component limit. + expect(validateChangeName('a'.repeat(200))).toEqual({ valid: true }); + const result = validateChangeName('a'.repeat(201)); + expect(result.valid).toBe(false); + expect(result.error).toContain('too long'); + }); + it('should accept name with multiple segments', () => { const result = validateChangeName('add-user-auth'); expect(result).toEqual({ valid: true });