-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(serve): add daemon file logger (#4548) #4559
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
1b7a1f2
aec7f86
1327bdc
dbc3d6b
becc750
b8bc60d
d1be101
dcda723
0f4c66f
cfb7b79
9e578e5
df67178
ad34528
f152614
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -697,6 +697,17 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| // (b) `server.close` rejecting new connections, during which a | ||||||||||||||||
| // late-arriving `POST /session` slips a fresh child past cleanup. | ||||||||||||||||
| let shuttingDown = false; | ||||||||||||||||
|
|
||||||||||||||||
| // Tee writeServeDebugLine through the optional onDiagnosticLine callback. | ||||||||||||||||
| // The module-level writeServeDebugLine is left intact for other entry points; | ||||||||||||||||
| // inside createHttpAcpBridge we use this wrapper exclusively. | ||||||||||||||||
| const teeServeDebugLine = (message: string): void => { | ||||||||||||||||
| writeServeDebugLine(message); | ||||||||||||||||
| if (opts.onDiagnosticLine && isServeDebugLoggingEnabled()) { | ||||||||||||||||
|
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] The daemon log — the whole point of this PR — silently misses bridge-internal diagnostics by default. An operator reading The stderr gate (
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||
| opts.onDiagnosticLine(`qwen serve debug: ${message}`, 'info'); | ||||||||||||||||
| } | ||||||||||||||||
| }; | ||||||||||||||||
|
|
||||||||||||||||
| // Coalesces concurrent `spawnOrAttach` calls under single-scope and | ||||||||||||||||
| // tracks in-progress thread-scope spawns for shutdown to await. | ||||||||||||||||
| // Single-scope uses the workspaceKey as the dedup key (at most one | ||||||||||||||||
|
|
@@ -1426,7 +1437,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| const published = entry.events.publish(envelope); | ||||||||||||||||
| if (published === undefined) { | ||||||||||||||||
| failureCount += 1; | ||||||||||||||||
| writeServeDebugLine( | ||||||||||||||||
| teeServeDebugLine( | ||||||||||||||||
| `broadcastWorkspaceEvent: publish on session ${entry.sessionId} no-op (bus closed)`, | ||||||||||||||||
| ); | ||||||||||||||||
| } else { | ||||||||||||||||
|
|
@@ -1439,7 +1450,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| `${JSON.stringify(entry.sessionId)} (type=${envelope.type}): ` + | ||||||||||||||||
| `${err instanceof Error ? err.message : String(err)}`; | ||||||||||||||||
| if (shuttingDown) { | ||||||||||||||||
| writeServeDebugLine(detail); | ||||||||||||||||
| teeServeDebugLine(detail); | ||||||||||||||||
| } else { | ||||||||||||||||
| writeStderrLine(`qwen serve: ${detail}`); | ||||||||||||||||
| } | ||||||||||||||||
|
|
@@ -2263,7 +2274,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| // `context.clientId` against this session's registry. | ||||||||||||||||
| const actualSessionId = permissionMediator.peekSessionFor(requestId); | ||||||||||||||||
| if (actualSessionId !== undefined && actualSessionId !== sessionId) { | ||||||||||||||||
| writeServeDebugLine( | ||||||||||||||||
| teeServeDebugLine( | ||||||||||||||||
| `rejected permission vote ${JSON.stringify(requestId)} ` + | ||||||||||||||||
| `for session ${JSON.stringify(sessionId)}; request belongs to ` + | ||||||||||||||||
| `session ${JSON.stringify(actualSessionId)}.`, | ||||||||||||||||
|
|
@@ -2349,7 +2360,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| // Mediator already emitted `permission_already_resolved`. | ||||||||||||||||
| return false; | ||||||||||||||||
| case 'unknown_request': | ||||||||||||||||
| writeServeDebugLine( | ||||||||||||||||
| teeServeDebugLine( | ||||||||||||||||
| `rejected permission vote ${JSON.stringify(requestId)} ` + | ||||||||||||||||
| `for session ${JSON.stringify(sessionId)}; mediator has no ` + | ||||||||||||||||
| `pending or resolved record.`, | ||||||||||||||||
|
|
@@ -2645,7 +2656,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| const published = entry.events.publish(event); | ||||||||||||||||
| if (published === undefined) { | ||||||||||||||||
| failureCount += 1; | ||||||||||||||||
| writeServeDebugLine( | ||||||||||||||||
| teeServeDebugLine( | ||||||||||||||||
| `publishWorkspaceEvent: publish on session ${entry.sessionId} no-op (bus closed)`, | ||||||||||||||||
| ); | ||||||||||||||||
| } else { | ||||||||||||||||
|
|
@@ -2658,7 +2669,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { | |||||||||||||||
| `${JSON.stringify(entry.sessionId)} (type=${event.type}): ` + | ||||||||||||||||
| `${err instanceof Error ? err.message : String(err)}`; | ||||||||||||||||
| if (shuttingDown) { | ||||||||||||||||
| writeServeDebugLine(detail); | ||||||||||||||||
| teeServeDebugLine(detail); | ||||||||||||||||
| } else { | ||||||||||||||||
| writeStderrLine(`qwen serve: ${detail}`); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
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]
teeServeDebugLinegates theonDiagnosticLinecallback onisServeDebugLoggingEnabled(). This means bridge-internal diagnostics (permission vote rejections, broadcast failures, workspace event publish errors) are not written to the daemon log unlessQWEN_SERVE_DEBUG=1was set at boot.The daemon log — the whole point of this PR — silently misses bridge-internal diagnostics by default. An operator reading
~/.qwen/debug/daemon/latestat 3 AM will see a gap exactly where they need data most.The stderr gate (
writeServeDebugLineon line 704) should remain. But the file path (onDiagnosticLineon line 705) should fire unconditionally:— qwen3.7-max via Qwen Code /review