Skip to content

Commit 3714fc3

Browse files
committed
fix(daemon): make od conversation info actually work and stop reporting 404 as daemon-not-running (#6116)
`od conversation info <conversationId>` always 404ed because: 1. `case 'info'` fetched the unscoped route `/api/conversations/<id>`, but the daemon only serves project-scoped routes (`/api/projects/:id/conversations/...`). 2. There was no GET handler for a single conversation — only PATCH and DELETE — so even with the right scope the request would have 404ed. 3. The 404 got mapped to `code: 'daemon-not-running'`, sending users on a wild-goose chase checking daemon sockets when the daemon was actually fine. Fix: - Add a project-scoped GET handler for `/api/projects/:id/conversations/:cid` that returns the conversation object. - Make `case 'info'` use `--project <id> <conversationId>` (consistent with `od conversation list <projectId>`) and hit the project-scoped route through `positionalArgs` so a flag value isn't mistaken for the conversation id. - On a 404 use `fallbackCode: 'not-found'` instead of the misleading `'daemon-not-running'`. The daemon answered — that's literally what a 404 proves. Regression test `conversation-info-cli.test.ts` pins all three behaviors: success path hits the scoped route (verifies the exact request URL seen by a stub HTTP server), 404 path emits `not-found` and never `daemon-not-running`, and the missing-`--project` path bails with the new usage line. Closes #6116. Co-authored-by: Nicholas-Xiong <2482929840@qq.com> Signed-off-by: xxiaoxiong <2482929840@qq.com>
1 parent 6b90486 commit 3714fc3

3 files changed

Lines changed: 149 additions & 5 deletions

File tree

apps/daemon/src/cli.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7146,13 +7146,18 @@ Common options:
71467146
return;
71477147
}
71487148
case 'info': {
7149-
const id = rest.find((a) => !a.startsWith('-'));
7150-
if (!id) {
7151-
console.error('Usage: od conversation info <conversationId>');
7149+
const projectId = typeof flags.project === 'string' && flags.project
7150+
? flags.project
7151+
: null;
7152+
const id = positionalArgs(rest, PROJECT_STRING_FLAGS)[0];
7153+
if (!projectId || !id) {
7154+
console.error('Usage: od conversation info --project <projectId> <conversationId>');
71527155
process.exit(2);
71537156
}
7154-
const resp = await fetch(`${base}/api/conversations/${encodeURIComponent(id)}`);
7155-
if (!resp.ok) return structuredHttpFailure(resp);
7157+
const resp = await fetch(
7158+
`${base}/api/projects/${encodeURIComponent(projectId)}/conversations/${encodeURIComponent(id)}`,
7159+
);
7160+
if (!resp.ok) return structuredHttpFailure(resp, resp.status === 404 ? 'not-found' : 'daemon-not-running');
71567161
const data = await resp.json();
71577162
process.stdout.write(JSON.stringify(data, null, 2) + '\n');
71587163
return;

apps/daemon/src/routes/project/conversations.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ export function registerProjectConversationRoutes(app: Express, ctx: RegisterPro
133133
res.json({ conversation: conv });
134134
});
135135

136+
app.get('/api/projects/:id/conversations/:cid', (req, res) => {
137+
// Project-scoped GET for a single conversation. Previously the
138+
// only per-conversation verbs were PATCH/DELETE; `od conversation
139+
// info <cid>` therefore had nothing to hit and 404ed (issue #6116).
140+
const conv = getConversation(db, req.params.cid);
141+
if (!conv || conv.projectId !== req.params.id) {
142+
return res.status(404).json({ error: 'not found' });
143+
}
144+
res.json({ conversation: conv });
145+
});
146+
136147
app.patch('/api/projects/:id/conversations/:cid', (req, res) => {
137148
const conv = getConversation(db, req.params.cid);
138149
if (!conv || conv.projectId !== req.params.id) {
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Regression for `od conversation info` always 404ing (#6116):
2+
// `case 'info'` used to fetch the unscoped route
3+
// `/api/conversations/<id>`, but the daemon only serves
4+
// project-scoped routes (`/api/projects/:id/conversations/…`). The
5+
// 404 then mapped to `daemon-not-running`, sending users on a wild
6+
// goose chase checking daemon sockets.
7+
//
8+
// Fix:
9+
// 1. `od conversation info --project <id> <conversationId>` hits
10+
// the project-scoped route.
11+
// 2. 404 now exits with `not-found`, not `daemon-not-running`.
12+
13+
import { execFile } from 'node:child_process';
14+
import http from 'node:http';
15+
import { promisify } from 'node:util';
16+
import { fileURLToPath } from 'node:url';
17+
import { describe, expect, it } from 'vitest';
18+
19+
const execFileAsync = promisify(execFile);
20+
const cliEntry = fileURLToPath(new URL('../src/cli.ts', import.meta.url));
21+
22+
describe('od conversation info CLI', () => {
23+
it('hits the project-scoped conversation route and prints the conversation', async () => {
24+
const seenRequests: Array<{ method: string; url: string }> = [];
25+
const server = http.createServer((req, res) => {
26+
let body = '';
27+
req.setEncoding('utf8');
28+
req.on('data', (chunk) => {
29+
body += chunk;
30+
});
31+
req.on('end', () => {
32+
seenRequests.push({ method: req.method ?? '', url: req.url ?? '' });
33+
if (req.method === 'GET' && req.url === '/api/projects/proj-1/conversations/conv-9') {
34+
res.writeHead(200, { 'content-type': 'application/json' });
35+
res.end(JSON.stringify({
36+
conversation: { id: 'conv-9', projectId: 'proj-1', title: 'Demo' },
37+
messages: [],
38+
}));
39+
return;
40+
}
41+
res.writeHead(404, { 'content-type': 'application/json' });
42+
res.end(JSON.stringify({ error: 'not found' }));
43+
});
44+
});
45+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
46+
const address = server.address();
47+
if (!address || typeof address === 'string') throw new Error('server did not bind');
48+
const port = address.port;
49+
50+
try {
51+
let stdout = '';
52+
try {
53+
const r = await execFileAsync(
54+
process.execPath,
55+
[
56+
'--import',
57+
'tsx',
58+
cliEntry,
59+
'conversation',
60+
'info',
61+
'--project', 'proj-1',
62+
'conv-9',
63+
'--daemon-url', `http://127.0.0.1:${port}`,
64+
'--json',
65+
],
66+
);
67+
stdout = r.stdout;
68+
} catch (err) {
69+
const e = err as NodeJS.ErrnoException & { stdout?: string; stderr?: string };
70+
stdout = e.stdout ?? '';
71+
}
72+
const parsed = JSON.parse(stdout);
73+
expect(parsed.conversation.id).toBe('conv-9');
74+
// Confirm we hit the project-scoped route, not the broken unscoped one.
75+
expect(seenRequests).toContainEqual({ method: 'GET', url: '/api/projects/proj-1/conversations/conv-9' });
76+
expect(seenRequests.some((r) => r.url === '/api/conversations/conv-9')).toBe(false);
77+
} finally {
78+
server.close();
79+
}
80+
});
81+
82+
it('exits with `not-found` (not daemon-not-running) when the conversation is missing', async () => {
83+
const server = http.createServer((req, res) => {
84+
req.resume();
85+
res.writeHead(404, { 'content-type': 'application/json' });
86+
res.end(JSON.stringify({ error: 'not found' }));
87+
});
88+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
89+
const address = server.address();
90+
if (!address || typeof address === 'string') throw new Error('server did not bind');
91+
const port = address.port;
92+
93+
try {
94+
const result = await execFileAsync(
95+
process.execPath,
96+
[
97+
'--import', 'tsx', cliEntry,
98+
'conversation', 'info',
99+
'--project', 'proj-1',
100+
'conv-missing',
101+
'--daemon-url', `http://127.0.0.1:${port}`,
102+
'--json',
103+
],
104+
).catch((err: NodeJS.ErrnoException & { stdout?: string; stderr?: string }) => err);
105+
106+
// 404 should produce a non-zero exit, AND the structured error
107+
// payload should use `not-found` rather than the misleading
108+
// `daemon-not-running` code.
109+
expect(result.code).toBeTruthy();
110+
const stderr = (result as { stderr?: string }).stderr ?? '';
111+
expect(stderr).toContain('"code":');
112+
expect(stderr).toContain('"not-found"');
113+
expect(stderr).not.toContain('daemon-not-running');
114+
} finally {
115+
server.close();
116+
}
117+
});
118+
119+
it('prints usage and exits non-zero when --project is missing', async () => {
120+
const result = await execFileAsync(
121+
process.execPath,
122+
['--import', 'tsx', cliEntry, 'conversation', 'info', 'conv-9', '--json'],
123+
).catch((err: NodeJS.ErrnoException & { stdout?: string; stderr?: string }) => err);
124+
expect(result.code).toBeTruthy();
125+
const stderr = (result as { stderr?: string }).stderr ?? '';
126+
expect(stderr).toContain('Usage: od conversation info --project <projectId> <conversationId>');
127+
});
128+
});

0 commit comments

Comments
 (0)