fix: suppress ora spinner when --json is used

[TabishB]

Summary

• Conditionally skip ora spinner creation when --json flag is passed in status, instructions, instructions apply, and templates commands
• Prevents spinner text from leaking to stderr, which broke JSON parsing for AI agents that combine stdout+stderr (Claude Code, Cursor, Windsurf, Copilot)
• Adds e2e tests asserting clean JSON output (no stderr, valid JSON on stdout) for all --json commands

Closes #957

Test plan

• openspec status --change <name> --json 2>&1 | jq . parses cleanly
• openspec instructions <artifact> --change <name> --json 2>&1 | jq . parses cleanly
• openspec instructions apply --change <name> --json 2>&1 | jq . parses cleanly
• openspec templates --json 2>&1 | jq . parses cleanly
• openspec list --json 2>&1 | jq . parses cleanly
• openspec schemas --json 2>&1 | jq . parses cleanly
• Non-JSON mode still shows spinners as before
• New e2e tests pass (basic.test.ts, artifact-workflow.test.ts)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed spinner animations from appearing in JSON output mode, ensuring clean JSON output and preventing parsing failures.

• Tests

  • Added comprehensive end-to-end and unit tests validating clean JSON output across all workflow commands when using JSON mode.

[1code-async]

Task completed.

I'll start by reviewing the PR changes to understand what was modified.

---

View full conversation

Powered by 1Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Conditionally disable ora spinner creation when --json flag is active across workflow commands (instructions, status, templates) and use optional chaining for stopping, preventing spinner output from mixing with JSON output in AI agent tools.

Changes

Cohort / File(s)	Summary
Spinner Lifecycle in Workflow Commands src/commands/workflow/instructions.ts, src/commands/workflow/status.ts, src/commands/workflow/templates.ts	Updated spinner initialization to skip creation when options.json is true; replaced all unconditional spinner.stop() calls with optional chaining (spinner?.stop()) to prevent errors when spinner is undefined.
E2E Test Coverage for JSON Output test/cli-e2e/basic.test.ts	Added shared test helper expectJsonOnlyOutput and six new e2e test cases validating --json output for list, schemas, status, instructions proposal, instructions apply, and templates subcommands, ensuring clean JSON with no stderr.
JSON Output Assertions test/commands/artifact-workflow.test.ts	Added expect(result.stderr).toBe('') assertions to existing JSON-output test cases for status --json, instructions --json, templates --json, and instructions apply --json.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• PR #759: Modifies statusCommand in src/commands/workflow/status.ts to add early-exit control flow logic, overlapping with this PR's changes to the same file's spinner lifecycle.

Poem

🐰 A spinner's tale most sorrowful,
Mixed with JSON, oh how dreadful!
But now with ?. so graceful and true,
Agent parsing works through and through! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely summarizes the main change: suppressing the ora spinner when the --json flag is used, which directly addresses the core problem in issue #957.
Linked Issues check	✅ Passed	The PR implementation fully satisfies issue #957 requirements: spinner creation is conditionally skipped when --json is truthy across all affected commands (status, instructions, instructions apply, templates), preventing spinner text from leaking to stderr and breaking JSON parsing.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: conditional spinner initialization in command files, optional spinner stopping to prevent errors, and e2e/unit tests validating the fix; no unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch TabishB/fix-issue-957

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[alfred-openspec]

Looks good. This is the right fix surface for #957: skip spinner creation in JSON mode rather than trying to scrub stderr after the fact, and the added tests cover the agent-facing commands that matter. CI is green; the only thing still pending was CodeRabbit when I reviewed.

[coderabbitai]

🧹 Nitpick comments (1)

src/commands/workflow/instructions.ts (1)

63-63: Optional cleanup: avoid duplicate spinner stop in throw paths.

You can remove the pre-throw spinner?.stop() calls and rely on the catch block to stop once, which slightly simplifies control flow.

Also applies to: 73-73, 92-92

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/commands/workflow/instructions.ts` at line 63, Remove the duplicate
pre-throw spinner stops: locate occurrences of "spinner?.stop()" immediately
before throwing an error in this file (the instances at the shown location and
the other two reported occurrences) and delete those pre-throw calls so that
only the catch block performs spinner?.stop(), ensuring the spinner is stopped
exactly once; keep the catch/finally spinner stop logic intact (e.g., the
existing catch/finally in the surrounding function that currently handles
spinner?.stop()).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/commands/workflow/instructions.ts`:
- Line 63: Remove the duplicate pre-throw spinner stops: locate occurrences of
"spinner?.stop()" immediately before throwing an error in this file (the
instances at the shown location and the other two reported occurrences) and
delete those pre-throw calls so that only the catch block performs
spinner?.stop(), ensuring the spinner is stopped exactly once; keep the
catch/finally spinner stop logic intact (e.g., the existing catch/finally in the
surrounding function that currently handles spinner?.stop()).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b9476404-e54b-49f0-b9ac-ccd0331d9152

📥 Commits

Reviewing files that changed from the base of the PR and between 4df6a48 and c4829c5.

📒 Files selected for processing (5)

• src/commands/workflow/instructions.ts
• src/commands/workflow/status.ts
• src/commands/workflow/templates.ts
• test/cli-e2e/basic.test.ts
• test/commands/artifact-workflow.test.ts