Archived
Merge remote-tracking branch 'origin/main' into feat/model-questions-ux
# Conflicts: # src/server/sessiond.ts # src/server/sessions/sessionRoutes.test.ts
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { PiSessionService, type PiSessionRuntime } from "./piSessionService.js";
|
||||
import { CapturingSessionEventHub, emptyArchiveStore, fakeRuntime, sessionGateway, sessionRecord, sessionRef, testModelRuntime } from "./piSessionService.testSupport.js";
|
||||
import { isSessionActive } from "../../shared/activity.js";
|
||||
import type { SessionActivity, SessionStartupProgressEvent } from "../../shared/apiTypes.js";
|
||||
|
||||
const TEST_AGENT_DIR = "/tmp/pi-web-test-agent";
|
||||
@@ -77,7 +78,7 @@ describe("PiSessionService session startup progress", () => {
|
||||
// The proof that matters: the user is told what is being waited on before
|
||||
// the wait ends, not after it.
|
||||
expect(startupText(hub)).toEqual(["Creating session: Starting the Pi session"]);
|
||||
expect(startupEvents(hub).at(0)).toMatchObject({ cwd: "/workspace", activity: { sessionId: "session-1", phase: "active" } });
|
||||
expect(startupEvents(hub).at(0)).toMatchObject({ activity: { sessionId: "session-1", phase: "active" } });
|
||||
|
||||
runtimeResult.resolve(fake.runtime);
|
||||
await started;
|
||||
@@ -157,11 +158,42 @@ describe("PiSessionService session startup progress", () => {
|
||||
|
||||
await service.start("/workspace");
|
||||
|
||||
expect(startupEvents(hub).at(-1)).toMatchObject({ cwd: "/workspace", activity: { sessionId: "session-1", phase: "idle", label: "idle" } });
|
||||
expect(startupEvents(hub).at(-1)).toMatchObject({ activity: { sessionId: "session-1", phase: "idle", label: "idle" } });
|
||||
expect(startupEvents(hub).at(-1)?.activity.detail).toBeUndefined();
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("echoes a create's correlation token on every startup report of that construction", async () => {
|
||||
const { hub, service } = startupService();
|
||||
|
||||
await service.start("/workspace", { startupToken: "pending-session-3-k2x9" });
|
||||
|
||||
// The token labels the browser row that is waiting, so it must ride every
|
||||
// report of this construction, the closing idle one included.
|
||||
expect(startupEvents(hub).map((event) => event.startupToken)).toEqual([
|
||||
"pending-session-3-k2x9",
|
||||
"pending-session-3-k2x9",
|
||||
"pending-session-3-k2x9",
|
||||
]);
|
||||
// The token is an opaque throwaway label, never the session's identity.
|
||||
expect(startupEvents(hub).map((event) => event.activity.sessionId)).toEqual(["session-1", "session-1", "session-1"]);
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("publishes no correlation token when a create supplies none, and none for an open", async () => {
|
||||
const created = startupService();
|
||||
await created.service.start("/workspace");
|
||||
const opened = startupService({ sessionRecords: [sessionRecord("session-1")] });
|
||||
await opened.service.status(sessionRef("session-1"));
|
||||
|
||||
for (const hub of [created.hub, opened.hub]) {
|
||||
expect(startupEvents(hub).length).toBeGreaterThan(0);
|
||||
expect(startupEvents(hub).every((event) => event.startupToken === undefined)).toBe(true);
|
||||
}
|
||||
await created.service.dispose();
|
||||
await opened.service.dispose();
|
||||
});
|
||||
|
||||
it("ends the startup window when the runtime construction itself fails", async () => {
|
||||
const failure = new Error("runtime unavailable");
|
||||
const { hub, service } = startupService({ createAgentRuntime: () => Promise.reject(failure) });
|
||||
@@ -204,6 +236,34 @@ describe("PiSessionService session startup progress", () => {
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("reports startup progress as starting rather than as work in progress", async () => {
|
||||
const { hub, service } = startupService();
|
||||
|
||||
await service.start("/workspace");
|
||||
|
||||
// Startup phases are published with an "active" phase so the waiting user
|
||||
// sees them, but opening a session is not work: nothing that decides whether
|
||||
// work is in progress may count them.
|
||||
const phases = startupEvents(hub).filter((event) => event.activity.phase === "active");
|
||||
expect(phases).toHaveLength(2);
|
||||
expect(phases.map((event) => isSessionActive(undefined, event.activity))).toEqual([false, false]);
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("still reports a real activity published during startup as work", async () => {
|
||||
const { hub, fake, service } = startupService();
|
||||
|
||||
await service.start("/workspace");
|
||||
fake.emit({ type: "tool_execution_start", toolName: "bash" });
|
||||
|
||||
// The marker belongs to the startup channel alone; an ordinary activity for
|
||||
// the same session still counts, or the fix would hide real work.
|
||||
const running = activityUpdates(hub).filter((activity) => activity.phase === "active");
|
||||
expect(running.length).toBeGreaterThan(0);
|
||||
expect(running.every((activity) => isSessionActive(undefined, activity))).toBe(true);
|
||||
await service.dispose();
|
||||
});
|
||||
|
||||
it("keeps startup reporting event-only, writing no session or workspace activity state", async () => {
|
||||
const recorder = recordingWorkspaceActivity();
|
||||
const failure = new Error("runtime unavailable");
|
||||
|
||||
@@ -204,6 +204,11 @@ type SessionCreationProvenance = "tracked-subsession";
|
||||
interface StartSessionOptions {
|
||||
parentSession?: string;
|
||||
initialModel?: AgentModel;
|
||||
/**
|
||||
* Opaque label, echoed on this construction's startup progress so a browser
|
||||
* row with no session id yet can recognise its own.
|
||||
*/
|
||||
startupToken?: string;
|
||||
}
|
||||
|
||||
interface InternalStartSessionOptions extends StartSessionOptions {
|
||||
@@ -395,7 +400,7 @@ interface PendingSessionOpen {
|
||||
promise: Promise<ActiveSession<PiSessionRuntime>>;
|
||||
}
|
||||
|
||||
interface CreateSessionRuntimeOptions extends Pick<InternalStartSessionOptions, "initialModel" | "creationProvenance"> {
|
||||
interface CreateSessionRuntimeOptions extends Pick<InternalStartSessionOptions, "initialModel" | "creationProvenance" | "startupToken"> {
|
||||
notificationGeneration?: SessionNotificationGeneration;
|
||||
notifications?: "enabled" | "disabled";
|
||||
/**
|
||||
@@ -1016,6 +1021,7 @@ export class PiSessionService implements SessionRouteService {
|
||||
cwd,
|
||||
{
|
||||
startupIntent: "create",
|
||||
...(options.startupToken === undefined ? {} : { startupToken: options.startupToken }),
|
||||
...(options.initialModel === undefined ? {} : { initialModel: options.initialModel }),
|
||||
...(options.creationProvenance === undefined ? {} : { creationProvenance: options.creationProvenance }),
|
||||
},
|
||||
@@ -2459,7 +2465,7 @@ export class PiSessionService implements SessionRouteService {
|
||||
cwd: string,
|
||||
options: CreateSessionRuntimeOptions = {},
|
||||
): Promise<ActiveSession<PiSessionRuntime>> {
|
||||
const startup = this.startupProgress(sessionManager, cwd, options.startupIntent ?? "open");
|
||||
const startup = this.startupProgress(sessionManager, options.startupIntent ?? "open", options.startupToken);
|
||||
try {
|
||||
return await this.createSessionRuntime(sessionManager, cwd, options, startup);
|
||||
} finally {
|
||||
@@ -3105,23 +3111,23 @@ export class PiSessionService implements SessionRouteService {
|
||||
/**
|
||||
* Build the reporter for one session construction.
|
||||
*
|
||||
* The session id and cwd are both known before any await — a `SessionManager`
|
||||
* has its id from construction — so the daemon can name what it is starting
|
||||
* even though the `PiAgentSession` that {@link publishActivity} needs does not
|
||||
* exist yet. When either is missing there is nothing honest to route on, so
|
||||
* the reporter stays silent and the browser keeps its own generic wording.
|
||||
* The session id is known before any await — a `SessionManager` has its id
|
||||
* from construction — so the daemon can name what it is starting even though
|
||||
* the `PiAgentSession` that {@link publishActivity} needs does not exist yet.
|
||||
* Without an id there is nothing to report against, so the reporter stays
|
||||
* silent and the browser keeps its own generic wording.
|
||||
*/
|
||||
private startupProgress(sessionManager: PiSessionManager, cwd: string, intent: "create" | "open"): SessionStartupProgressReporter {
|
||||
private startupProgress(sessionManager: PiSessionManager, intent: "create" | "open", startupToken: string | undefined): SessionStartupProgressReporter {
|
||||
const sessionId = sessionManager.getSessionId();
|
||||
if (sessionId === "" || cwd === "") return { report: noop, end: noop };
|
||||
if (sessionId === "") return { report: noop, end: noop };
|
||||
const label = intent === "create" ? "Creating session" : "Opening session";
|
||||
return {
|
||||
report: (phase) => { this.publishStartupProgress(sessionId, cwd, label, "active", this.startupDetail(phase)); },
|
||||
report: (phase) => { this.publishStartupProgress(sessionId, startupToken, label, "active", this.startupDetail(phase)); },
|
||||
end: () => {
|
||||
// A real activity published during the window (an extension error, say)
|
||||
// is the truth about this session and must survive the clear.
|
||||
if (this.activities.has(sessionId)) return;
|
||||
this.publishStartupProgress(sessionId, cwd, "idle", "idle", undefined);
|
||||
this.publishStartupProgress(sessionId, startupToken, "idle", "idle", undefined);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -3133,17 +3139,22 @@ export class PiSessionService implements SessionRouteService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Report startup progress on the global channel only, keyed by `cwd` so a
|
||||
* browser row that has no session id yet can find it.
|
||||
* Report startup progress on the global channel only, echoing the caller's
|
||||
* correlation token so a waiting browser row recognises its own construction.
|
||||
*
|
||||
* Unlike {@link publishActivity} this deliberately records nothing: no
|
||||
* `activities` entry, no workspace activity, no unread observation. There is
|
||||
* no session to own that state, and a failed creation would leave it stranded.
|
||||
*
|
||||
* Every report is marked `startup`, which is what keeps a session that is
|
||||
* merely opening from counting as one doing work. This is the only publisher
|
||||
* that sets the marker, and because it writes no `activities` entry no later
|
||||
* heartbeat re-publication can carry it.
|
||||
*/
|
||||
private publishStartupProgress(sessionId: string, cwd: string, label: string, phase: "active" | "idle", detail: string | undefined): void {
|
||||
private publishStartupProgress(sessionId: string, startupToken: string | undefined, label: string, phase: "active" | "idle", detail: string | undefined): void {
|
||||
const at = new Date().toISOString();
|
||||
const activity = detail === undefined ? { sessionId, phase, label, at } : { sessionId, phase, label, detail, at };
|
||||
this.events.publishGlobal({ type: "session.startup", cwd, activity });
|
||||
const activity = detail === undefined ? { sessionId, phase, label, at, startup: true } : { sessionId, phase, label, detail, at, startup: true };
|
||||
this.events.publishGlobal(startupToken === undefined ? { type: "session.startup", activity } : { type: "session.startup", startupToken, activity });
|
||||
}
|
||||
|
||||
private publishActivity(session: PiAgentSession, label: string, phase: "active" | "idle" | "error", detail?: string): void {
|
||||
|
||||
@@ -28,6 +28,7 @@ import { PiSessionService, type PiSessionManagerGateway } from "./piSessionServi
|
||||
import { testModelRuntime } from "./piSessionService.testSupport.js";
|
||||
import { SessionNotificationStore } from "./sessionNotificationStore.js";
|
||||
import type { SessionRouteLookup, SessionRouteService } from "./sessionService.js";
|
||||
import type { ClientSession } from "../types.js";
|
||||
import { registerSessionRoutes } from "./sessionRoutes.js";
|
||||
import type { NormalizedSessionCleanupRequest } from "./sessionCleanup.js";
|
||||
|
||||
@@ -762,6 +763,35 @@ describe("session routes", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("forwards a create's optional correlation token alongside the normalized cwd", async () => {
|
||||
const routeApp = Fastify({ logger: false });
|
||||
await routeApp.register(fastifyWebsocket);
|
||||
const eventHub = new SessionEventHub();
|
||||
const routeService = new CapturingRouteSessionService();
|
||||
registerSessionRoutes(routeApp, routeService, eventHub);
|
||||
|
||||
try {
|
||||
const requestCwd = resolve("/repo");
|
||||
const withToken = await routeApp.inject({ method: "POST", url: "/sessions", payload: { cwd: requestCwd, startupToken: "pending-session-3-k2x9" } });
|
||||
const withoutToken = await routeApp.inject({ method: "POST", url: "/sessions", payload: { cwd: requestCwd } });
|
||||
// An older browser, or any non-browser caller, sends no token; and a
|
||||
// malformed one must not reach the service as a label it would echo.
|
||||
const malformedToken = await routeApp.inject({ method: "POST", url: "/sessions", payload: { cwd: requestCwd, startupToken: 7 } });
|
||||
|
||||
expect(withToken.statusCode).toBe(200);
|
||||
expect(withoutToken.statusCode).toBe(200);
|
||||
expect(malformedToken.statusCode).toBe(400);
|
||||
expect(malformedToken.json()).toEqual({ error: "startupToken field must be a string" });
|
||||
expect(routeService.startCalls).toEqual([
|
||||
{ cwd: requestCwd, startupToken: "pending-session-3-k2x9" },
|
||||
{ cwd: requestCwd, startupToken: undefined },
|
||||
]);
|
||||
} finally {
|
||||
await routeService.dispose();
|
||||
await routeApp.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects malformed bulk mutation bodies before calling the service", async () => {
|
||||
const routeApp = Fastify({ logger: false });
|
||||
await routeApp.register(fastifyWebsocket);
|
||||
@@ -804,6 +834,7 @@ class CapturingRouteSessionService implements SessionRouteService {
|
||||
readonly navigateTreeCalls: { lookup: SessionRouteLookup; request: SessionTreeNavigateRequest }[] = [];
|
||||
readonly submitAskCalls: { lookup: SessionRouteLookup; askId: string; submission: AskUserSubmission }[] = [];
|
||||
readonly cancelAskCalls: { lookup: SessionRouteLookup; askId: string }[] = [];
|
||||
readonly startCalls: { cwd: string; startupToken: string | undefined }[] = [];
|
||||
askError: Error | undefined;
|
||||
reloadError: Error | undefined;
|
||||
clearQueueError: Error | undefined;
|
||||
@@ -883,7 +914,11 @@ class CapturingRouteSessionService implements SessionRouteService {
|
||||
}
|
||||
|
||||
list(): never { throw unusedRouteMethod("list"); }
|
||||
start(): never { throw unusedRouteMethod("start"); }
|
||||
|
||||
start(cwd: string, options?: { startupToken?: string }): Promise<ClientSession> {
|
||||
this.startCalls.push({ cwd, startupToken: options?.startupToken });
|
||||
return Promise.resolve({ id: "session-1", path: "/tmp/session-1.jsonl", cwd, created: "2026-06-25T00:00:00.000Z", modified: "2026-06-25T00:00:00.000Z", messageCount: 0, firstMessage: "" });
|
||||
}
|
||||
|
||||
dismissWarning(lookup: SessionRouteLookup, dismissId: string): Promise<SessionStatus> {
|
||||
this.dismissWarningCalls.push({ lookup, dismissId });
|
||||
|
||||
@@ -45,10 +45,14 @@ export function registerSessionRoutes(app: FastifyInstance, sessions: SessionRou
|
||||
}
|
||||
});
|
||||
|
||||
app.post<{ Body: { cwd?: unknown } | undefined }>(`${prefix}/sessions`, async (request, reply) => {
|
||||
app.post<{ Body: { cwd?: unknown; startupToken?: unknown } | undefined }>(`${prefix}/sessions`, async (request, reply) => {
|
||||
try {
|
||||
const body = requireRecord(request.body);
|
||||
return await sessions.start(normalizeRequestCwd(requireString(body, "cwd")));
|
||||
// An opaque label the caller uses to recognise its own construction's
|
||||
// startup reports. Optional: only a browser row waiting for a session id
|
||||
// has anything to correlate.
|
||||
const startupToken = body["startupToken"] === undefined ? undefined : requireNonEmptyString(body, "startupToken");
|
||||
return await sessions.start(normalizeRequestCwd(requireString(body, "cwd")), optionalField("startupToken", startupToken));
|
||||
} catch (error) {
|
||||
return reply.code(400).send({ error: errorMessage(error) });
|
||||
}
|
||||
|
||||
@@ -42,7 +42,12 @@ export type SessionRouteLookup = string | SessionRouteRef;
|
||||
*/
|
||||
export interface SessionRouteService {
|
||||
list(cwd: string): Promise<ClientSession[]>;
|
||||
start(cwd: string): Promise<ClientSession>;
|
||||
/**
|
||||
* Create a session. `startupToken` is an opaque label the caller supplies so
|
||||
* it can recognise this construction's startup progress reports; the service
|
||||
* echoes it and never interprets it.
|
||||
*/
|
||||
start(cwd: string, options?: { startupToken?: string }): Promise<ClientSession>;
|
||||
messages(ref: SessionRouteLookup, page?: { before?: number; limit?: number }): Promise<unknown[] | ClientMessagePage>;
|
||||
status(ref: SessionRouteLookup): Promise<ClientSessionStatus>;
|
||||
streamSnapshot(ref: SessionRouteLookup): Promise<SessionStreamSnapshot>;
|
||||
|
||||
Reference in New Issue
Block a user