Skip to content

feat(server): add local agent attention notification hook#1226

Open
xiaobaifly7 wants to merge 2 commits into
getpaseo:mainfrom
xiaobaifly7:agent-attention-command-hook
Open

feat(server): add local agent attention notification hook#1226
xiaobaifly7 wants to merge 2 commits into
getpaseo:mainfrom
xiaobaifly7:agent-attention-command-hook

Conversation

@xiaobaifly7
Copy link
Copy Markdown

Linked issue\n\nCloses #568\n\n### Type of change\n\n- [x] New feature (with prior issue + design alignment)\n\n### What does this PR do\n\nAdds a generic local command hook for agent attention notifications, wired through persisted config, daemon bootstrap, and websocket broadcast flow. The hook only runs when Paseo already plans to push an external notification, and hook failures never block built-in push or in-app delivery.\n\n### How did you verify it\n\n- packages/server/src/server/agent-attention-hooks.test.ts: passed with �itest run using the sibling checkout's node_modules.\n- packages/server/src/server/config-notifications.test.ts, packages/server/src/server/persisted-config.test.ts, packages/server/src/server/websocket-server.notifications.test.ts: validated the config/load and broadcast paths.\n- mcp__code_review_graph__.detect_changes_tool on the 11 changed files reported overall risk score 0.00.\n-

pm run typecheck and workspace lint on this checkout were blocked by missing local tool/dependency resolution in this environment, not by the hook logic itself.\n\n### Checklist\n\n- [x] One focused change. Unrelated cleanups split out.\n- [ ]
pm run typecheck passes\n- [ ]
pm run lint passes\n- [x]
pm run format ran (Biome)\n- [ ] UI changes include screenshots or video for every affected platform\n- [x] Tests added or updated where it made sense

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR adds a local command hook for agent attention notifications, wired through persisted config (Zod schema), daemon bootstrap, and the websocket broadcast path. The implementation is well-structured and the error-handling in the hook runner (timeout, stdin write errors, synchronous throw) is solid and test-covered.

  • Config & schema: AgentAttentionHookSchema with a superRefine cross-field rule correctly requires command unless enabled: false; the JSON Schema adds the same constraint via if/then for IDE-level validation.
  • Hook runner (agent-attention-hooks.ts): Spawns the command, writes a newline-delimited JSON payload on stdin, handles async and synchronous write failures, and enforces a configurable 5-second timeout — all with settled-flag idempotency to prevent double-resolution.
  • Websocket integration: The hook fires unconditionally after the if (plan.shouldPush) block, meaning it runs on every attention event. The user-facing documentation currently says the opposite (hook fires only when push qualifies), which needs to be corrected.

Confidence Score: 4/5

Safe to merge after correcting the documentation — the hook fires on every attention event, not only when push qualifies, and the user-facing docs say the opposite.

The hook implementation and all error-handling paths are correct and well-tested. The one concrete issue is that public-docs/configuration.md tells users the hook fires only when an external push also fires, while the code (confirmed by an explicit test) fires it unconditionally on every attention event. A user who configures a hook based on the documentation will be surprised when it fires while they are actively connected with no push delivery happening.

public-docs/configuration.md — the opening description of when the hook fires needs to be corrected before this ships to users.

Important Files Changed

Filename Overview
public-docs/configuration.md Documentation incorrectly states the hook fires only when push notification qualifies; actual behavior (confirmed by tests) is unconditional on every attention event.
packages/server/src/server/websocket-server.ts Hook runner is invoked unconditionally after the if (plan.shouldPush) block, meaning it fires for every attention event regardless of whether external push fires — directly contradicting the documentation and PR description.
packages/server/src/server/agent-attention-hooks.ts New hook runner: spawns a child process, writes JSON payload to stdin, handles timeout/error/kill paths correctly. The prior synchronous-write bug is addressed with child.kill() in the catch block.
packages/server/src/server/persisted-config.ts Adds AgentAttentionHookSchema with correct Zod superRefine cross-field validation: command required unless enabled: false.
packages/server/src/server/config.ts Adds resolveAgentAttentionHookConfig that correctly maps persisted config to AgentAttentionHookConfig, stripping the enabled flag and returning undefined when disabled.
packages/server/src/server/bootstrap.ts Wires the hook runner into VoiceAssistantWebSocketServer using optional config; correctly passes undefined when no hook is configured.
packages/website/public/schemas/paseo.config.v1.json JSON Schema for agentAttention hook with if/then conditional correctly encoding the cross-field constraint (command required unless enabled is false).
packages/server/src/server/agent-attention-hooks.test.ts Comprehensive unit tests covering: normal path, non-zero exit, start error, stdin error, sync write throw, timeout (default and custom), and timeout no-double-warn.
packages/server/src/server/websocket-server.notifications.test.ts New tests verify hook fires with the same payload as push, hook fires when plan does not push, and hook failures do not block in-app delivery.
packages/server/src/server/persisted-config.test.ts Adds Zod schema tests: accepts command hook, rejects enabled hook without command, allows disabled hook without command.
packages/server/src/server/config-notifications.test.ts Integration tests for config load paths: full config, omitted enabled field, and disabled hook all produce the correct agentAttentionHook shape in the resolved config.

Sequence Diagram

sequenceDiagram
    participant AM as AgentManager
    participant WS as WebSocketServer
    participant Push as PushNotificationSender
    participant Hook as AgentAttentionHookRunner
    participant Cmd as Local Command (child_process)
    participant Clients as WS Clients

    AM->>WS: broadcastAgentAttention(agentId, provider, reason)
    WS->>WS: buildAgentAttentionNotificationPayload()
    WS->>WS: computeNotificationPlan(clientStates)

    alt plan.shouldPush
        WS->>Push: send(notification)
        Push-->>WS: (fire-and-forget)
    end

    WS->>Hook: "run({ agentId, provider, reason, notification })"
    Hook->>Cmd: spawn(command, args)
    Hook->>Cmd: stdin.write(JSON payload)
    Hook->>Cmd: stdin.end()
    alt success (exit 0)
        Cmd-->>Hook: close(0, null)
        Hook-->>WS: resolve
    else non-zero exit / timeout / error
        Cmd-->>Hook: close(code, signal) / error
        Hook->>Hook: logger.warn(...)
        Hook-->>WS: resolve (never rejects)
    end

    WS->>Clients: broadcast attention_required (shouldNotify per plan)
Loading

Reviews (2): Last reviewed commit: "fix(server): 完善 agent attention hook 通知" | Re-trigger Greptile

Comment thread packages/server/src/server/agent-attention-hooks.ts
Comment thread packages/website/public/schemas/paseo.config.v1.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant