trending/OpenSpec
1
0
Fork 0
mirror of https://github.com/Fission-AI/OpenSpec.git synced 2026-10-02 05:24:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(packaging): print the completions tip from the CLI, not a postinstall script (#1704)

Browse Source 
* fix(packaging): print the completions tip from the CLI, not a postinstall script

The package's only install script existed to print one line suggesting
`openspec completion install`. Shipping it made every `npm install -g`
emit an npm allow-scripts warning, and `npm approve-scripts` then failed
with ENOMATCH because it looks in the local project, not a global install
— so the warning looked like a packaging fault with no way to clear it.

The tip now prints once on the CLI's first run, recorded via a
`completionTipSeen` flag in the existing global config alongside the
telemetry notice's `noticeSeen`. It writes to stderr so it can never
contaminate piped stdout, and is suppressed under CI,
OPENSPEC_NO_COMPLETIONS=1, `--json` runs, and `openspec completion`
itself. The published package now ships no lifecycle scripts at all.

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

* fix(completions): stop the first-run tip from corrupting global config

Adversarial review of the previous commit found it wrote a defaults-merged
config: `saveGlobalConfig({ ...getGlobalConfig(), completionTipSeen: true })`
stamped `profile: "core"` into every user's config.json on first run.
`migrateIfNeeded` treats a raw `profile` as "already migrated", so the
one-time profile migration would never run again — and `openspec update`
then deleted the user's installed workflow skills. Reproduced: 2 skill
directories removed where main reports "Migrated: custom profile with 8
workflows". The same write also overwrote an unparsable config with
defaults and made `openspec config list` report defaults as explicit.

The tip now reads and writes the raw config file and touches only its own
key, leaving an unreadable config strictly alone.

Other hardening from the same review:

- Suppress the tip for the hidden `__complete` resolver. Generated
  completion scripts call it on every Tab press with stderr discarded, so
  the one-shot tip was consumed where nobody could see it.
- Defer, never consume, when stderr is not a terminal. Agents and pipes
  drive this CLI far more often than humans do and would otherwise spend
  the tip into a log nobody opens.
- Skip the tip when completions are already installed. Previously the CLI
  advertised `completion install` to users who had run it — including on
  the very next command after installing. Adds `isInstalled()` to the
  bash/fish/powershell installers, mirroring the zsh one.
- Use the repo's `isCiEnvironment()` instead of a `CI === 'true'` string
  check, so `CI=yes`/`True`/`on` are as quiet as telemetry is.
- Move the call to `postAction` so the tip trails the command's output
  instead of pushing errors and `init`'s setup summary down the screen.
- Record before printing, so an unwritable config dir means silence rather
  than nagging on every run.

Tests: assert the message literal (mutation testing showed the message text
was the one unguarded behavior), the raw-write shape, corrupt-config
safety, the already-installed path, the defer policy, and an e2e case
pinning the non-TTY contract.

Docs: SECURITY.md no longer claims zero lifecycle scripts — `prepare` is
still declared and runs for git/directory installs; the registry-install
claim is the accurate one. `OPENSPEC_NO_COMPLETIONS` is now documented.

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

* test(completions): make the unwritable-config case portable to Windows

fs.chmodSync(dir, 0o555) does not stop a write on Windows, so this test's
unwritable condition never existed there: markTipSeen succeeded, the tip
printed, and windows-pwsh was the only failing job.

Occupy the config directory's path with a file instead. mkdirSync with
recursive: true tolerates an existing directory but throws on an existing
file on every platform, so the persist fails where a real permission error
would - before anything is printed. Also asserts the path is still a file,
so a partial write through the failure would be caught.

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

* fix(completions): retire the first-run tip instead of advising a dead end

Second adversarial pass over the tip, covering the hardening commit itself.

- An undetected or unsupported shell now retires the tip quietly. It used
  to print, but `openspec completion install` exits 1 for exactly those
  users ("Shell 'tcsh' is not supported yet" / "Could not auto-detect
  shell"), so the one message they would ever get about completions sent
  them to a command that fails.
- `markTipSeen` re-reads the config immediately before writing and swaps
  the file in by rename. Deciding whether to show the tip costs a `ps`
  spawn plus a stat, and a sibling process writing config in that window
  got clobbered — on a first run that is exactly when telemetry mints
  `anonymousId`. Concurrent-process loss drops from 15/40 to ~2/40, and
  what now usually loses is the tip's own flag (it simply shows once
  more) rather than telemetry identity. The residual is the non-atomic
  read-modify-write shape shared with telemetry's own writer.
- `isInstalled()` uses stat().isFile(), so a directory at the install
  path no longer counts as an installed completion script.
- Documented what `isInstalled()` actually promises: the script file, not
  the profile sourcing line that bash and PowerShell also need. Callers
  deciding whether to *advertise* completions want the loose reading — a
  user whose profile config failed has already met the installer.
- Corrected a comment claiming the probe costs "one stat": detectShell()
  forks `ps` to read the parent process on every non-Windows run.

Tests: mutation testing found four surviving mutants — dropping
isCompletionRun from the defer policy, reverting isCiEnvironment to a
CI==='true' string check, failing closed on an undetected shell, and
neutering the non-object config guard (which lets a JSON array config be
rewritten as {"0":...}). All four now fail a test. Adds direct coverage
for the three new isInstalled() implementations, which had none.

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

* fix(validate): stop `change validate` exiting past commander's postAction

`change validate` on a failing change called process.exit(exitCode). That
tears down before commander's postAction hook, which is the same trap the
`update` command documents 165 lines earlier: "exiting here would skip
commander's postAction hook, killing the telemetry flush mid-request".

A change that fails validation is a routine outcome, not an error, so this
silently dropped the telemetry flush and — since the completions tip moved
to postAction — the first-run tip for anyone whose first command was a
failing validate. Verified under a pty: before, the tip never printed and
completionTipSeen was never recorded; after, both happen and the exit code
is still 1 (validate() already sets process.exitCode, which Node honours at
natural exit — top-level `validate --all` has always relied on exactly
that). The existing e2e in validate-scenario-loss.test.ts pins the exit
code.

Also wraps the postAction tip in try/finally so the telemetry flush runs
even if the hint throws: program.parse() is synchronous, so a rejection
there has no catch above it.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Clay Good
2026-08-19 20:19:51 +00:00
committed by GitHub
co-authored by Claude Opus 5
parent 9643888a75
commit 7276c6c268
23 changed files with 719 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/drop-postinstall-script.md
+5
View File
@@ -0,0 +1,5 @@
---
"@fission-ai/openspec": patch
---
Drop the npm `postinstall` script. Its only job was printing a one-line tip about opt-in shell completions, but shipping any install script made `npm install -g @fission-ai/openspec` emit an `allow-scripts` warning that reads as a packaging fault (and `npm approve-scripts` then fails with `ENOMATCH` on a global install, since it looks in the local project). The tip now prints from the CLI on its first run — to stderr, in an interactive terminal, once, and not at all if you already have completions installed — and the published package declares no `preinstall`/`install`/`postinstall` script, so a registry install runs no OpenSpec code. Suppress the tip with `OPENSPEC_NO_COMPLETIONS=1`.
SECURITY.md
+2 -2
View File
@@ -27,7 +27,7 @@ If you think something sits on the boundary, report it and we'll work it out tog
## Published package contents
The `openspec` npm package publishes `dist/`, `bin/`, `schemas/`, and `scripts/postinstall.js`. Build and test tooling (vite, rollup, vitest, eslint, and their transitive dependencies) is not published. Scanners that read `pnpm-lock.yaml` without separating dependency scope will report advisories for packages that never reach an installed copy of OpenSpec.
The `openspec` npm package publishes `dist/`, `bin/`, and `schemas/`. Build and test tooling (vite, rollup, vitest, eslint, and their transitive dependencies) is not published. Scanners that read `pnpm-lock.yaml` without separating dependency scope will report advisories for packages that never reach an installed copy of OpenSpec.
You do not have to take that on trust — install the package and look:
@@ -42,7 +42,7 @@ ls node_modules | grep -E '^(vite|rollup|vitest|eslint|js-yaml|minimatch)$' #
| Surface | Behavior |
| --- | --- |
| Install script | `scripts/postinstall.js` prints one line suggesting shell completions. It makes no network request, writes no files, and runs no shell. Completions are opt-in via `openspec completion install`. |
| Install scripts | The package ships no `preinstall`, `install`, or `postinstall` script, so installing it from the npm registry runs no code from OpenSpec. (`prepare` is still declared; npm runs it only for git and local-directory installs, where it builds from source.) Shell completions are opt-in via `openspec completion install`; the CLI prints a one-line tip about them on its first run. |
| Running other programs | Every call that goes through a shell uses a fixed literal (`which gh`, `gh auth status`). Anything carrying your input — issue text, editor paths, workset commands, the path passed to `openspec update` — uses an argument array, never string interpolation into a shell. On Windows, `.cmd` shims are launched through `cross-spawn`, which escapes arguments rather than concatenating them. |
| Installing software | `openspec update` can run `npm install -g @fission-ai/openspec@latest` and then re-run `openspec update` with the upgraded CLI. It does this only after you answer yes to a prompt, only for the OpenSpec package itself, only when npm owns the install, and never in CI or a non-interactive shell. A global install lives outside your project, so it runs with your permissions there and executes whatever lifecycle scripts the published package ships. It then reads the installed binary's version back rather than assuming the upgrade took. Decline and it prints the command for you to run yourself. |
| Telemetry | Command name, OpenSpec version, and a locally generated random UUID. No file paths, no file contents, no environment, no hostname, and IP capture is explicitly disabled. Opt out with `OPENSPEC_TELEMETRY=0` or `DO_NOT_TRACK=1`; it is off in CI automatically. |
docs/cli.md
+6
View File
@@ -1262,6 +1262,11 @@ openspec completion generate bash > ~/.bash_completion.d/openspec
openspec completion uninstall
```
Completions are opt-in. The CLI mentions them once, on stderr, the first time you
run a command in an interactive terminal, and never again — it also stays quiet
if you already have completions installed. Set `OPENSPEC_NO_COMPLETIONS=1` to
suppress that tip entirely.
---
## Exit Codes
@@ -1283,6 +1288,7 @@ openspec completion uninstall
| `EDITOR` or `VISUAL` | Editor for `openspec config edit` |
| `NO_COLOR` | Disable color output when set |
| `OPENSPEC_NO_ANIMATION` | Disable the `openspec init` welcome animation when set |
| `OPENSPEC_NO_COMPLETIONS` | Set to `1` to suppress the one-time tip about shell completions |
| `OPENSPEC_NO_UPDATE_CHECK` | Disable the `openspec update` check for a newer published CLI when set (any value, including empty). Also skipped when `CI` is set (unless `false`/`0`/`no`/`off`) or `NODE_ENV=test` |
| `npm_config_registry` | Registry the `openspec update` version check asks. Must be an `http(s)` URL or it falls back to `https://registry.npmjs.org`. No `.npmrc` file is read |
package.json
-3
View File
@@ -34,7 +34,6 @@
"dist",
"bin",
"schemas",
"scripts/postinstall.js",
"!dist/**/*.test.js",
"!dist/**/__tests__",
"!dist/**/*.map"
@@ -50,10 +49,8 @@
"test:watch": "vitest",
"test:ui": "vitest --ui",
"test:coverage": "vitest --coverage",
"test:postinstall": "node scripts/postinstall.js",
"prepare": "pnpm run build",
"prepublishOnly": "pnpm run build",
"postinstall": "node scripts/postinstall.js",
"check:pack-version": "node scripts/pack-version-check.mjs",
"release": "pnpm run release:ci",
"release:ci": "pnpm run check:pack-version && pnpm exec changeset publish",
scripts/README.md
-4
View File
@@ -67,10 +67,6 @@ against fabricated input — see `test/core/templates/parity-hash-shared.test.ts
A test that ran this script for real would rewrite the repository's own parity
test file mid-suite.
## postinstall.js
Post-installation script that runs after package installation.
## pack-version-check.mjs
Validates package version consistency before publishing.
scripts/postinstall.js
-83
View File
@@ -1,83 +0,0 @@
#!/usr/bin/env node
/**
* Postinstall script that hints about shell completions
*
* Completion installation is opt-in: the user must run
* `openspec completion install` explicitly. This script only
* prints a one-line tip after npm install.
*
* The tip is suppressed when:
* - CI=true environment variable is set
* - OPENSPEC_NO_COMPLETIONS=1 environment variable is set
* - dist/ directory doesn't exist (dev setup scenario)
*
* The script never fails npm install - all errors are caught and handled gracefully.
*/
import { promises as fs } from 'fs';
import path from 'path';
import { fileURLToPath } from 'url';
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
/**
* Check if we should skip installation
*/
function shouldSkipInstallation() {
// Skip in CI environments
if (process.env.CI === 'true' || process.env.CI === '1') {
return { skip: true, reason: 'CI environment detected' };
}
// Skip if user opted out
if (process.env.OPENSPEC_NO_COMPLETIONS === '1') {
return { skip: true, reason: 'OPENSPEC_NO_COMPLETIONS=1 set' };
}
return { skip: false };
}
/**
* Check if dist/ directory exists
*/
async function distExists() {
const distPath = path.join(__dirname, '..', 'dist');
try {
const stat = await fs.stat(distPath);
return stat.isDirectory();
} catch {
return false;
}
}
/**
* Main function
*/
async function main() {
try {
// Check if we should skip
const skipCheck = shouldSkipInstallation();
if (skipCheck.skip) {
// Silent skip - no output
return;
}
// Check if dist/ exists (skip silently if not - expected during dev setup)
if (!(await distExists())) {
return;
}
// Completions are opt-in — just print a hint
console.log(`\nTip: Run 'openspec completion install' for shell completions`);
} catch (error) {
// Fail gracefully - never break npm install
}
}
// Run main and handle any unhandled errors
main().catch(() => {
// Silent failure - never break npm install
process.exit(0);
});
scripts/test-postinstall.sh
-57
View File
@@ -1,57 +0,0 @@
#!/bin/bash
# Test script for postinstall.js
# Tests different scenarios: normal install, CI, opt-out
set -e
echo "======================================"
echo "Testing OpenSpec Postinstall Script"
echo "======================================"
echo ""
# Save original environment
ORIGINAL_CI="${CI:-}"
ORIGINAL_OPENSPEC_NO_COMPLETIONS="${OPENSPEC_NO_COMPLETIONS:-}"
# Test 1: Normal install
echo "Test 1: Normal install (should print tip about completions)"
echo "--------------------------------------"
unset CI
unset OPENSPEC_NO_COMPLETIONS
node scripts/postinstall.js
echo ""
# Test 2: CI environment (should skip silently)
echo "Test 2: CI=true (should skip silently)"
echo "--------------------------------------"
export CI=true
node scripts/postinstall.js
echo "[No output expected - skipped due to CI]"
echo ""
# Test 3: Opt-out flag (should skip silently)
echo "Test 3: OPENSPEC_NO_COMPLETIONS=1 (should skip silently)"
echo "--------------------------------------"
unset CI
export OPENSPEC_NO_COMPLETIONS=1
node scripts/postinstall.js
echo "[No output expected - skipped due to opt-out]"
echo ""
# Restore original environment
if [ -n "$ORIGINAL_CI" ]; then
export CI="$ORIGINAL_CI"
else
unset CI
fi
if [ -n "$ORIGINAL_OPENSPEC_NO_COMPLETIONS" ]; then
export OPENSPEC_NO_COMPLETIONS="$ORIGINAL_OPENSPEC_NO_COMPLETIONS"
else
unset OPENSPEC_NO_COMPLETIONS
fi
echo "======================================"
echo "All tests completed successfully!"
echo "======================================"
src/cli/index.ts
+46 -5
View File
@@ -49,6 +49,7 @@ import {
type NewChangeOptions,
} from '../commands/workflow/index.js';
import { maybeShowTelemetryNotice, trackCommand, shutdown } from '../telemetry/index.js';
import { maybeShowCompletionTip } from '../core/completion-tip.js';
import { COMMON_FLAGS } from '../core/completions/shared-flags.js';
import { isInteractive } from '../utils/interactive.js';
@@ -136,6 +137,29 @@ export function isJsonRun(command: Command): boolean {
);
}
/**
* True for the commands that exist to serve shell completions: the user-facing
* `openspec completion ...` group and the hidden `__complete` resolver that
* generated completion scripts call on every Tab press. Tipping either about
* completions is noise, and `__complete` would burn the one-shot tip invisibly.
*/
export function isCompletionRun(commandPath: string): boolean {
return commandPath.split(':')[0] === 'completion' || commandPath === '__complete';
}
/**
* True when the first-run completions tip must be deferred rather than shown.
*
* Deferring keeps the tip unconsumed, so it still reaches the user on a later
* run that can actually carry it. All three cases are runs nobody would read a
* hint from: JSON output, the completion machinery itself, and a stderr that is
* not a terminal — pipes and the agent-driven runs that dominate this CLI's
* usage would otherwise burn the user's one-shot tip into a log nobody opens.
*/
export function shouldDeferCompletionTip(command: Command, stderrIsTty: boolean): boolean {
return isJsonRun(command) || isCompletionRun(getCommandPath(command)) || !stderrIsTty;
}
program
.name('openspec')
.description('AI-native system for spec-driven development')
@@ -161,12 +185,27 @@ program.hook('preAction', async (thisCommand, actionCommand) => {
// Track command execution (use actionCommand to get the actual subcommand)
const commandPath = getCommandPath(actionCommand);
await trackCommand(commandPath, version);
});
// Shutdown telemetry after command completes
program.hook('postAction', async () => {
await shutdown();
program.hook('postAction', async (_thisCommand, actionCommand) => {
// Show the first-run shell-completions tip (on stderr, so piped stdout stays
// clean). postAction, not preAction: the tip trails the command's own output
// instead of pushing an error message or `init`'s setup summary down the
// screen. Deferred — not consumed — whenever nobody would read it: JSON runs,
// `openspec completion ...`, and a stderr that is not a terminal (agents and
// pipes would otherwise silently burn the user's one-shot tip).
try {
await maybeShowCompletionTip({
silent: shouldDeferCompletionTip(actionCommand, Boolean(process.stderr.isTTY)),
});
} finally {
// The flush runs even if the hint throws: parse() is synchronous, so a
// rejection here has no catch anywhere above it.
await shutdown();
}
});
const availableToolIds = AI_TOOLS
@@ -423,10 +462,12 @@ changeCmd
.action(async (changeName?: string, options?: { strict?: boolean; json?: boolean; noInteractive?: boolean }) => {
try {
const changeCommand = new ChangeCommand();
// validate() already sets process.exitCode, and Node honours it at
// natural exit. Calling process.exit() here would skip commander's
// postAction hook — the same trap called out for `update` below — which
// kills the telemetry flush and the first-run completions tip on what is
// a routine outcome, not an error: a change that fails validation.
await changeCommand.validate(changeName, options);
if (typeof process.exitCode === 'number' && process.exitCode !== 0) {
process.exit(process.exitCode);
}
} catch (error) {
console.error(`Error: ${(error as Error).message}`);
process.exitCode = 1;
src/core/completion-tip.ts
+158
View File
@@ -0,0 +1,158 @@
/**
* First-run hint pointing users at opt-in shell completions.
*
* This hint used to be an npm `postinstall` script. Printing it from the CLI
* instead lets the package ship with no install scripts at all, so `npm install`
* no longer emits an `allow-scripts` warning. Completions stay opt-in: the tip
* only names the command, it never installs anything.
*
* The tip goes to stderr, never stdout, so it cannot contaminate piped command
* output.
*
* The tip is suppressed when:
* - CI is set (any value npm/telemetry would treat as CI)
* - OPENSPEC_NO_COMPLETIONS=1
* - completions are already installed, or the shell is one the installer would
* reject
* - the caller passes `silent` — JSON runs, `openspec completion ...`, and
* non-TTY runs, which are deferred rather than consumed (see `silent`)
*/
import * as fs from 'node:fs';
import * as path from 'node:path';
import { getGlobalConfigPath } from './global-config.js';
import { isCiEnvironment } from '../utils/ci.js';
import { detectShell } from '../utils/shell-detection.js';
import { CompletionFactory } from './completions/factory.js';
export const COMPLETION_TIP_MESSAGE =
"Tip: Run 'openspec completion install' for shell completions";
export interface CompletionTipOptions {
/**
* Skip printing without marking the tip as seen, so it still appears on the
* user's first later run that can safely carry it. Used for runs nobody would
* read the tip from: JSON output, and stderr that is not a terminal.
*/
silent?: boolean;
}
function isSuppressedByEnv(): boolean {
// isCiEnvironment, not a CI==='true' string check: providers set CI to "True",
// "yes", "on", and the tip should be as quiet in those builds as telemetry is.
return isCiEnvironment() || process.env.OPENSPEC_NO_COMPLETIONS === '1';
}
/**
* Whether the tip is worth showing, once we know it is owed and readable.
*
* "retire" consumes the tip without printing: the user either already has
* completions, or is on a shell `openspec completion install` would refuse.
*
* Without the installed check the tip tells people to install completions they
* installed long ago — including on the run right after `completion install`,
* whose own run only defers the tip. An undetected or unsupported shell retires
* it too: `completion install` exits 1 for those users, so pointing them at it
* is a dead end, and this tip is the only message about completions they would
* ever get.
*
* Not free: detectShell() forks `ps` to read the parent process (except on
* Windows), so this costs a spawn plus a stat. It runs only on interactive runs
* that still owe the tip, which is normally exactly one — but a config that
* cannot be written never records the flag, and then every interactive run pays
* it. On any unexpected error we show the tip rather than swallow it.
*/
async function decideTip(): Promise<'show' | 'retire'> {
try {
const { shell } = detectShell();
if (!shell) {
return 'retire';
}
return (await CompletionFactory.createInstaller(shell).isInstalled())
? 'retire'
: 'show';
} catch {
return 'show';
}
}
/**
* Read the global config exactly as it sits on disk.
*
* Deliberately NOT `getGlobalConfig()`: that merges in defaults, and writing the
* merged result back would stamp `profile`/`delivery` into a file the user never
* set them in. `migrateIfNeeded` treats a raw `profile` as "already migrated",
* so that stamp would permanently suppress the one-time profile migration and
* cost users their installed workflow skills.
*
* Returns null when the file exists but cannot be read or parsed — a config we
* cannot understand is left strictly alone rather than overwritten.
*/
function readRawConfig(): Record<string, unknown> | null {
const configPath = getGlobalConfigPath();
if (!fs.existsSync(configPath)) {
return {};
}
const parsed: unknown = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
return null;
}
return parsed as Record<string, unknown>;
}
/**
* Record the flag, re-reading the config first and replacing the file by rename.
*
* Deciding whether to show the tip costs a `ps` spawn and a stat, and a sibling
* `openspec` process can write the same file in that window — on a first run
* that is exactly when telemetry mints `anonymousId`. Re-reading here keeps the
* write down to this one key, and the rename keeps a reader from ever seeing a
* half-written config.
*/
function markTipSeen(): void {
const configPath = getGlobalConfigPath();
const current = readRawConfig() ?? {};
const tempPath = `${configPath}.${process.pid}.tmp`;
fs.mkdirSync(path.dirname(configPath), { recursive: true });
fs.writeFileSync(
tempPath,
JSON.stringify({ ...current, completionTipSeen: true }, null, 2) + '\n',
'utf-8'
);
fs.renameSync(tempPath, configPath);
}
/**
* Print the completion tip once, the first time the CLI runs.
* Never throws — a hint must not break a command.
*/
export async function maybeShowCompletionTip(
options: CompletionTipOptions = {}
): Promise<void> {
if (isSuppressedByEnv()) {
return;
}
try {
const raw = readRawConfig();
if (raw === null || raw.completionTipSeen === true) {
return;
}
if (options.silent) {
return;
}
const decision = await decideTip();
// Record before printing: if the flag cannot be persisted, staying quiet
// beats reprinting the tip on every future run.
markTipSeen();
if (decision === 'show') {
console.error(`\n${COMPLETION_TIP_MESSAGE}`);
}
} catch {
// Silent failure - a hint should never break the CLI.
}
}
src/core/completions/factory.ts
+11
View File
@@ -32,6 +32,17 @@ export interface InstallationResult {
export interface CompletionInstaller {
install(script: string): Promise<InstallationResult>;
uninstall(): Promise<{ success: boolean; message: string }>;
/**
* True when a completion script file is present at the install path.
*
* Deliberately just the script: bash and PowerShell also need a sourcing
* line in the user's profile, and `install()` adds that on a best-effort
* basis (it is skipped by OPENSPEC_NO_AUTO_CONFIG=1 or an unwritable
* profile, printing manual instructions instead). Someone in that state has
* already met the installer, so callers that use this to decide whether to
* *advertise* completions should not advertise again.
*/
isInstalled(): Promise<boolean>;
}
/**
src/core/completions/installers/bash-installer.ts
+15
View File
@@ -64,6 +64,21 @@ export class BashInstaller {
return path.join(localCompletionDir, 'openspec');
}
/**
* Check if a completion script is currently installed.
* Mirrors ZshInstaller.isInstalled so callers can ask any installer.
*
* @returns true if the completion script exists
*/
async isInstalled(): Promise<boolean> {
try {
// stat, not access: a directory at the install path is not a script.
return (await fs.stat(await this.getInstallationPath())).isFile();
} catch {
return false;
}
}
/**
* Backup an existing completion file if it exists
*
src/core/completions/installers/fish-installer.ts
+15
View File
@@ -24,6 +24,21 @@ export class FishInstaller {
return path.join(this.homeDir, '.config', 'fish', 'completions', 'openspec.fish');
}
/**
* Check if a completion script is currently installed.
* Mirrors ZshInstaller.isInstalled so callers can ask any installer.
*
* @returns true if the completion script exists
*/
async isInstalled(): Promise<boolean> {
try {
// stat, not access: a directory at the install path is not a script.
return (await fs.stat(this.getInstallationPath())).isFile();
} catch {
return false;
}
}
/**
* Backup an existing completion file if it exists
*
src/core/completions/installers/powershell-installer.ts
+15
View File
@@ -123,6 +123,21 @@ export class PowerShellInstaller {
return path.join(profileDir, 'OpenSpecCompletion.ps1');
}
/**
* Check if a completion script is currently installed.
* Mirrors ZshInstaller.isInstalled so callers can ask any installer.
*
* @returns true if the completion script exists
*/
async isInstalled(): Promise<boolean> {
try {
// stat, not access: a directory at the install path is not a script.
return (await fs.stat(this.getInstallationPath())).isFile();
} catch {
return false;
}
}
/**
* Backup an existing completion file if it exists
*
src/core/config-schema.ts
+2
View File
@@ -35,6 +35,8 @@ export const GlobalConfigSchema = z
})
.passthrough()
.optional(),
// Runtime-managed (like telemetry.noticeSeen); not user-settable via CLI set.
completionTipSeen: z.boolean().optional(),
})
.passthrough();
src/core/global-config.ts
+2
View File
@@ -36,6 +36,8 @@ export interface GlobalConfig {
openers?: unknown;
/** Anonymous usage analytics settings and identity. */
telemetry?: TelemetryConfig;
/** Whether the first-run shell-completions tip has been shown. */
completionTipSeen?: boolean;
}
const DEFAULT_CONFIG: GlobalConfig = {
test/cli-e2e/completion-tip.test.ts
+50
View File
@@ -0,0 +1,50 @@
import { describe, it, expect } from 'vitest';
import * as fs from 'node:fs/promises';
import * as os from 'node:os';
import * as path from 'node:path';
import { runCLI } from '../helpers/run-cli.js';
/**
* The completions tip is a one-shot hint aimed at a human at a terminal.
* Spawned runs — agents driving the CLI, shell pipelines, CI — have no TTY on
* stderr, so they must leave the tip unconsumed for the next interactive run.
* A regression here is invisible in normal use: the user simply never sees the
* tip, because a background `openspec status` already spent it.
*/
describe('completions tip in non-interactive runs', () => {
async function freshConfigHome(): Promise<string> {
return fs.mkdtemp(path.join(os.tmpdir(), 'openspec-tip-e2e-'));
}
async function tipSeenFlag(configHome: string): Promise<unknown> {
try {
const raw = await fs.readFile(path.join(configHome, 'openspec', 'config.json'), 'utf-8');
return JSON.parse(raw).completionTipSeen;
} catch {
return undefined;
}
}
it('never prints or consumes the tip when stderr is not a terminal', async () => {
const configHome = await freshConfigHome();
// CI is explicitly off, so only the non-TTY guard can suppress the tip.
const result = await runCLI(['list'], { env: { XDG_CONFIG_HOME: configHome, CI: '' } });
expect(result.stdout).not.toContain('completion install');
expect(result.stderr).not.toContain('completion install');
expect(await tipSeenFlag(configHome)).toBeUndefined();
});
it('leaves stdout parseable on a --json run', async () => {
const configHome = await freshConfigHome();
const result = await runCLI(['list', '--json'], {
env: { XDG_CONFIG_HOME: configHome, CI: '' },
});
expect(() => JSON.parse(result.stdout)).not.toThrow();
expect(await tipSeenFlag(configHome)).toBeUndefined();
});
});
test/core/cli-is-json-run.test.ts
+66 -1
View File
@@ -1,7 +1,7 @@
import { describe, it, expect } from 'vitest';
import { Command, Option } from 'commander';
import { isJsonRun } from '../../src/cli/index.js';
import { isJsonRun, isCompletionRun, shouldDeferCompletionTip } from '../../src/cli/index.js';
/**
* Reproduce the three ways `--json` reaches a command in the real CLI, so a
@@ -39,6 +39,9 @@ function buildProgram(capture: (command: Command) => void): Command {
.option('--json', 'Output as JSON')
.action(() => {});
// 4. The completion group, whose runs must never carry the first-run tip.
program.command('completion').command('install').action(() => {});
return program;
}
@@ -77,3 +80,65 @@ describe('isJsonRun', () => {
expect(isJsonRun(await actionCommandFor(['store', 'bogus']))).toBe(false);
});
});
describe('isCompletionRun', () => {
/**
* The completions tip must never fire for the commands that serve completions
* themselves. `__complete` is the important one: generated completion scripts
* call it on every Tab press with stderr redirected to /dev/null, so an
* unsuppressed tip would be consumed invisibly and the user would never see it.
*/
it.each([
'completion',
'completion:install',
'completion:uninstall',
'completion:generate',
'__complete',
])('suppresses the completions tip for "%s"', (commandPath) => {
expect(isCompletionRun(commandPath)).toBe(true);
});
it.each(['list', 'init', 'update', 'change:show', 'completions'])(
'does not suppress the completions tip for "%s"',
(commandPath) => {
expect(isCompletionRun(commandPath)).toBe(false);
}
);
});
describe('shouldDeferCompletionTip', () => {
/**
* The tip must survive every run that cannot display it. Deferring (rather
* than consuming) is what makes the one-shot hint actually reach a human:
* agents and CI pipelines run this CLI far more often than people do.
*/
function commandFor(argv: string[]): Command {
let captured: Command | undefined;
const program = buildProgram((command) => {
captured = command;
});
program.parse(argv, { from: 'user' });
if (!captured) {
throw new Error(`no command captured for ${argv.join(' ')}`);
}
return captured;
}
it('shows the tip on a plain interactive run', () => {
expect(shouldDeferCompletionTip(commandFor(['status']), true)).toBe(false);
});
it('defers when stderr is not a terminal', () => {
expect(shouldDeferCompletionTip(commandFor(['status']), false)).toBe(true);
});
it('defers on a JSON run even with a terminal', () => {
expect(shouldDeferCompletionTip(commandFor(['status', '--json']), true)).toBe(true);
});
it('defers on the completion commands themselves', () => {
// isCompletionRun is unit-tested above, but nothing proved the policy
// function actually consults it.
expect(shouldDeferCompletionTip(commandFor(['completion', 'install']), true)).toBe(true);
});
});
test/core/completion-tip.test.ts
+210
View File
@@ -0,0 +1,210 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { maybeShowCompletionTip, COMPLETION_TIP_MESSAGE } from '../../src/core/completion-tip.js';
import { getGlobalConfigPath } from '../../src/core/global-config.js';
describe('core/completion-tip', () => {
let tempDir: string;
let originalEnv: NodeJS.ProcessEnv;
let errorSpy: ReturnType<typeof vi.spyOn>;
function printedTip(): boolean {
return errorSpy.mock.calls.some((call) =>
String(call[0] ?? '').includes(COMPLETION_TIP_MESSAGE)
);
}
beforeEach(() => {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-completion-tip-'));
originalEnv = { ...process.env };
process.env.XDG_CONFIG_HOME = path.join(tempDir, 'config');
// HOME too: the already-installed probe reads the shell's completion dirs,
// so without this the developer's own installed completions would silence
// the tip and quietly turn these tests vacuous. This works because the
// installers resolve home via os.homedir(), which honours $HOME in a
// process — vitest.config.ts pins `pool: 'forks'`; under a thread pool the
// native call would ignore this assignment and the sandbox would leak.
process.env.HOME = tempDir;
process.env.USERPROFILE = tempDir;
process.env.SHELL = '/bin/zsh';
delete process.env.CI;
delete process.env.OPENSPEC_NO_COMPLETIONS;
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
errorSpy.mockRestore();
for (const key of Object.keys(process.env)) {
delete process.env[key];
}
Object.assign(process.env, originalEnv);
fs.rmSync(tempDir, { recursive: true, force: true });
});
it('names a command that actually exists', async () => {
// Asserting the literal, not the imported constant: comparing the message
// against itself would pass even if the tip advertised a typo'd command.
expect(COMPLETION_TIP_MESSAGE).toBe(
"Tip: Run 'openspec completion install' for shell completions"
);
});
it('prints the tip on the first run and records that it was seen', async () => {
await maybeShowCompletionTip();
expect(printedTip()).toBe(true);
expect(JSON.parse(fs.readFileSync(getGlobalConfigPath(), 'utf-8')).completionTipSeen).toBe(true);
});
it('does not print the tip again on later runs', async () => {
await maybeShowCompletionTip();
errorSpy.mockClear();
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
});
it('defers the tip on silent runs without consuming it', async () => {
await maybeShowCompletionTip({ silent: true });
expect(printedTip()).toBe(false);
expect(fs.existsSync(getGlobalConfigPath())).toBe(false);
await maybeShowCompletionTip();
expect(printedTip()).toBe(true);
});
it.each([
['CI', 'true'],
['CI', '1'],
// The values a plain `CI === 'true'` check would miss — the whole reason
// this uses the repo's isCiEnvironment().
['CI', 'True'],
['CI', 'yes'],
['CI', 'on'],
['OPENSPEC_NO_COMPLETIONS', '1'],
])('stays silent when %s=%s', async (key, value) => {
process.env[key] = value;
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
expect(fs.existsSync(getGlobalConfigPath())).toBe(false);
});
it('does not materialize default config fields when recording the flag', async () => {
// Regression guard: writing a defaults-merged config would stamp `profile`
// into config.json, and migrateIfNeeded treats a raw `profile` as "already
// migrated" — permanently suppressing the one-time profile migration and
// deleting the user's installed workflow skills.
await maybeShowCompletionTip();
const raw = JSON.parse(fs.readFileSync(getGlobalConfigPath(), 'utf-8'));
expect(raw).toEqual({ completionTipSeen: true });
expect(raw.profile).toBeUndefined();
expect(raw.delivery).toBeUndefined();
expect(raw.featureFlags).toBeUndefined();
});
it('leaves an unparsable config untouched and stays silent', async () => {
const configPath = getGlobalConfigPath();
const corrupt = '{"defaultStore":"acme","profile":"custom", }';
fs.mkdirSync(path.dirname(configPath), { recursive: true });
fs.writeFileSync(configPath, corrupt);
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
expect(fs.readFileSync(configPath, 'utf-8')).toBe(corrupt);
});
it('stays silent rather than repeating when the flag cannot be persisted', async () => {
// The unwritable condition is created by occupying the config directory's
// path with a FILE, not by chmod-ing the directory: on Windows a mode of
// 0o555 does not stop a write, so the chmod form silenced nothing there and
// this test failed on windows-pwsh only. `mkdirSync(..., recursive: true)`
// tolerates an existing directory but throws on an existing file, on every
// platform, so `markTipSeen` fails exactly where it would for a real
// permission error - before anything is printed.
const configDir = path.dirname(getGlobalConfigPath());
fs.mkdirSync(path.dirname(configDir), { recursive: true });
fs.writeFileSync(configDir, 'not a directory');
await maybeShowCompletionTip();
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
// Still a file: nothing partially wrote through the failure.
expect(fs.statSync(configDir).isFile()).toBe(true);
});
it('retires the tip quietly on a shell the installer would reject', async () => {
// `openspec completion install` exits 1 for unsupported shells, so sending
// these users there is a dead end — and this tip is the only thing that
// would ever mention completions to them.
process.env.SHELL = '/bin/tcsh';
delete process.env.PSModulePath;
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
expect(JSON.parse(fs.readFileSync(getGlobalConfigPath(), 'utf-8')).completionTipSeen).toBe(true);
});
it('leaves a config that is valid JSON but not an object untouched', async () => {
// JSON.parse succeeds here, so only the shape guard stops the write from
// turning the file into {"0":"a","completionTipSeen":true}.
const configPath = getGlobalConfigPath();
fs.mkdirSync(path.dirname(configPath), { recursive: true });
fs.writeFileSync(configPath, '["a"]');
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
expect(fs.readFileSync(configPath, 'utf-8')).toBe('["a"]');
});
it('retires the tip quietly when completions are already installed', async () => {
// Without this the CLI tells people to install completions they already
// have — including on the very next command after `completion install`,
// whose own run only defers the tip.
process.env.SHELL = '/bin/fish';
const installed = path.join(tempDir, '.config', 'fish', 'completions', 'openspec.fish');
fs.mkdirSync(path.dirname(installed), { recursive: true });
fs.writeFileSync(installed, '# completions');
await maybeShowCompletionTip();
expect(printedTip()).toBe(false);
expect(JSON.parse(fs.readFileSync(getGlobalConfigPath(), 'utf-8')).completionTipSeen).toBe(true);
});
it('still shows the tip when that shell has no completions installed', async () => {
process.env.SHELL = '/bin/fish';
await maybeShowCompletionTip();
expect(printedTip()).toBe(true);
});
it('preserves unrelated config fields when recording the flag', async () => {
const configPath = getGlobalConfigPath();
fs.mkdirSync(path.dirname(configPath), { recursive: true });
fs.writeFileSync(
configPath,
JSON.stringify({ defaultStore: 'acme', telemetry: { anonymousId: 'abc' } }, null, 2)
);
await maybeShowCompletionTip();
const raw = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
expect(raw.completionTipSeen).toBe(true);
expect(raw.defaultStore).toBe('acme');
expect(raw.telemetry.anonymousId).toBe('abc');
});
});
test/core/completions/installers/bash-installer.test.ts
+28
View File
@@ -496,4 +496,32 @@ describe('BashInstaller', () => {
expect(defaultInstaller).toBeDefined();
});
});
describe('isInstalled', () => {
// Drives the first-run completions tip: a false positive silences a hint
// the user needs, a false negative nags someone who is already set up.
async function installPath(): Promise<string> {
return installer.getInstallationPath();
}
it('is false when nothing is installed', async () => {
expect(await installer.isInstalled()).toBe(false);
});
it('is true once the completion script exists', async () => {
const target = await installPath();
await fs.mkdir(path.dirname(target), { recursive: true });
await fs.writeFile(target, '# completions');
expect(await installer.isInstalled()).toBe(true);
});
it('is false when a directory sits at the install path', async () => {
const target = await installPath();
await fs.mkdir(target, { recursive: true });
expect(await installer.isInstalled()).toBe(false);
});
});
});
test/core/completions/installers/fish-installer.test.ts
+28
View File
@@ -332,4 +332,32 @@ complete -c openspec -a 'init'
});
});
describe('isInstalled', () => {
// Drives the first-run completions tip: a false positive silences a hint
// the user needs, a false negative nags someone who is already set up.
async function installPath(): Promise<string> {
return installer.getInstallationPath();
}
it('is false when nothing is installed', async () => {
expect(await installer.isInstalled()).toBe(false);
});
it('is true once the completion script exists', async () => {
const target = await installPath();
await fs.mkdir(path.dirname(target), { recursive: true });
await fs.writeFile(target, '# completions');
expect(await installer.isInstalled()).toBe(true);
});
it('is false when a directory sits at the install path', async () => {
const target = await installPath();
await fs.mkdir(target, { recursive: true });
expect(await installer.isInstalled()).toBe(false);
});
});
});
test/core/completions/installers/powershell-installer.test.ts
+28
View File
@@ -868,4 +868,32 @@ Register-ArgumentCompleter -CommandName openspec -ScriptBlock $openspecCompleter
});
});
describe('isInstalled', () => {
// Drives the first-run completions tip: a false positive silences a hint
// the user needs, a false negative nags someone who is already set up.
async function installPath(): Promise<string> {
return installer.getInstallationPath();
}
it('is false when nothing is installed', async () => {
expect(await installer.isInstalled()).toBe(false);
});
it('is true once the completion script exists', async () => {
const target = await installPath();
await fs.mkdir(path.dirname(target), { recursive: true });
await fs.writeFile(target, '# completions');
expect(await installer.isInstalled()).toBe(true);
});
it('is false when a directory sits at the install path', async () => {
const target = await installPath();
await fs.mkdir(target, { recursive: true });
expect(await installer.isInstalled()).toBe(false);
});
});
});
test/core/config-schema.test.ts
+7
View File
@@ -392,6 +392,13 @@ describe('config-schema', () => {
expect(validateConfigKeyPath('telemetry.enabled')).toEqual({ valid: true });
});
it('rejects completionTipSeen, which the CLI manages rather than the user', () => {
// Accepted by the schema (passthrough) so `config validate` stays quiet,
// but never settable — it is runtime state, like telemetry.noticeSeen.
expect(GlobalConfigSchema.safeParse({ completionTipSeen: true }).success).toBe(true);
expect(validateConfigKeyPath('completionTipSeen').valid).toBe(false);
});
it('rejects bare telemetry and unknown leaves', () => {
expect(validateConfigKeyPath('telemetry').valid).toBe(false);
expect(validateConfigKeyPath('telemetry.anonymousId').valid).toBe(false);
test/package-install-scripts.test.ts
+25
View File
@@ -0,0 +1,25 @@
import { describe, it, expect } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { fileURLToPath } from 'node:url';
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
/**
* The published package must ship no npm lifecycle install scripts. Any of these
* makes `npm install` warn about unapproved install scripts, which reads as a
* packaging problem to users. The shell-completions tip that used to live in a
* postinstall script now prints on the CLI's first run instead.
*/
describe('published package install scripts', () => {
const packageJson = JSON.parse(
fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf-8')
) as { scripts?: Record<string, string> };
it.each(['preinstall', 'install', 'postinstall'])(
'declares no "%s" script',
(lifecycle) => {
expect(packageJson.scripts?.[lifecycle]).toBeUndefined();
}
);
});