-
Notifications
You must be signed in to change notification settings - Fork 0
chore: sync upstream parent/main #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ement (Fission-AI#382) * feat(cli): add openspec config command for global configuration management Implements the `openspec config` command with subcommands: - `path`: Show config file location - `list [--json]`: Show all current settings - `get <key>`: Get a specific value (raw output for scripting) - `set <key> <value> [--string]`: Set a value with auto type coercion - `unset <key>`: Remove a key (revert to default) - `reset --all [-y]`: Reset configuration to defaults - `edit`: Open config in $EDITOR/$VISUAL Key features: - Dot notation for nested key access (e.g., featureFlags.someFlag) - Auto type coercion (true/false → boolean, numbers → number) - --string flag to force string storage - Zod schema validation with unknown field passthrough - Reserved --scope flag for future project-local config - Windows-compatible editor spawning with proper path quoting - Shell completion registry integration * test(config): add additional unit tests for validation and coercion - Add tests for unknown fields with various types - Add test to verify error message path for featureFlags - Add test for number values rejection in featureFlags - Add config set simulation tests to verify full coerce → set → validate flow * fix(config): avoid shell parsing in config edit to handle paths with spaces Use spawn with shell: false and pass configPath as an argument instead of building a shell command string. This correctly handles spaces in both the EDITOR path and config file path on all platforms. * chore(openspec): archive add-config-command and create cli-config spec Move completed change to archive and apply spec deltas to create the cli-config specification documenting the config command interface. * Validate config keys on set
* Version Packages * chore: trigger CI --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tabish Bidiwale <tabishbidiwale@gmail.com>
Replace classic npm token authentication with OIDC trusted publishing: - Add `id-token: write` permission for OIDC token generation - Upgrade to Node 24 (includes npm 11.5.1+ required for OIDC) - Remove NPM_TOKEN/NODE_AUTH_TOKEN env vars (OIDC replaces them) This eliminates the need for rotating npm access tokens and provides cryptographically verified publisher identity with automatic provenance attestation. Requires configuring trusted publisher on npmjs.com: - Organization: Fission-AI - Repository: OpenSpec - Workflow: release-prepare.yml
…ission-AI#392) * fix(cli): use dynamic import for @inquirer/prompts in config command The config command (added in Fission-AI#382) reintroduced the pre-commit hook hang issue that was fixed in Fission-AI#380. The static import of @inquirer/prompts at module load time causes stdin event listeners to be registered even when running non-interactive commands, preventing clean process exit when stdin is piped (as pre-commit does). Convert the static import to a dynamic import that only loads inquirer when the `config reset` command is actually used interactively. Fixes Fission-AI#367 * chore: add ESLint with no-restricted-imports rule for @InQuirer Add ESLint configuration that prevents static imports of @inquirer/* modules. This prevents future regressions of the pre-commit hook hang issue fixed in this PR. The rule shows a helpful error message pointing to issue Fission-AI#367 for context. init.ts is exempted since it's already dynamically imported from the CLI. * ci: add ESLint step to lint job Run `pnpm lint` in CI to enforce the no-restricted-imports rule that prevents static @InQuirer imports.
* Version Packages * chore: trigger CI --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tabish Bidiwale <tabishbidiwale@gmail.com>
…AI#395) * fix(cli): respect --no-interactive flag in validate command The validate command's spinner was starting regardless of the --no-interactive flag, causing hangs in pre-commit hooks. Changes: - Pass noInteractive option to runBulkValidation - Handle Commander.js --no-* flag syntax (sets interactive=false) - Only start ora spinner when in interactive mode - Add CI environment variable check to isInteractive() for industry standard compliance * test: add unit tests for interactive utilities and CLI flag - Export resolveNoInteractive() helper for reuse - Add InteractiveOptions type export for testing - Refactor validate.ts to use resolveNoInteractive() - Add 17 unit tests for isInteractive() and resolveNoInteractive() - Add CLI integration test for --no-interactive flag This prevents future regressions where Commander.js --no-* flag parsing is not properly handled.
* Version Packages * chore: trigger CI --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tabish Bidiwale <tabishbidiwale@gmail.com>
Add internal documentation for the artifact-based approach to OpenSpec core. This document outlines design decisions, terminology, and the philosophy behind treating dependencies as enablers rather than gates.
…Fission-AI#403) (Fission-AI#404) When creating a new spec file, REMOVED requirements are now ignored with a warning instead of causing archive to fail. This enables refactoring scenarios where old fields are removed while documenting a capability for the first time. Fixes Fission-AI#403
86d6548 to
5120283
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR syncs the latest upstream commits from the parent repository (Fission-AI/OpenSpec), bringing in several bug fixes, new features, and infrastructure improvements.
Key Changes:
- Adds a new
openspec configcommand for managing global configuration with subcommands (path, list, get, set, unset, reset, edit) - Fixes archive command to allow REMOVED requirements when creating new spec files
- Fixes --no-interactive flag handling in validate command to prevent hangs
- Migrates CI to npm OIDC trusted publishing (removes token-based auth)
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/interactive.test.ts | New comprehensive test suite for interactive utility functions |
| test/core/config-schema.test.ts | New test suite for config schema validation and manipulation |
| test/core/archive.test.ts | Added tests for REMOVED requirements in new specs scenario |
| test/commands/validate.test.ts | Added test for --no-interactive flag handling |
| test/commands/config.test.ts | New integration tests for config command |
| src/utils/interactive.ts | Exported types/functions and added CI environment detection |
| src/core/config-schema.ts | New config schema with zod validation and utility functions |
| src/core/completions/command-registry.ts | Added config command and subcommands to completion registry |
| src/core/archive.ts | Modified to allow REMOVED requirements for new specs with warnings |
| src/commands/validate.ts | Fixed to respect --no-interactive flag and pass it through |
| src/commands/config.ts | New command implementing full config management with dynamic imports |
| src/cli/index.ts | Registered new config command |
| eslint.config.js | New ESLint config preventing static @InQuirer imports |
| docs/artifact_poc.md | New documentation for artifact-based POC architecture |
| CHANGELOG.md | Updated with versions 0.17.0, 0.17.1, 0.17.2 |
| .gitignore | Removed docs/ directory from gitignore |
| .github/workflows/release-prepare.yml | Updated for npm OIDC trusted publishing |
| .github/workflows/ci.yml | Added lint step to CI pipeline |
| pnpm-lock.yaml | Added ESLint and typescript-eslint dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!validation.success) { | ||
| console.error(`Error: Invalid configuration - ${validation.error}`); | ||
| process.exitCode = 1; | ||
| } | ||
| } catch (error) { | ||
| if ((error as NodeJS.ErrnoException).code === 'ENOENT') { | ||
| console.error(`Error: Config file not found at ${configPath}`); | ||
| } else if (error instanceof SyntaxError) { | ||
| console.error(`Error: Invalid JSON in ${configPath}`); | ||
| console.error(error.message); | ||
| } else { | ||
| console.error(`Error: Unable to validate configuration - ${error instanceof Error ? error.message : String(error)}`); |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation error messages in lines 217-228 could be more specific. When validation fails, consider including which specific configuration key or value caused the issue rather than just the generic validation error. This would make debugging configuration problems easier for users.
| if (plan.removed.length > 0) { | ||
| console.log( | ||
| chalk.yellow( | ||
| `⚠️ Warning: ${specName} - ${plan.removed.length} REMOVED requirement(s) ignored for new spec (nothing to remove).` | ||
| ) |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting the warning message string to a constant or improving clarity. The double negative "REMOVED requirement(s) ignored for new spec (nothing to remove)" could be confusing. Consider: "Warning: ${specName} - ${plan.removed.length} REMOVED requirement(s) were ignored because this is a new specification with no existing content to remove from."
| }, | ||
| }, | ||
| { | ||
| ignores: ['dist/**', 'node_modules/**', '*.js', '*.mjs'], |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ignores pattern on line 40 uses '*.js', '*.mjs' which will only match files in the root directory, not JavaScript files in subdirectories. Consider using '**/*.js', '**/*.mjs' to match JavaScript files at any level, or document why only root-level JS files should be ignored.
| ignores: ['dist/**', 'node_modules/**', '*.js', '*.mjs'], | |
| ignores: ['dist/**', 'node_modules/**', '**/*.js', '**/*.mjs'], |
| @@ -0,0 +1,125 @@ | |||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | |||
| import { isInteractive, resolveNoInteractive, InteractiveOptions } from '../../src/utils/interactive.js'; | |||
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import InteractiveOptions.
| import { isInteractive, resolveNoInteractive, InteractiveOptions } from '../../src/utils/interactive.js'; | |
| import { isInteractive, resolveNoInteractive } from '../../src/utils/interactive.js'; |
5120283 to
9d776a2
Compare
Merges latest commits from Fission-AI/OpenSpec: - fix(archive): allow REMOVED requirements - feat(cli): add openspec config command - fix(cli): respect --no-interactive flag - fix(cli): dynamic import for @inquirer/prompts - feat(ci): migrate to npm OIDC trusted publishing - docs: add artifact POC analysis document Resolved conflict: kept fork version 0.1.0
9d776a2 to
2dc0872
Compare
Summary
Syncs latest commits from Fission-AI/OpenSpec (parent).
Upstream commits included:
Conflict resolution:
package.json: kept fork version0.1.0(upstream is at0.17.2)Test plan