feat(run,notifications,profiles): stop sending prompts nobody typed, and fix notification + profile delivery
Run console:
- Picking a pipeline no longer pre-writes anything into the prompt box
(`promptPrefill` in `LaneConsolePane.tsx` is gone — it wrote the driving
slash command on a claude lane and a stage checklist on a codex one).
Whatever sits in that box is sent as a real prompt the moment the run
starts, so the box is the user's alone now.
- Resuming a session sends NO prompt at all (`handleStart` in `RunSetup.tsx`):
the resumed session carries its own transcript, and leftover text in the box
fired an extra turn into it.
Notifications:
- `client/src/lib/notify.ts` is now the ONE sender, used by both
`useNotifications` and Settings' test button, and it reports where the
notification landed: `POST /api/push/send` answering `{native:false,pushed:0}`
means it went NOWHERE, so the client falls back to a local notification
instead of counting the 200 as delivered.
- A notification carries the lane that raised it (`/workspace?lane=<id>`), and
Workspace selects that lane on arrival.
Profiles:
- `server/lib/cc-profiles.js` reads and writes the macOS login Keychain item
`Claude Code-credentials`, where the `claude` CLI actually keeps
`claudeAiOauth` on darwin — a switch used to rewrite a file nothing reads.
The file stays the fallback, is kept in step only when it already exists, and
`AGENTHUB_CC_KEYCHAIN=0` pins it (tests set it).
Tests: npm run test:server (1820 pass), client vitest (553 pass).
This commit is contained in:
@@ -36,7 +36,9 @@ describe("useNotifications", () => {
|
||||
localStorage.clear();
|
||||
// notify() posts to /api/push/send first and only falls back to a local
|
||||
// Notification when that throws, so the fetch spy is what we assert on.
|
||||
fetchMock = vi.fn().mockResolvedValue({ ok: true });
|
||||
fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: true, json: async () => ({ ok: true, native: true, pushed: 1 }) });
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
vi.stubGlobal(
|
||||
"Notification",
|
||||
@@ -76,6 +78,32 @@ describe("useNotifications", () => {
|
||||
expect(sentTitles()[0]).toBe("Claude Finished Responding");
|
||||
});
|
||||
|
||||
it("shows a local notification when the push landed nowhere", async () => {
|
||||
// What a browser with no push subscription gets back: HTTP 200, zero
|
||||
// deliveries. Before this, the 200 alone counted as success and the user
|
||||
// saw nothing at all.
|
||||
fetchMock.mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ ok: true, native: false, pushed: 0, failed: 0 }),
|
||||
});
|
||||
enable();
|
||||
renderHook(() => useNotifications());
|
||||
|
||||
emit("Stop", "Done");
|
||||
await vi.waitFor(() => expect(Notification).toHaveBeenCalledTimes(1));
|
||||
expect(vi.mocked(Notification).mock.calls[0]?.[0]).toBe("Claude Finished Responding");
|
||||
});
|
||||
|
||||
it("does not double up when the push was delivered", async () => {
|
||||
enable();
|
||||
renderHook(() => useNotifications());
|
||||
|
||||
emit("Stop", "Done");
|
||||
await vi.waitFor(() => expect(sentTitles()).toHaveLength(1));
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
expect(Notification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stays silent while notifications are disabled", async () => {
|
||||
renderHook(() => useNotifications());
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
* - `../i18n`
|
||||
* - `../lib/eventBus`
|
||||
* - `../lib/push`
|
||||
* - `../lib/notify`
|
||||
* - `../lib/types`
|
||||
*
|
||||
* ## Public surface
|
||||
@@ -48,43 +49,14 @@
|
||||
*
|
||||
* ----------------------------------------------------------------------------- */
|
||||
|
||||
import { useEffect } from "react";
|
||||
import { useEffect, useRef } from "react";
|
||||
import i18n from "../i18n";
|
||||
import { api } from "../lib/api";
|
||||
import { eventBus } from "../lib/eventBus";
|
||||
import { subscribeToPush } from "../lib/push";
|
||||
import { notify } from "../lib/notify";
|
||||
import { loadNotifPrefs } from "../lib/notifPrefs";
|
||||
import type { WSMessage, Session, Agent, DashboardEvent } from "../lib/types";
|
||||
|
||||
/**
|
||||
* Shows a browser notification, preferring a server-relayed push (so it can
|
||||
* arrive even if this tab isn't the active one, or the browser is backgrounded)
|
||||
* and falling back to a local service-worker/`Notification` call if the
|
||||
* server is unreachable. No-ops when the user hasn't granted permission.
|
||||
* @param title Notification title.
|
||||
* @param body Notification body text.
|
||||
*/
|
||||
async function notify(title: string, body: string) {
|
||||
if (!("Notification" in window) || Notification.permission !== "granted") return;
|
||||
try {
|
||||
await fetch("/api/push/send", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ title, body }),
|
||||
});
|
||||
} catch {
|
||||
// Server unreachable - fall back to local notification
|
||||
try {
|
||||
if ("serviceWorker" in navigator) {
|
||||
const registration = await navigator.serviceWorker.ready;
|
||||
await registration.showNotification(title, { body, icon: "/favicon.ico", silent: false });
|
||||
} else {
|
||||
new Notification(title, { body, icon: "/favicon.ico" });
|
||||
}
|
||||
} catch {
|
||||
// Silently ignore
|
||||
}
|
||||
}
|
||||
}
|
||||
import type { WSMessage, Session, Agent, DashboardEvent, Lane } from "../lib/types";
|
||||
|
||||
/**
|
||||
* Wires the dashboard's {@link eventBus} up to browser notifications, per the
|
||||
@@ -96,13 +68,40 @@ async function notify(title: string, body: string) {
|
||||
* already been granted, so push delivery survives a page reload.
|
||||
*/
|
||||
export function useNotifications() {
|
||||
// A lane's `session_id` is its current/last-run session — matching on it
|
||||
// is how a click lands on the lane that actually raised the notification,
|
||||
// instead of just focusing whichever browser tab was already open.
|
||||
const lanesRef = useRef<Lane[]>([]);
|
||||
|
||||
useEffect(() => {
|
||||
const prefs = loadNotifPrefs();
|
||||
if (prefs.enabled && "Notification" in window && Notification.permission === "granted") {
|
||||
subscribeToPush().catch(() => {});
|
||||
}
|
||||
|
||||
api.lanes
|
||||
.list()
|
||||
.then((r) => (lanesRef.current = r.lanes))
|
||||
.catch(() => {});
|
||||
|
||||
function laneUrlForSession(sessionId: string): string | undefined {
|
||||
const lane = lanesRef.current.find((l) => l.session_id === sessionId);
|
||||
return lane ? `/workspace?lane=${lane.id}` : undefined;
|
||||
}
|
||||
|
||||
return eventBus.subscribe((msg: WSMessage) => {
|
||||
if (msg.type === "lane_update") {
|
||||
const payload = msg.data as { lane?: Lane; removed?: number };
|
||||
if (payload.lane) {
|
||||
const i = lanesRef.current.findIndex((l) => l.id === payload.lane!.id);
|
||||
if (i === -1) lanesRef.current = [...lanesRef.current, payload.lane];
|
||||
else lanesRef.current = lanesRef.current.map((l, idx) => (idx === i ? payload.lane! : l));
|
||||
} else if (payload.removed !== undefined) {
|
||||
lanesRef.current = lanesRef.current.filter((l) => l.id !== payload.removed);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const prefs = loadNotifPrefs();
|
||||
if (!prefs.enabled) return;
|
||||
|
||||
@@ -112,7 +111,8 @@ export function useNotifications() {
|
||||
const s = msg.data as Session;
|
||||
notify(
|
||||
i18n.t("errors:notifications.newSession"),
|
||||
s.name || `${i18n.t("errors:notifications.sessionDefault")}${s.id.slice(0, 8)}`
|
||||
s.name || `${i18n.t("errors:notifications.sessionDefault")}${s.id.slice(0, 8)}`,
|
||||
laneUrlForSession(s.id)
|
||||
);
|
||||
break;
|
||||
}
|
||||
@@ -121,7 +121,8 @@ export function useNotifications() {
|
||||
if (s.status === "error" && prefs.onSessionError) {
|
||||
notify(
|
||||
i18n.t("errors:notifications.sessionError"),
|
||||
s.name || `${i18n.t("errors:notifications.sessionDefault")}${s.id.slice(0, 8)}`
|
||||
s.name || `${i18n.t("errors:notifications.sessionDefault")}${s.id.slice(0, 8)}`,
|
||||
laneUrlForSession(s.id)
|
||||
);
|
||||
}
|
||||
break;
|
||||
@@ -130,12 +131,17 @@ export function useNotifications() {
|
||||
if (!prefs.onSubagentSpawn) return;
|
||||
const a = msg.data as Agent;
|
||||
if (a.type === "subagent") {
|
||||
notify(i18n.t("errors:notifications.subagentSpawned"), a.name);
|
||||
notify(
|
||||
i18n.t("errors:notifications.subagentSpawned"),
|
||||
a.name,
|
||||
laneUrlForSession(a.session_id)
|
||||
);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case "new_event": {
|
||||
const ev = msg.data as DashboardEvent;
|
||||
const url = laneUrlForSession(ev.session_id);
|
||||
// The two states where Claude is BLOCKED on the user get their own
|
||||
// flags (issue #29): `Notification` is a permission prompt or a
|
||||
// question, `Stop` is the end of a turn. Both used to hang off
|
||||
@@ -144,17 +150,20 @@ export function useNotifications() {
|
||||
if (ev.event_type === "Notification" && prefs.onAwaitingInput) {
|
||||
notify(
|
||||
i18n.t("errors:notifications.awaitingInput"),
|
||||
ev.summary || i18n.t("errors:notifications.defaultBody")
|
||||
ev.summary || i18n.t("errors:notifications.defaultBody"),
|
||||
url
|
||||
);
|
||||
} else if (ev.event_type === "Stop" && prefs.onTurnComplete) {
|
||||
notify(
|
||||
i18n.t("errors:notifications.finishedResponding"),
|
||||
ev.summary || i18n.t("errors:notifications.readyForInput")
|
||||
ev.summary || i18n.t("errors:notifications.readyForInput"),
|
||||
url
|
||||
);
|
||||
} else if (ev.event_type === "SessionEnd" && prefs.onSessionComplete) {
|
||||
notify(
|
||||
i18n.t("errors:notifications.sessionCompleted"),
|
||||
ev.summary || i18n.t("errors:notifications.sessionClosed")
|
||||
ev.summary || i18n.t("errors:notifications.sessionClosed"),
|
||||
url
|
||||
);
|
||||
}
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user