-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(serve): query a single session's status by id #5857
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
Changes from all commits
44e1f71
e029794
e4104ef
842d722
7eff417
21cda4c
762c94b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1074,6 +1074,7 @@ function advertisedMaxPendingPromptsPerSession( | |
| * - `POST /session/:id/load` | ||
| * - `POST /session/:id/resume` | ||
| * - `GET /workspace/:id/sessions` | ||
| * - `GET /session/:id/status` | ||
| * - `GET /session/:id/context` | ||
| * - `GET /session/:id/supported-commands` | ||
| * - `GET /session/:id/tasks` | ||
|
|
@@ -3323,6 +3324,19 @@ export function createServeApp( | |
| } | ||
| }); | ||
|
|
||
| app.get('/session/:id/status', (req, res) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This adds a new public route, but the route side tables are not updated with it. Please add an always-on capability tag such as — GPT-5 Codex via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the always-on
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This returns raw bridge data while Suggested fix: Add a server-level test that sets up a session with persisted metadata and asserts the two endpoints return different — qwen3.7-max via Qwen Code /review |
||
| const sessionId = requireSessionId(req, res); | ||
| if (sessionId === null) return; | ||
| try { | ||
| res.status(200).json(bridge.getSessionSummary(sessionId)); | ||
| } catch (err) { | ||
| sendBridgeError(res, err, { | ||
| route: 'GET /session/:id/status', | ||
| sessionId, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| app.get('/session/:id/context', async (req, res) => { | ||
| const sessionId = requireSessionId(req, res); | ||
| if (sessionId === null) return; | ||
|
|
||
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.
[Suggestion] This 200 test only covers the
displayName-present case. The PR documents a specific/statuscontract — "returnsdisplayNameonly when one is set on the live session" (docs/users/qwen-serve.md) — but nothing asserts the key is absent from the HTTP body when unset.That omission relies entirely on
res.json()dropping theundefined-valued key thattoSessionSummaryalways sets (bridge.ts:displayName: entry.displayName). A future "consistency" change to that helper (e.g.entry.displayName ?? '') would silently break the documented shape with the whole suite still green. A sibling test pins the contract:— claude-opus-4-8[1m] via Qwen Code /qreview
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.
Added your suggested test in 842d722 — a 200 case with a no-
displayNamesummary asserting'displayName' in res.body === false, so a future change to the shared summary builder can't silently break the documented shape. Thanks!