Skip to content

Commit 9c22939

Browse files
clay-goodclaude
andcommitted
fix(feedback): submit feedback when the repo has no feedback label
`openspec feedback` passed `--label feedback` unconditionally, but the repository does not define that label. gh resolves label names before creating the issue, so it failed with "could not add label: 'feedback' not found" on every invocation and the command exited non-zero, discarding the feedback the user had just composed. Retry once without the label when — and only when — gh reports that it could not add the label, and tell the user the label was not applied. Every other failure keeps its existing behavior: print gh's error and exit with gh's exit code. If the label is created later, the first attempt succeeds again and the retry never runs. The cli-feedback spec gains a scenario for the unlabeled path; its error-handling and fallback scenarios are unchanged. Closes #1091 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a13abea commit 9c22939

4 files changed

Lines changed: 189 additions & 34 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@fission-ai/openspec': patch
3+
---
4+
5+
Fix `openspec feedback` failing when the repository does not define the `feedback` label. The command now retries without the label and notes that it was not applied, instead of exiting with an error and discarding the feedback.

openspec/specs/cli-feedback/spec.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ The system SHALL provide an `openspec feedback` command that creates a GitHub Is
1616
- **AND** the issue has the `feedback` label
1717
- **AND** the system displays the created issue URL
1818

19+
#### Scenario: Repository does not define the feedback label
20+
21+
- **WHEN** user executes `openspec feedback "Great tool!"`
22+
- **AND** the repository does not define the `feedback` label, so `gh` refuses to create the issue
23+
- **THEN** the system retries `gh issue create` without the label
24+
- **AND** the issue is created in the openspec repository without the `feedback` label
25+
- **AND** the system displays the created issue URL
26+
- **AND** the system notes that the label was not applied
27+
1928
#### Scenario: Safe command execution
2029

2130
- **WHEN** submitting feedback via `gh` CLI

src/commands/feedback.ts

Lines changed: 76 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -119,41 +119,89 @@ function displayFormattedFeedback(title: string, body: string): void {
119119
}
120120

121121
/**
122-
* Submit feedback via gh CLI
122+
* Check whether gh refused the issue because the repository does not define
123+
* the label. gh resolves label names before creating the issue, so this
124+
* failure means no issue was created.
125+
*/
126+
function isMissingLabelError(error: any): boolean {
127+
const output = `${error?.stderr ?? ''}${error?.message ?? ''}`;
128+
129+
return /could not add label/i.test(output);
130+
}
131+
132+
/**
133+
* Report a gh CLI failure and exit, preserving gh's exit code
134+
*/
135+
function reportGhFailure(error: any): void {
136+
// Display the error output from gh CLI
137+
if (error.stderr) {
138+
console.error(error.stderr.toString());
139+
} else if (error.message) {
140+
console.error(error.message);
141+
}
142+
143+
// Exit with the same code as gh CLI
144+
process.exit(error.status ?? 1);
145+
}
146+
147+
/**
148+
* Create the feedback issue via gh CLI
123149
* Uses execFileSync to prevent shell injection vulnerabilities
124150
*/
151+
function createIssue(title: string, body: string, labels: string[]): string {
152+
const args = [
153+
'issue',
154+
'create',
155+
'--repo',
156+
'Fission-AI/OpenSpec',
157+
'--title',
158+
title,
159+
'--body',
160+
body,
161+
];
162+
163+
for (const label of labels) {
164+
args.push('--label', label);
165+
}
166+
167+
const result = execFileSync('gh', args, { encoding: 'utf-8', stdio: 'pipe' });
168+
169+
return result.trim();
170+
}
171+
172+
/**
173+
* Submit feedback via gh CLI
174+
*/
125175
function submitViaGhCli(title: string, body: string): void {
126-
try {
127-
const result = execFileSync(
128-
'gh',
129-
[
130-
'issue',
131-
'create',
132-
'--repo',
133-
'Fission-AI/OpenSpec',
134-
'--title',
135-
title,
136-
'--body',
137-
body,
138-
'--label',
139-
'feedback',
140-
],
141-
{ encoding: 'utf-8', stdio: 'pipe' }
142-
);
176+
let issueUrl: string;
177+
let labelApplied = true;
143178

144-
const issueUrl = result.trim();
145-
console.log(`\n✓ Feedback submitted successfully!`);
146-
console.log(`Issue URL: ${issueUrl}\n`);
179+
try {
180+
issueUrl = createIssue(title, body, ['feedback']);
147181
} catch (error: any) {
148-
// Display the error output from gh CLI
149-
if (error.stderr) {
150-
console.error(error.stderr.toString());
151-
} else if (error.message) {
152-
console.error(error.message);
182+
if (!isMissingLabelError(error)) {
183+
reportGhFailure(error);
184+
return;
153185
}
154186

155-
// Exit with the same code as gh CLI
156-
process.exit(error.status ?? 1);
187+
// The repository does not define the 'feedback' label. Nothing was
188+
// created, so retry unlabeled rather than dropping the feedback.
189+
try {
190+
issueUrl = createIssue(title, body, []);
191+
labelApplied = false;
192+
} catch (retryError: any) {
193+
reportGhFailure(retryError);
194+
return;
195+
}
196+
}
197+
198+
console.log(`\n✓ Feedback submitted successfully!`);
199+
console.log(`Issue URL: ${issueUrl}\n`);
200+
201+
if (!labelApplied) {
202+
console.log(
203+
"Note: created without the 'feedback' label because the repository does not define it.\n"
204+
);
157205
}
158206
}
159207

test/commands/feedback.test.ts

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,17 +327,110 @@ describe('FeedbackCommand', () => {
327327
throw error;
328328
});
329329

330-
try {
331-
await feedbackCommand.execute('Test');
332-
} catch (error: any) {
333-
// Should exit with the same code as gh CLI
334-
expect(error.message).toBe('process.exit(1)');
335-
}
330+
await expect(feedbackCommand.execute('Test')).rejects.toThrow(
331+
'process.exit(1)'
332+
);
336333

337334
// Should display the error from gh CLI
338335
expect(consoleErrorSpy).toHaveBeenCalledWith(
339336
expect.stringContaining('Network connectivity issue')
340337
);
338+
339+
// A non-label failure must NOT be retried
340+
expect(mockExecFileSync).toHaveBeenCalledTimes(1);
341+
});
342+
343+
it('should retry without the label when the repo does not define it', async () => {
344+
const issueUrl = 'https://github.com/Fission-AI/OpenSpec/issues/129';
345+
346+
mockExecSync.mockImplementation((cmd: string, options?: any) => {
347+
if (cmd === 'which gh' || cmd === 'where gh') {
348+
return Buffer.from('/usr/local/bin/gh');
349+
}
350+
if (cmd === 'gh auth status') {
351+
return Buffer.from('Logged in');
352+
}
353+
return '';
354+
});
355+
356+
// gh resolves label names before creating the issue, so a repo without
357+
// the label fails with no issue created
358+
mockExecFileSync.mockImplementation((_cmd: string, args: string[]) => {
359+
if (args.includes('--label')) {
360+
const error: any = new Error('gh failed');
361+
error.status = 1;
362+
error.stderr = Buffer.from("could not add label: 'feedback' not found");
363+
throw error;
364+
}
365+
return `${issueUrl}\n`;
366+
});
367+
368+
await feedbackCommand.execute('Test');
369+
370+
expect(mockExecFileSync).toHaveBeenCalledTimes(2);
371+
372+
// First attempt asks for the label
373+
expect(mockExecFileSync).toHaveBeenNthCalledWith(
374+
1,
375+
'gh',
376+
expect.arrayContaining(['--label', 'feedback']),
377+
expect.any(Object)
378+
);
379+
380+
// Retry drops it
381+
expect(mockExecFileSync).toHaveBeenNthCalledWith(
382+
2,
383+
'gh',
384+
expect.not.arrayContaining(['--label']),
385+
expect.any(Object)
386+
);
387+
388+
// The feedback still lands as an issue, and the user is told the label
389+
// was not applied
390+
expect(consoleLogSpy).toHaveBeenCalledWith(
391+
expect.stringContaining('Feedback submitted successfully')
392+
);
393+
expect(consoleLogSpy).toHaveBeenCalledWith(
394+
expect.stringContaining(issueUrl)
395+
);
396+
expect(consoleLogSpy).toHaveBeenCalledWith(
397+
expect.stringContaining("without the 'feedback' label")
398+
);
399+
});
400+
401+
it('should preserve gh exit code when the unlabeled retry also fails', async () => {
402+
mockExecSync.mockImplementation((cmd: string, options?: any) => {
403+
if (cmd === 'which gh' || cmd === 'where gh') {
404+
return Buffer.from('/usr/local/bin/gh');
405+
}
406+
if (cmd === 'gh auth status') {
407+
return Buffer.from('Logged in');
408+
}
409+
return '';
410+
});
411+
412+
mockExecFileSync.mockImplementation((_cmd: string, args: string[]) => {
413+
const error: any = new Error('gh failed');
414+
415+
if (args.includes('--label')) {
416+
error.status = 1;
417+
error.stderr = Buffer.from("could not add label: 'feedback' not found");
418+
} else {
419+
error.status = 4;
420+
error.stderr = Buffer.from('Error: issues are disabled');
421+
}
422+
423+
throw error;
424+
});
425+
426+
await expect(feedbackCommand.execute('Test')).rejects.toThrow(
427+
'process.exit(4)'
428+
);
429+
430+
expect(mockExecFileSync).toHaveBeenCalledTimes(2);
431+
expect(consoleErrorSpy).toHaveBeenCalledWith(
432+
expect.stringContaining('issues are disabled')
433+
);
341434
});
342435

343436
it('should handle quotes in title and body without escaping (no shell injection)', async () => {

0 commit comments

Comments
 (0)