fix: make agent metadata title/branch application reliable (#81)
This commit is contained in:
@@ -439,6 +439,38 @@ describe("AgentManager", () => {
|
|||||||
expect(afterReload?.config?.title).toBeUndefined();
|
expect(afterReload?.config?.title).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("setTitle bumps updatedAt and persists title in the same snapshot write", async () => {
|
||||||
|
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-set-title-updated-at-"));
|
||||||
|
const storagePath = join(workdir, "agents");
|
||||||
|
const storage = new AgentStorage(storagePath, logger);
|
||||||
|
const manager = new AgentManager({
|
||||||
|
clients: {
|
||||||
|
codex: new TestAgentClient(),
|
||||||
|
},
|
||||||
|
registry: storage,
|
||||||
|
logger,
|
||||||
|
idFactory: () => "00000000-0000-4000-8000-000000000127",
|
||||||
|
});
|
||||||
|
|
||||||
|
const snapshot = await manager.createAgent({
|
||||||
|
provider: "codex",
|
||||||
|
cwd: workdir,
|
||||||
|
});
|
||||||
|
|
||||||
|
const before = await storage.get(snapshot.id);
|
||||||
|
expect(before).not.toBeNull();
|
||||||
|
|
||||||
|
await manager.setTitle(snapshot.id, "Generated title");
|
||||||
|
|
||||||
|
const after = await storage.get(snapshot.id);
|
||||||
|
expect(after?.title).toBe("Generated title");
|
||||||
|
expect(Date.parse(after!.updatedAt)).toBeGreaterThan(Date.parse(before!.updatedAt));
|
||||||
|
|
||||||
|
const live = manager.getAgent(snapshot.id);
|
||||||
|
expect(live).not.toBeNull();
|
||||||
|
expect(live!.updatedAt.getTime()).toBeGreaterThan(Date.parse(before!.updatedAt));
|
||||||
|
});
|
||||||
|
|
||||||
test("reloadAgentSession cancels active run and resumes existing session once thread_started is observed", async () => {
|
test("reloadAgentSession cancels active run and resumes existing session once thread_started is observed", async () => {
|
||||||
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-reload-active-"));
|
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-reload-active-"));
|
||||||
const storagePath = join(workdir, "agents");
|
const storagePath = join(workdir, "agents");
|
||||||
|
|||||||
@@ -859,7 +859,12 @@ export class AgentManager {
|
|||||||
|
|
||||||
async setTitle(agentId: string, title: string): Promise<void> {
|
async setTitle(agentId: string, title: string): Promise<void> {
|
||||||
const agent = this.requireAgent(agentId);
|
const agent = this.requireAgent(agentId);
|
||||||
await this.registry?.setTitle(agentId, title);
|
const normalizedTitle = title.trim();
|
||||||
|
if (!normalizedTitle) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.touchUpdatedAt(agent);
|
||||||
|
await this.persistSnapshot(agent, { title: normalizedTitle });
|
||||||
this.emitState(agent);
|
this.emitState(agent);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -878,6 +883,7 @@ export class AgentManager {
|
|||||||
if (!agent || agent.internal) {
|
if (!agent || agent.internal) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
this.touchUpdatedAt(agent);
|
||||||
this.emitState(agent);
|
this.emitState(agent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -237,6 +237,7 @@ export async function generateAndApplyAgentMetadata(
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
await renameCurrentBranchImpl(options.cwd, normalizedBranch);
|
await renameCurrentBranchImpl(options.cwd, normalizedBranch);
|
||||||
|
options.agentManager.notifyAgentState(options.agentId);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
options.logger.warn(
|
options.logger.warn(
|
||||||
{ err: error, agentId: options.agentId, branch: normalizedBranch },
|
{ err: error, agentId: options.agentId, branch: normalizedBranch },
|
||||||
|
|||||||
@@ -19,6 +19,23 @@ const NON_GIT_CHECKOUT_STATUS = {
|
|||||||
ReturnType<NonNullable<AgentMetadataGeneratorDeps["getCheckoutStatus"]>>
|
ReturnType<NonNullable<AgentMetadataGeneratorDeps["getCheckoutStatus"]>>
|
||||||
>;
|
>;
|
||||||
|
|
||||||
|
const ELIGIBLE_WORKTREE_CHECKOUT_STATUS = {
|
||||||
|
isGit: true,
|
||||||
|
repoRoot: "/tmp/repo/metadata-worktree",
|
||||||
|
mainRepoRoot: "/tmp/repo",
|
||||||
|
currentBranch: "metadata-worktree",
|
||||||
|
isDirty: false,
|
||||||
|
baseRef: "main",
|
||||||
|
aheadBehind: null,
|
||||||
|
aheadOfOrigin: null,
|
||||||
|
behindOfOrigin: null,
|
||||||
|
hasRemote: false,
|
||||||
|
remoteUrl: null,
|
||||||
|
isPaseoOwnedWorktree: true,
|
||||||
|
} as Awaited<
|
||||||
|
ReturnType<NonNullable<AgentMetadataGeneratorDeps["getCheckoutStatus"]>>
|
||||||
|
>;
|
||||||
|
|
||||||
function createDeps(
|
function createDeps(
|
||||||
generateStructuredAgentResponseWithFallback: NonNullable<
|
generateStructuredAgentResponseWithFallback: NonNullable<
|
||||||
AgentMetadataGeneratorDeps["generateStructuredAgentResponseWithFallback"]
|
AgentMetadataGeneratorDeps["generateStructuredAgentResponseWithFallback"]
|
||||||
@@ -80,4 +97,51 @@ describe("agent metadata generator auto-title", () => {
|
|||||||
expect(generateStructured).not.toHaveBeenCalled();
|
expect(generateStructured).not.toHaveBeenCalled();
|
||||||
expect(setTitle).not.toHaveBeenCalled();
|
expect(setTitle).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("notifies agent state after successfully renaming a generated branch", async () => {
|
||||||
|
const setTitle = vi.fn().mockResolvedValue(undefined);
|
||||||
|
const notifyAgentState = vi.fn();
|
||||||
|
const manager = {
|
||||||
|
setTitle,
|
||||||
|
notifyAgentState,
|
||||||
|
} as unknown as AgentManager;
|
||||||
|
const renameCurrentBranch = vi
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue({
|
||||||
|
previousBranch: "metadata-worktree",
|
||||||
|
currentBranch: "feature/metadata-worktree",
|
||||||
|
}) as NonNullable<AgentMetadataGeneratorDeps["renameCurrentBranch"]>;
|
||||||
|
const generateStructured = vi.fn().mockResolvedValue({
|
||||||
|
branch: "feature/metadata-worktree",
|
||||||
|
}) as NonNullable<
|
||||||
|
AgentMetadataGeneratorDeps["generateStructuredAgentResponseWithFallback"]
|
||||||
|
>;
|
||||||
|
const getCheckoutStatus = vi
|
||||||
|
.fn()
|
||||||
|
.mockResolvedValue(ELIGIBLE_WORKTREE_CHECKOUT_STATUS) as NonNullable<
|
||||||
|
AgentMetadataGeneratorDeps["getCheckoutStatus"]
|
||||||
|
>;
|
||||||
|
|
||||||
|
await generateAndApplyAgentMetadata({
|
||||||
|
agentManager: manager,
|
||||||
|
agentId: "agent-branch",
|
||||||
|
cwd: "/tmp/repo/metadata-worktree",
|
||||||
|
initialPrompt: "Rename this worktree branch.",
|
||||||
|
explicitTitle: "Keep explicit title",
|
||||||
|
paseoHome: "/tmp/paseo-home",
|
||||||
|
logger,
|
||||||
|
deps: {
|
||||||
|
generateStructuredAgentResponseWithFallback: generateStructured,
|
||||||
|
getCheckoutStatus,
|
||||||
|
renameCurrentBranch,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(renameCurrentBranch).toHaveBeenCalledWith(
|
||||||
|
"/tmp/repo/metadata-worktree",
|
||||||
|
"feature/metadata-worktree"
|
||||||
|
);
|
||||||
|
expect(notifyAgentState).toHaveBeenCalledWith("agent-branch");
|
||||||
|
expect(setTitle).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -269,6 +269,42 @@ describe("AgentStorage", () => {
|
|||||||
expect(record?.lastStatus).toBe("running");
|
expect(record?.lastStatus).toBe("running");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("applySnapshot waits for in-flight writes before reading existing title", async () => {
|
||||||
|
const agentId = "agent-pending-write";
|
||||||
|
await storage.applySnapshot(createManagedAgent({ id: agentId }));
|
||||||
|
const initialRecord = await storage.get(agentId);
|
||||||
|
expect(initialRecord).not.toBeNull();
|
||||||
|
|
||||||
|
let releasePendingWrite: (() => void) | null = null;
|
||||||
|
const pendingWrite = new Promise<void>((resolve) => {
|
||||||
|
releasePendingWrite = resolve;
|
||||||
|
});
|
||||||
|
|
||||||
|
const storageInternals = storage as unknown as {
|
||||||
|
pendingWrites: Map<string, Promise<void>>;
|
||||||
|
cache: Map<string, any>;
|
||||||
|
};
|
||||||
|
storageInternals.pendingWrites.set(agentId, pendingWrite);
|
||||||
|
|
||||||
|
const applySnapshotPromise = storage.applySnapshot(
|
||||||
|
createManagedAgent({
|
||||||
|
id: agentId,
|
||||||
|
lifecycle: "running",
|
||||||
|
updatedAt: new Date("2025-01-02T00:00:00.000Z"),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
storageInternals.cache.set(agentId, {
|
||||||
|
...initialRecord!,
|
||||||
|
title: "Generated title",
|
||||||
|
});
|
||||||
|
releasePendingWrite?.();
|
||||||
|
|
||||||
|
await applySnapshotPromise;
|
||||||
|
const record = await storage.get(agentId);
|
||||||
|
expect(record?.title).toBe("Generated title");
|
||||||
|
});
|
||||||
|
|
||||||
test("list returns all agents including internal ones", async () => {
|
test("list returns all agents including internal ones", async () => {
|
||||||
// Create a normal agent
|
// Create a normal agent
|
||||||
await storage.applySnapshot(
|
await storage.applySnapshot(
|
||||||
|
|||||||
@@ -228,6 +228,7 @@ export class AgentStorage {
|
|||||||
options?: { title?: string | null; internal?: boolean }
|
options?: { title?: string | null; internal?: boolean }
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
await this.load();
|
await this.load();
|
||||||
|
await this.waitForPendingWrite(agent.id);
|
||||||
const existing = (await this.get(agent.id)) ?? null;
|
const existing = (await this.get(agent.id)) ?? null;
|
||||||
const hasTitleOverride =
|
const hasTitleOverride =
|
||||||
options !== undefined && Object.prototype.hasOwnProperty.call(options, "title");
|
options !== undefined && Object.prototype.hasOwnProperty.call(options, "title");
|
||||||
@@ -252,6 +253,7 @@ export class AgentStorage {
|
|||||||
|
|
||||||
async setTitle(agentId: string, title: string): Promise<void> {
|
async setTitle(agentId: string, title: string): Promise<void> {
|
||||||
await this.load();
|
await this.load();
|
||||||
|
await this.waitForPendingWrite(agentId);
|
||||||
const record = await this.get(agentId);
|
const record = await this.get(agentId);
|
||||||
if (!record) {
|
if (!record) {
|
||||||
throw new Error(`Agent ${agentId} not found`);
|
throw new Error(`Agent ${agentId} not found`);
|
||||||
@@ -377,6 +379,12 @@ export class AgentStorage {
|
|||||||
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async waitForPendingWrite(agentId: string): Promise<void> {
|
||||||
|
await (this.pendingWrites.get(agentId) ?? Promise.resolve()).catch(
|
||||||
|
() => undefined
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function projectDirNameFromCwd(cwd: string): string {
|
function projectDirNameFromCwd(cwd: string): string {
|
||||||
|
|||||||
Reference in New Issue
Block a user