Inject current user device context into agent runtime#33
Inject current user device context into agent runtime#33artile wants to merge 2 commits intojakemor:mainfrom
Conversation
Greptile SummaryThis PR propagates browser-detected device context from the client through the WebSocket layer into Claude and Codex agent runtimes, allowing the AI to answer device-related questions without spawning shell commands. Key changes:
Issues found:
Confidence Score: 5/5Safe to merge — all remaining findings are P2 quality/test improvements with no runtime impact. The device-context propagation pipeline is correct end-to-end: detection, transmission, storage, env injection, and system-prompt injection all work as intended. The two findings are (1) a weakened test assertion that loses coverage on ping-absence, and (2) a minor system-prompt inconsistency that has no observable effect today because the label always equals the device tag. Neither constitutes a present defect on the changed path. src/client/app/socket.test.ts — the 'does not ping' test should assert ws.sent has exactly one message to preserve the original test's intent. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser as Client (Browser)
participant Socket as KannaSocket
participant WsRouter as ws-router (Server)
participant Agent as AgentCoordinator
participant Claude as Claude / Codex
Browser->>Socket: WebSocket open
Socket->>WsRouter: system.setClientContext { currentUserDevice: "pixel" }
WsRouter->>WsRouter: ws.data.clientContext = context
WsRouter-->>Socket: ack
Browser->>Socket: chat.send { content: "..." }
Socket->>WsRouter: chat.send command
WsRouter->>Agent: send(command, ws.data.clientContext)
Agent->>Agent: buildAgentEnv → CURRENT_USER_DEVICE=pixel
Agent->>Agent: buildRuntimeContextInstruction → system prompt snippet
Agent->>Claude: start turn (env overrides + system prompt)
Claude-->>Agent: stream response
Agent-->>WsRouter: ack + chatId
WsRouter-->>Socket: ack
Reviews (1): Last reviewed commit: "fix: drop invalid ws-router protocol imp..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| await socket.ensureHealthyConnection() | ||
|
|
||
| expect(ws.sent).toHaveLength(0) | ||
| expect(ws.sent[0]).toMatchObject({ | ||
| type: "command", | ||
| command: { | ||
| type: "system.setClientContext", | ||
| context: { | ||
| currentUserDevice: "pixel", | ||
| currentUserDeviceLabel: "pixel", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The original assertion was expect(ws.sent).toHaveLength(0), which guaranteed that no messages were sent during ensureHealthyConnection() when the connection is fresh. The replacement only checks ws.sent[0] (the new setClientContext message) but never asserts that ws.sent[1] is absent. If a regression caused a spurious system.ping to be sent, this test would silently pass — defeating its purpose of confirming no ping occurs on a fresh connection.
| await socket.ensureHealthyConnection() | |
| expect(ws.sent).toHaveLength(0) | |
| expect(ws.sent[0]).toMatchObject({ | |
| type: "command", | |
| command: { | |
| type: "system.setClientContext", | |
| context: { | |
| currentUserDevice: "pixel", | |
| currentUserDeviceLabel: "pixel", | |
| }, | |
| }, | |
| expect(ws.sent[0]).toMatchObject({ | |
| type: "command", | |
| command: { | |
| type: "system.setClientContext", | |
| context: { | |
| currentUserDevice: "pixel", | |
| currentUserDeviceLabel: "pixel", | |
| }, | |
| }, | |
| }) | |
| expect(ws.sent).toHaveLength(1) |
| `- If the user asks for CURRENT_USER_DEVICE or CURRENT_USER_DEVICE_LABEL, answer with \`${currentUserDevice}\` directly.`, | ||
| "- Do not run shell commands just to read CURRENT_USER_DEVICE or CURRENT_USER_DEVICE_LABEL.", |
There was a problem hiding this comment.
CURRENT_USER_DEVICE and CURRENT_USER_DEVICE_LABEL
The instruction on line 79 reads:
If the user asks for
CURRENT_USER_DEVICEorCURRENT_USER_DEVICE_LABEL, answer with${currentUserDevice}directly.
But CURRENT_USER_DEVICE_LABEL is a separate field (clientContext.currentUserDeviceLabel) and the two can differ. buildAgentEnv already handles this correctly by falling back through the label chain, yet the system prompt hard-codes the device tag as the answer for both variable names. Today detectClientContext sets them to the same value, so there's no observable bug — but if a future caller provides a richer label (e.g. "Pixel 8 Pro") the AI would still answer "pixel" for CURRENT_USER_DEVICE_LABEL, contradicting the actual env var.
Consider tracking both values in buildRuntimeContextInstruction:
function buildRuntimeContextInstruction(clientContext?: ClientContext) {
const currentUserDevice = clientContext?.currentUserDevice ?? "unknown"
const currentUserDeviceLabel =
clientContext?.currentUserDeviceLabel ?? currentUserDevice
return [
"Runtime context:",
"- You are running on the VPS host, not on the user's local device.",
`- The user's current device tag is \`${currentUserDevice}\`.`,
"- If you need to mention the current device, use that exact tag unless later tool output proves otherwise.",
`- If the user asks for CURRENT_USER_DEVICE, answer with \`${currentUserDevice}\` directly.`,
`- If the user asks for CURRENT_USER_DEVICE_LABEL, answer with \`${currentUserDeviceLabel}\` directly.`,
"- Do not run shell commands just to read CURRENT_USER_DEVICE or CURRENT_USER_DEVICE_LABEL.",
].join("\n")
}1 participant
Summary
Notes
Verification