Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions apps/ade-cli/src/adeRpcServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3042,19 +3042,16 @@ describe("adeRpcServer", () => {
expect(payload.rebaseStatus).toBe("idle");
});

it("records succeeded audit metadata for read-only tools", async () => {
it("does not record operation metadata for read-only action calls", async () => {
const { runtime, operationStart, operationFinish } = createRuntime();
const handler = createAdeRpcRequestHandler({ runtime, serverVersion: "test" });

await initialize(handler);
const response = await callTool(handler, "list_lanes", {});

expect(response.isError).toBeUndefined();
expect(operationStart).toHaveBeenCalledTimes(1);
expect(operationFinish).toHaveBeenCalledTimes(1);
const finishArgs = operationFinish.mock.calls[0]?.[0] ?? {};
expect(finishArgs.status).toBe("succeeded");
expect(finishArgs.metadataPatch?.resultStatus).toBe("success");
expect(operationStart).not.toHaveBeenCalled();
expect(operationFinish).not.toHaveBeenCalled();
});

// ---------- Rate limit tests ----------
Expand Down
1 change: 1 addition & 0 deletions apps/ade-cli/src/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,7 @@ export async function createAdeRuntime(args: {
};
automationService.bindAdeActionRegistry(adeActionLookup);

usageTrackingService.start();
runtimeCreated = true;
return runtime;
} finally {
Expand Down
30 changes: 20 additions & 10 deletions apps/desktop/src/main/services/cto/linearDispatcherService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2402,17 +2402,21 @@ export function createLinearDispatcherService(args: {
targetStatus: currentTargetStatus,
});
} else if (action === "approve") {
if (currentStepRow && currentStep?.type === "request_human_review") {
updateStep(currentStepRow.id, {
status: "completed",
completedAt: nowIso(),
payload: {
reviewState: "approved",
reviewerIdentityKey: reviewContext?.reviewerIdentityKey ?? null,
note: note ?? null,
},
});
if (run.status !== "awaiting_human_review") {
throw new Error("This workflow run is not awaiting supervisor review.");
}
if (!currentStep || currentStep.type !== "request_human_review" || !currentStepRow) {
throw new Error("This workflow run is not on a human review step.");
}
updateStep(currentStepRow.id, {
status: "completed",
completedAt: nowIso(),
payload: {
reviewState: "approved",
reviewerIdentityKey: reviewContext?.reviewerIdentityKey ?? null,
note: note ?? null,
},
});
updateRun(run.id, {
reviewState: "approved",
latestReviewNote: note ?? null,
Expand All @@ -2429,6 +2433,12 @@ export function createLinearDispatcherService(args: {
});
}
} else if (action === "reject") {
if (run.status !== "awaiting_human_review") {
throw new Error("This workflow run is not awaiting supervisor review.");
}
if (!currentStep || currentStep.type !== "request_human_review" || !currentStepRow || !reviewContext) {
throw new Error("This workflow run is not on a human review step.");
}
updateRun(run.id, {
reviewState: reviewContext?.rejectAction === "loop_back" ? "changes_requested" : "rejected",
latestReviewNote: note ?? "Rejected by reviewer.",
Expand Down
60 changes: 60 additions & 0 deletions apps/desktop/src/main/services/cto/linearSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2263,6 +2263,66 @@ describe("linearDispatcherService (file group)", () => {
db.close();
});

it.each(["approve", "reject"] as const)("rejects early %s before the supervisor review gate is active", async (action) => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "ade-linear-dispatcher-early-review-action-"));
const db = await openKvDb(path.join(root, "ade.db"), { debug() {}, info() {}, warn() {}, error() {} } as any);
const policy = buildSupervisedWorkerPolicy();

const dispatcher = createLinearDispatcherService({
db,
projectId: "project-1",
issueTracker: {
fetchIssueById: vi.fn(async () => issueFixture),
fetchWorkflowStates: vi.fn(async () => []),
updateIssueState: vi.fn(async () => {}),
addLabel: vi.fn(async () => {}),
createComment: vi.fn(async () => ({ commentId: "comment-1" })),
} as any,
workerAgentService: {
listAgents: vi.fn(() => [{ id: "agent-1", slug: "backend-dev", adapterType: "claude-local", capabilities: [] }]),
} as any,
workerHeartbeatService: {
triggerWakeup: vi.fn(async () => ({ runId: "worker-run-1" })),
listRuns: vi.fn(() => [{ id: "worker-run-1", status: "completed" }]),
} as any,
agentChatService: { ensureIdentitySession: vi.fn(), sendMessage: vi.fn(async () => {}), listSessions: vi.fn(async () => []) } as any,
laneService: {
ensurePrimaryLane: vi.fn(async () => {}),
list: vi.fn(async () => [{ id: "lane-1", laneType: "primary" }]),
create: vi.fn(async () => ({ id: "lane-2", name: "Fresh lane" })),
} as any,
templateService: { renderTemplate: vi.fn(() => ({ prompt: "Implement the issue." })) } as any,
closeoutService: { applyOutcome: vi.fn(async () => {}) } as any,
outboundService: createOutboundServiceMocks(),
workerTaskSessionService: {
deriveTaskKey: vi.fn(() => "task-key-1"),
ensureTaskSession: vi.fn(() => ({ id: "task-session-1" })),
} as any,
prService: {
getForLane: vi.fn(() => null),
createFromLane: vi.fn(async () => ({ id: "pr-1", githubPrNumber: 101 })),
} as any,
});

const run = dispatcher.createRun({ ...issueFixture, labels: ["workflow:backend-supervised"] }, buildMatch(policy));
await dispatcher.advanceRun(run.id, policy);

expect(run.id).toBeTruthy();
const detailBeforeReview = await dispatcher.getRunDetail(run.id, policy);
expect(detailBeforeReview?.run.status).not.toBe("awaiting_human_review");

await expect(
dispatcher.resolveRunAction(run.id, action, "Pre-resolved.", policy),
).rejects.toThrow("not awaiting supervisor review");
const detailAfterRejectedAction = await dispatcher.getRunDetail(run.id, policy);
expect(detailAfterRejectedAction?.run.status).toBe(detailBeforeReview?.run.status);
expect(detailAfterRejectedAction?.run.reviewState).not.toBe(action === "approve" ? "approved" : "rejected");

const awaitingReview = await dispatcher.advanceRun(run.id, policy);
expect(awaitingReview?.status).toBe("awaiting_human_review");
db.close();
Comment on lines +2321 to +2323
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Second advanceRun may not reach awaiting_human_review without a prior state change. After the blocked action, the run is still in whatever state the first advanceRun left it (e.g., awaiting_delegation or in_progress). If advanceRun is not idempotent for that intermediate state — or if additional external signals are required — the second call could return the same blocked status rather than awaiting_human_review, making the final assertion flaky. The mock returns listRuns: [{ status: "completed" }] which should drive progression, but explicitly asserting the intermediate state before the second advanceRun call would make the progression assumptions visible.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/services/cto/linearSync.test.ts
Line: 2321-2323

Comment:
**Second `advanceRun` may not reach `awaiting_human_review` without a prior state change.** After the blocked action, the run is still in whatever state the first `advanceRun` left it (e.g., `awaiting_delegation` or `in_progress`). If `advanceRun` is not idempotent for that intermediate state — or if additional external signals are required — the second call could return the same blocked status rather than `awaiting_human_review`, making the final assertion flaky. The mock returns `listRuns: [{ status: "completed" }]` which should drive progression, but explicitly asserting the intermediate state before the second `advanceRun` call would make the progression assumptions visible.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Codex Fix in Claude Code

});

it("loops back to delegated work when supervisor requests changes", async () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "ade-linear-dispatcher-loopback-"));
const db = await openKvDb(path.join(root, "ade.db"), { debug() {}, info() {}, warn() {}, error() {} } as any);
Expand Down
6 changes: 5 additions & 1 deletion docs/features/linear-integration/dispatch-and-sync.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ A run's steps advance via `currentStepIndex`. For each step type:
on timeout the step is marked failed with `review_timeout` and the
run advances rather than stalling. `rejectAction` drives the
rejection path (`cancel`, `reopen_issue`, or `loop_back` which
resets `currentStepIndex` to `loopToStepId ?? "launch"`).
resets `currentStepIndex` to `loopToStepId ?? "launch"`). Approve
and reject actions are only valid while the run status is
`awaiting_human_review` and the current step row is still a
`request_human_review` step; early or stale resolutions throw
without mutating `reviewState`.
- `emit_app_notification` — broadcasts a
`linear-workflow-notification` event via
`ctoLinearWorkflowEvent` IPC. The renderer listens in `CtoPage` and
Expand Down
2 changes: 1 addition & 1 deletion docs/features/remote-runtime/internal-architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Runtime event streaming uses `ade/actions/call` with `name: "stream_events"` for

Each `stream_events` response carries a per-runtime `eventEpoch` UUID minted when the daemon's `eventBuffer` is constructed. The preload event pump compares it against the last seen epoch for the active binding; if it changes (daemon restart, ssh reconnect to a fresh process) the cursor and dedup set reset and the next poll starts from `cursor=0`. The `startedAtMs` "drop events older than the pump start" filter is only applied to **local** bindings — remote pumps rely on the epoch reset instead, so older events backfilled after a reconnect are still delivered.

The remote event allowlist (`toRemoteRuntimeBufferedEvent`) accepts the runtime event categories desktop renders today: `category in {agent_chat, terminal, lane, pr, file_watch, process, test, project_state, orchestrator, dag_mutation, runtime, pty}`. The runtime additionally emits source-tagged events that preload routes to dedicated remote subscribers: `usage`, `usage_threshold`, `automation_event`, `conflict_event`, `github_status_changed`, `linear_workflow_event`, `feedback_submission_event`, `computer_use_event`, `ios_simulator_event`, `app_control_event`, and `macos_vm` (re-keyed to its `eventType`). ade-cli wires these into the runtime event buffer in `bootstrap.ts` so a remote-bound window sees the same usage, automation, conflict, GitHub, Linear, feedback, Computer Use, iOS Simulator, App Control, and macOS VM events as the local host.
The remote event allowlist (`toRemoteRuntimeBufferedEvent`) accepts the runtime event categories desktop renders today: `category in {agent_chat, terminal, lane, pr, file_watch, process, test, project_state, orchestrator, dag_mutation, runtime, pty}`. The runtime additionally emits source-tagged events that preload routes to dedicated remote subscribers: `usage`, `usage_threshold`, `automation_event`, `conflict_event`, `github_status_changed`, `linear_workflow_event`, `feedback_submission_event`, `computer_use_event`, `ios_simulator_event`, `app_control_event`, and `macos_vm` (re-keyed to its `eventType`). ade-cli wires these into the runtime event buffer in `bootstrap.ts` so a remote-bound window sees the same usage, automation, conflict, GitHub, Linear, feedback, Computer Use, iOS Simulator, App Control, and macOS VM events as the local host. Headless runtimes start `usageTrackingService` during `createAdeRuntime()` after the ADE action registry is bound, so the usage poller and threshold events run only once the runtime can answer the matching usage/budget actions.

## SSH transport

Expand Down
Loading