feat: add feedback command for submitting user feedback #509
Conversation
Implement `openspec feedback` command that creates GitHub Issues using the gh CLI. Includes graceful fallback to manual submission when gh is not available or not authenticated. Features: - Automatic gh CLI detection and authentication check - Graceful fallback with pre-filled issue URLs for manual submission - Automatic metadata inclusion (version, platform, timestamp) - /feedback skill for agent-assisted feedback with context enrichment - Comprehensive test coverage with mocked gh CLI calls
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as openspec CLI
participant Cmd as FeedbackCommand
participant Detect as gh Detection (which/where, gh auth)
participant GH as gh CLI (execFileSync)
participant Manual as Manual Submission URL
User->>CLI: feedback "message" --body "details"
CLI->>Cmd: execute(message, options)
Cmd->>Detect: isGhInstalled()?
alt gh not installed
Detect-->>Cmd: false
Cmd->>Manual: generateManualSubmissionUrl()
Cmd->>User: display fallback + formatted body
else gh installed
Detect-->>Cmd: true
Cmd->>Detect: isGhAuthenticated()?
alt unauthenticated
Detect-->>Cmd: false
Cmd->>Manual: generateManualSubmissionUrl()
Cmd->>User: display auth guidance + URL
else authenticated
Detect-->>Cmd: true
Cmd->>Cmd: generateMetadata() & formatBody()/formatTitle()
Cmd->>GH: execFileSync("gh", ["issue","create", ...])
alt gh succeeds
GH-->>Cmd: returns issue URL
Cmd->>User: print issue URL
else gh fails
GH-->>Cmd: error + exit code
Cmd->>User: print error, exit with gh status
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (5)openspec/changes/*/proposal.md📄 CodeRabbit inference engine (openspec/AGENTS.md)
Files:
openspec/changes/*/{proposal,design,tasks}.md📄 CodeRabbit inference engine (openspec/AGENTS.md)
Files:
openspec/changes/*/tasks.md📄 CodeRabbit inference engine (openspec/AGENTS.md)
Files:
openspec/changes/*/specs/**/spec.md📄 CodeRabbit inference engine (openspec/AGENTS.md)
Files:
openspec/**/**/spec.md📄 CodeRabbit inference engine (openspec/AGENTS.md)
Files:
🧠 Learnings (5)📚 Learning: 2025-11-25T01:08:02.839ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
📚 Learning: 2026-01-13T22:51:14.330ZApplied to files:
🧬 Code graph analysis (1)test/commands/feedback.test.ts (1)
🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/commands/feedback.ts`:
- Around line 10-17: The isGhInstalled function incorrectly uses the Unix-only
"which" command; replace its logic to run a cross-platform validation like
executing "gh --version" via execSync inside isGhInstalled and return true only
if the command succeeds (capture stdout/stderr with stdio:'pipe'), otherwise
catch the error and return false; keep the function signature isGhInstalled():
boolean and ensure the catch block handles any thrown error from execSync rather
than relying on "which".
- Around line 122-143: The submitViaGhCli function currently builds a single
shell string and only escapes double quotes, leaving it vulnerable to shell
metacharacter interpretation; replace execSync with execFileSync (or spawnSync)
and pass the gh executable and an args array (e.g., ['issue', 'create',
'--repo', 'Fission-AI/OpenSpec', '--title', title, '--body', body, '--label',
'feedback']) so user input is passed as argv elements rather than interpolated
into a shell command, capture stdout/stderr via stdio/options to read the
returned issue URL, and preserve the existing error handling behavior by using
the child error.status (defaulting to 1) for process.exit.
🧹 Nitpick comments (2)
src/commands/feedback.ts (1)
172-195: Consider extracting title/body formatting to reduce duplication.The
formatTitleandformatBodycalls are repeated in all three branches. Moving them before the conditionals would reduce duplication and make the code easier to maintain.♻️ Proposed refactor
export class FeedbackCommand { async execute(message: string, options?: { body?: string }): Promise<void> { + const title = formatTitle(message); + const body = formatBody(options?.body); + // Check if gh CLI is installed if (!isGhInstalled()) { - const title = formatTitle(message); - const body = formatBody(options?.body); handleFallback(title, body, 'missing'); return; } // Check if gh CLI is authenticated if (!isGhAuthenticated()) { - const title = formatTitle(message); - const body = formatBody(options?.body); handleFallback(title, body, 'unauthenticated'); return; } // Submit via gh CLI - const title = formatTitle(message); - const body = formatBody(options?.body); submitViaGhCli(title, body); } }openspec/changes/add-feedback-command/specs/cli-feedback/spec.md (1)
47-48: Minor: Consider "in the future" for American English consistency.The phrase "in future" is British English. For consistency, consider changing to "in the future".
-- **AND** displays authentication instructions: "To auto-submit in future: gh auth login" +- **AND** displays authentication instructions: "To auto-submit in the future: gh auth login"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
openspec/changes/add-feedback-command/proposal.mdopenspec/changes/add-feedback-command/specs/cli-feedback/spec.mdopenspec/changes/add-feedback-command/tasks.mdsrc/cli/index.tssrc/commands/feedback.tssrc/core/completions/command-registry.tssrc/core/templates/skill-templates.tstest/commands/feedback.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
openspec/changes/*/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Create
proposal.mdwith Why, What Changes (with BREAKING markers), and Impact sections
Files:
openspec/changes/add-feedback-command/proposal.md
openspec/changes/*/{proposal,design,tasks}.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Use direct file references in format
file.ts:42and reference specs asspecs/auth/spec.mdfor clarity
Files:
openspec/changes/add-feedback-command/proposal.mdopenspec/changes/add-feedback-command/tasks.md
openspec/changes/*/tasks.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
Create
tasks.mdwith numbered implementation sections and checkboxes in format- [ ] X.Y Task description
Files:
openspec/changes/add-feedback-command/tasks.md
openspec/changes/*/specs/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/*/specs/**/spec.md: Use## ADDED|MODIFIED|REMOVED|RENAMED Requirementsheaders in spec delta files with at least one#### Scenario:per requirement
Format scenarios with exactly four hashtags (#### Scenario:) followed by bullet points with**WHEN**and**THEN**syntax
In MODIFIED Requirements, copy the entire existing requirement block and paste under## MODIFIED Requirements, then edit to reflect new behavior
Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
In spec deltas, include a**Reason**field for REMOVED requirements and a**Migration**field explaining how to handle the removal
Files:
openspec/changes/add-feedback-command/specs/cli-feedback/spec.md
openspec/**/**/spec.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/**/spec.md: Use SHALL/MUST for normative requirements in specs; avoid should/may unless intentionally non-normative
Ensure every requirement in spec files has at least one#### Scenario:with WHEN/THEN structure
Files:
openspec/changes/add-feedback-command/specs/cli-feedback/spec.md
🧠 Learnings (6)
📚 Learning: 2025-11-25T01:08:02.839Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:02.839Z
Learning: Use `@/openspec/AGENTS.md` to learn how to create and apply change proposals, spec format and conventions, and project structure and guidelines
Applied to files:
openspec/changes/add-feedback-command/proposal.mdopenspec/changes/add-feedback-command/specs/cli-feedback/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/tasks.md : Create `tasks.md` with numbered implementation sections and checkboxes in format `- [ ] X.Y Task description`
Applied to files:
openspec/changes/add-feedback-command/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Complete all tasks sequentially from `tasks.md` and update every task to `- [x]` before marking proposal as complete
Applied to files:
openspec/changes/add-feedback-command/tasks.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use ADDED for new orthogonal capabilities; use MODIFIED for behavior/scope/criteria changes; use RENAMED for name-only changes
Applied to files:
openspec/changes/add-feedback-command/specs/cli-feedback/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use `## ADDED|MODIFIED|REMOVED|RENAMED Requirements` headers in spec delta files with at least one `#### Scenario:` per requirement
Applied to files:
openspec/changes/add-feedback-command/specs/cli-feedback/spec.md
📚 Learning: 2026-01-13T22:51:14.330Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2026-01-13T22:51:14.330Z
Learning: Scaffold `proposal.md`, `tasks.md`, optional `design.md`, and delta specs under `openspec/changes/<id>/` when creating a change proposal
Applied to files:
src/core/completions/command-registry.ts
🧬 Code graph analysis (2)
src/cli/index.ts (1)
src/commands/feedback.ts (1)
FeedbackCommand(172-195)
test/commands/feedback.test.ts (1)
src/commands/feedback.ts (1)
FeedbackCommand(172-195)
🪛 Gitleaks (8.30.0)
src/core/templates/skill-templates.ts
[high] 2374-2374: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
openspec/changes/add-feedback-command/specs/cli-feedback/spec.md
[locale-violation] ~47-~47: The phrase ‘in future’ is British English. Did you mean: “in the future”?
Context: ...ntication instructions: "To auto-submit in future: gh auth login" - AND exits with ze...
(IN_FUTURE)
🔇 Additional comments (12)
src/core/completions/command-registry.ts (1)
158-169: LGTM - feedback command registration looks correct.The command definition aligns with the CLI implementation. Minor observation: other commands with
acceptsPositional: truetypically specify apositionalType(e.g.,'path','change-id'). Consider addingpositionalType: 'message'or similar for consistency in shell completion hints, though this is optional since completions likely won't suggest message content.src/core/templates/skill-templates.ts (1)
2304-2411: Well-structured feedback skill template with comprehensive privacy guidance.The template provides clear instructions for context gathering, anonymization, and user confirmation before submission. The anonymization examples effectively demonstrate what sensitive data patterns to redact.
Regarding the static analysis hint about the API key at line 2374: this is a false positive. The
sk_live_abc123xyzstring is intentionally placed within a "Before:" example block to illustrate what sensitive data looks like before anonymization. The "After:" block shows it properly replaced with<redacted>. This is documentation, not an actual credential.openspec/changes/add-feedback-command/proposal.md (1)
1-19: Proposal structure follows conventions.The proposal correctly includes Why, What Changes, and Impact sections as per coding guidelines. The external dependency on
ghCLI is appropriately documented. File references are clear and match the actual implementation paths.src/cli/index.ts (2)
16-16: Import correctly added.
297-311: Feedback command integration follows established patterns.The command registration is consistent with other commands in the file:
- Proper error handling with try/catch
- Uses
ora().fail()for error display- Exits with code 1 on failure
- Awaits the async
execute()methodtest/commands/feedback.test.ts (4)
1-29: Test setup looks solid.The mock setup for
execSync, console spies, andprocess.exitfollows standard Vitest patterns. Theprocess.exitmock that throws an error is a common technique to prevent actual process termination while still being able to assert on exit codes.
98-223: Comprehensive coverage for successful submission scenarios.Tests verify:
- gh issue create invocation
- Success message display
- Body flag inclusion
- Title formatting with "Feedback:" prefix
- Metadata injection (version, platform, timestamp)
- Label addition
Good coverage of the happy path.
225-275: Error handling tests are thorough.Good coverage of:
- gh CLI execution failures with proper exit code propagation
- Quote escaping to prevent shell injection
The quote escaping test at lines 255-274 is particularly important for security, ensuring user input with quotes doesn't break the shell command.
277-331: Fallback output tests validate manual submission guidance.Tests confirm that when gh CLI is unavailable, users receive:
- Formatted feedback display with clear delimiters
- Pre-filled GitHub issue URL with proper parameters
- Title, body, and labels properly encoded in the URL
openspec/changes/add-feedback-command/tasks.md (1)
1-27: LGTM!The tasks file follows the required format with numbered implementation sections and checkboxes in the
- [x] X.Y Task descriptionformat. All tasks are properly organized and marked complete, aligning with the PR objectives. Based on learnings, this matches the expected structure.openspec/changes/add-feedback-command/specs/cli-feedback/spec.md (2)
105-110: Spec/implementation mismatch: network connectivity suggestion not implemented.The spec states the system "suggests checking network connectivity" on network failure, but the implementation in
feedback.tsonly displays the error fromghCLI without adding this additional guidance. Either update the implementation to add this suggestion, or adjust the spec to match the current behavior.
1-163: Spec follows required format conventions.The spec correctly uses
## ADDED Requirementsheaders,#### Scenario:format with exactly four hashtags, and**WHEN**/**THEN**syntax throughout. Each requirement has at least one scenario. Based on learnings, this aligns with the expected spec delta format.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add Windows compatibility: use 'where gh' on Windows, 'which gh' on Unix/macOS - Fix shell injection vulnerability: replace execSync with execFileSync and argument arrays - Fix British English phrasing: "in future" → "in the future" - Reduce code duplication: extract formatTitle/formatBody to single location - Update tests to verify execFileSync usage and cross-platform command detection - Add spec scenarios for safe command execution and cross-platform support
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
Summary
Implements the
openspec feedbackcommand that allows users and agents to submit feedback about OpenSpec directly from the CLI by creating GitHub Issues using theghCLI.This PR completes all tasks from the
add-feedback-commandchange proposal (16/16 tasks complete).What Changes
1. Feedback Command (
src/commands/feedback.ts)openspec feedback <message> [--body <text>]ghCLI detection: Checks ifghis installed usingwhich ghgh auth statusbefore submittinggh issue createto submit feedback to the OpenSpec repositoryghis missing or unauthenticated:2. Metadata Inclusion
All feedback submissions automatically include:
3. Agent Skill (
/feedback)Added feedback skill template in
src/core/templates/skill-templates.ts:<path><redacted><company><user>4. Shell Completions
5. Comprehensive Testing
Added
test/commands/feedback.test.tswith 11 test cases covering:ghCLI with fallbackghwith fallbackghCLI failuresTest plan
Implementation Notes
execSyncforghCLI interactionsghCLI behavior🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
feedbackcommand to submit feedback as GitHub issues.Tests
✏️ Tip: You can customize this high-level summary in your review settings.