Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
41 changes: 40 additions & 1 deletion apps/ade-cli/src/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ import {
createSessionService,
STALE_RUNNING_SESSION_FRESH_ACTIVITY_GRACE_MS,
} from "../../desktop/src/main/services/sessions/sessionService";
import { createSettleTeardownWiring } from "../../desktop/src/main/services/sessions/settleTeardownWiring";
import type {
SettleResidueItem,
SettleTeardownContext,
SettleTeardownOutcome,
} from "../../desktop/src/main/services/sessions/sessionSettleTeardown";
import { createProjectConfigService } from "../../desktop/src/main/services/config/projectConfigService";
import { createConflictService } from "../../desktop/src/main/services/conflicts/conflictService";
import { createGitOperationsService } from "../../desktop/src/main/services/git/gitOperationsService";
Expand Down Expand Up @@ -755,7 +761,30 @@ export async function createAdeRuntime(args: {
// services. Session changes still use it once publishing is attached.
let pushPublisherForPtySignals: PushPublisherService | null = null;
let ptyServiceForSessionChanges: ReturnType<typeof createPtyService> | null = null;
const sessionService = createSessionService({ db });
// Late-bound: the chat service that owns the work is constructed further
// down. Without this the brain — which owns phone sync, remote commands and
// the PR-merge poller in a normal install — would settle sessions while
// stopping nothing.
const settleTeardownRef: {
run: ((sessionId: string, ctx: SettleTeardownContext) => Promise<SettleTeardownOutcome>) | null;
report: ((args: { columns: string[]; changesetSessionCount: number }) => void) | null;
residue: ((args: { provider: string | null; items: SettleResidueItem[] }) => void) | null;
} = { run: null, report: null, residue: null };
const sessionService = createSessionService({
db,
runSettleTeardown: async (sessionId, ctx) =>
settleTeardownRef.run ? await settleTeardownRef.run(sessionId, ctx) : { residue: [], confirmed: false },
onRemoteSettleWrite: (args) => settleTeardownRef.report?.(args),
onSettleResidue: (args) => settleTeardownRef.residue?.(args),
});
// Inbound settle-tuple writes get this host's lifecycle revision, so an
// in-flight settle can see a peer's decision and abandon rather than
// overwrite it. Registered here because the DB layer must not know what a
// settle means — and because the brain, not the desktop, is where changesets
// are actually applied in a normal install.
db.sync.setRemoteSettleTupleHandler((changes) => {
sessionService.reconcileRemoteSettleTuple(changes);
});
sessionService.onChanged((event) => {
pushEvent("runtime", { type: "terminal_session_changed", event });
const session = sessionService.get(event.sessionId);
Expand Down Expand Up @@ -1249,6 +1278,16 @@ export async function createAdeRuntime(args: {
countActiveForLane: (laneId) => agentChatService.countActiveForLane(laneId),
disposeForLane: (laneId) => agentChatService.disposeForLane(laneId),
};
const settleWiring = createSettleTeardownWiring({
agentChatService,
logger,
analytics: productAnalyticsService ?? null,
// The brain is the non-GUI runtime surface, matching its other analytics.
surface: "api",
});
settleTeardownRef.run = settleWiring.runSettleTeardown;
settleTeardownRef.report = settleWiring.onRemoteSettleWrite;
settleTeardownRef.residue = settleWiring.onSettleResidue;
}
autoRebaseActivityReady = true;
void autoRebaseService
Expand Down
39 changes: 38 additions & 1 deletion apps/desktop/src/main/main.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { app, BrowserWindow, dialog, ipcMain, Menu, nativeImage, Notification, powerMonitor, protocol, safeStorage, shell } from "electron";

Check warning on line 1 in apps/desktop/src/main/main.ts

View workflow job for this annotation

GitHub Actions / lint-desktop

'shell' is defined but never used. Allowed unused vars must match /^_/u

if (app.isPackaged && process.env.ADE_RUNTIME_PACKAGED === undefined) {
process.env.ADE_RUNTIME_PACKAGED = "1";
Expand Down Expand Up @@ -85,6 +85,8 @@
import { createOAuthRedirectService } from "./services/lanes/oauthRedirectService";
import { createRuntimeDiagnosticsService } from "./services/lanes/runtimeDiagnosticsService";
import { createSessionService } from "./services/sessions/sessionService";
import type { SettleResidueItem, SettleTeardownContext, SettleTeardownOutcome } from "./services/sessions/sessionSettleTeardown";
import { createSettleTeardownWiring } from "./services/sessions/settleTeardownWiring";
import { createSessionDeltaService } from "./services/sessions/sessionDeltaService";
import { createPtyService } from "./services/pty/ptyService";
import { createSupervisedPtyLoader } from "./services/pty/supervisedPtyHost";
Expand Down Expand Up @@ -210,7 +212,7 @@
import { localIpcListenOptions } from "../../../ade-cli/src/services/runtime/localIpcListenOptions";
import { normalizeProjectRootPath } from "../../../ade-cli/src/services/projects/projectRoots";
import {
ACCOUNT_SESSION_CREDENTIAL_KEY,

Check warning on line 215 in apps/desktop/src/main/main.ts

View workflow job for this annotation

GitHub Actions / lint-desktop

'ACCOUNT_SESSION_CREDENTIAL_KEY' is defined but never used. Allowed unused vars must match /^_/u
getSignedInAccountAccessToken,
} from "../../../ade-cli/src/services/account/accountAuthService";
import { createPushRelayClient } from "../../../ade-cli/src/services/push/pushRelayClient";
Expand Down Expand Up @@ -2870,10 +2872,34 @@
emitProjectEvent(projectRoot, IPC.lanesEnvEvent, ev),
});

const sessionService = createSessionService({ db });
// Late-bound: the chat service that owns the work does not exist yet at
// this point, and the settle path must not depend on construction order.
const settleTeardownRef: {
run: ((sessionId: string, ctx: SettleTeardownContext) => Promise<SettleTeardownOutcome>) | null;
report: ((args: { columns: string[]; changesetSessionCount: number }) => void) | null;
residue: ((args: { provider: string | null; items: SettleResidueItem[] }) => void) | null;
} = { run: null, report: null, residue: null };
const sessionService = createSessionService({
db,
onRemoteSettleWrite: (args) => settleTeardownRef.report?.(args),
onSettleResidue: (args) => settleTeardownRef.residue?.(args),
runSettleTeardown: async (sessionId, ctx) =>
settleTeardownRef.run
? await settleTeardownRef.run(sessionId, ctx)
// Before the chat service is up there is no background work to stop,
// so an empty teardown is the honest answer, not a skipped one.
: { residue: [], confirmed: false },
});
sessionService.onChanged((event) => {
emitProjectEvent(projectRoot, IPC.sessionsChanged, event);
});
// Inbound settle-tuple writes go through the chokepoint instead of landing
// raw, so a peer's decision gains this host's revision, settling window and
// abort semantics (R7). Registered here because the DB layer must not know
// what a settle means.
db.sync.setRemoteSettleTupleHandler((changes) => {
sessionService.reconcileRemoteSettleTuple(changes);
});
const processRegistry = createProcessRegistryService({
db,
logger,
Expand Down Expand Up @@ -3600,6 +3626,17 @@
countActiveForLane: (laneId) => agentChatService.countActiveForLane(laneId),
disposeForLane: (laneId) => agentChatService.disposeForLane(laneId),
};
{
const wiring = createSettleTeardownWiring({
agentChatService,
logger,
analytics: productAnalyticsService ?? null,
surface: "desktop",
});
settleTeardownRef.run = wiring.runSettleTeardown;
settleTeardownRef.report = wiring.onRemoteSettleWrite;
settleTeardownRef.residue = wiring.onSettleResidue;
}
autoRebaseActivityReady = true;
void autoRebaseService
.refreshActiveRebaseNeeds("activity_services_ready")
Expand Down
5 changes: 5 additions & 0 deletions apps/desktop/src/main/services/adeActions/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@
expect(isAllowedAdeAction("session", "requestSessionAttention")).toBe(true);
expect(isAllowedAdeAction("session", "setSessionStatusNote")).toBe(true);
expect(isAllowedAdeAction("session", "settleSession")).toBe(true);
// The residue read path. It was added to the CTO-only list but NOT to the
// allowlist, which silently refused every call — and left the settle design
// claiming a user-visible guarantee ("settled never quietly means something
// is still running") that nothing could actually reach.
expect(isAllowedAdeAction("session", "getSettleResidue")).toBe(true);
expect(isAllowedAdeAction("session", "unsettleSession")).toBe(true);
expect(isCtoOnlyAdeAction("session", "settleSession")).toBe(true);
expect(isCtoOnlyAdeAction("session", "unsettleSession")).toBe(true);
Expand Down Expand Up @@ -1887,7 +1892,7 @@

it("does not pretend a native CLI prompt was dismissed while its process is still blocked", async () => {
const settleSession = vi.fn(() => true);
const settleSessionReportingAbort = vi.fn(() => ({ found: true, settled: true }));

Check warning on line 1895 in apps/desktop/src/main/services/adeActions/registry.test.ts

View workflow job for this annotation

GitHub Actions / lint-desktop

'settleSessionReportingAbort' is assigned a value but never used. Allowed unused vars must match /^_/u
const setSessionRuntimeState = vi.fn(() => true);
const runtime = {
sessionService: {
Expand Down
15 changes: 15 additions & 0 deletions apps/desktop/src/main/services/adeActions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,7 @@ export const ADE_ACTION_ALLOWLIST: Partial<Record<AdeActionDomain, readonly stri
"get",
"getDelta",
"getLifecycleSettings",
"getSettleResidue",
"list",
"readTranscriptTail",
"requestSessionAttention",
Expand Down Expand Up @@ -2193,6 +2194,20 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null {
sessionService.unsettleSessions(sessionIds);
return { ok: true };
},
/**
* Work a settle could not confirm it stopped (design 3d option 3).
*
* Read-only, and the reason it exists: option 3 was signed off on the
* condition that the residue stay DISCOVERABLE rather than merely recorded.
* Without a read path, "settled" would quietly mean "and something may still
* be running" — the exact outcome the option was chosen to avoid.
*/
getSettleResidue: (args?: unknown) => {
const record = readObjectActionArg(args, "session.getSettleResidue");
const sessionId = typeof record.sessionId === "string" ? record.sessionId : "";
if (!sessionId) throw new Error("session.getSettleResidue requires sessionId.");
return sessionService.getSettleResidue(sessionId) ?? { recordedAt: null, items: [] };
},
// -----------------------------------------------------------------------
// Snooze / wake / settle-override. Snooze is a synced VISIBILITY overlay:
// it hides a row until its deadline without touching lifecycle columns, so
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ export function createCtoOperatorTools(deps: CtoOperatorToolDeps): Record<string
}),
execute: async ({ sessionId, outcome }) => {
try {
const result = deps.sessionService.settleSessionReportingAbort(sessionId, {
const result = await deps.sessionService.settleSessionReportingAbort(sessionId, {
...(outcome ? { outcome } : {}),
source: "operator",
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const STRING_PROPERTIES = new Set([
"duration_bucket", "error_kind", "route_kind", "connection_state", "drop_reason", "source", "mode",
"entry_point", "release_channel", "summary_kind", "reason", "last_command", "leg", "code",
"escalation_reason", "install_source", "trigger", "from_version", "to_version", "user_action",
"tool_error_kind", "crash_reason",
"tool_error_kind", "crash_reason", "count_bucket",
]);
const NUMBER_PROPERTIES = new Set([
"sent_count", "dropped_count", "interaction_count", "session_count", "chat_session_count",
Expand All @@ -120,6 +120,10 @@ const ANALYTICS_ONLY_ACTIONS = new Set([
"mention_expanded",
"transaction_failed",
"scope_selected",
// Settle teardown: work a settle could not confirm it stopped, and a peer
// settle-tuple write that had to be reconciled through the chokepoint.
"settle_teardown_residue",
"settle_remote_write_reconciled",
]);

const EVENT_PROPERTY_KEYS: Record<ProductAnalyticsEventName, ReadonlySet<string>> = {
Expand All @@ -132,7 +136,7 @@ const EVENT_PROPERTY_KEYS: Record<ProductAnalyticsEventName, ReadonlySet<string>
ade_project_opened: new Set(["route_kind", "source", "mode", "connection_state"]),
ade_feature_used: new Set([
"feature", "action", "outcome", "source", "mode", "provider", "model_family", "duration_bucket", "connection_state",
"bytes_freed", "files_compressed",
"bytes_freed", "files_compressed", "count_bucket",
]),
ade_work_session_started: new Set(["feature", "action", "outcome", "source", "mode", "provider"]),
ade_work_session_completed: new Set([
Expand Down Expand Up @@ -183,6 +187,9 @@ const SAFE_STRING_VALUES: Partial<Record<string, ReadonlySet<string>>> = {
outcome: new Set([
"success", "started", "completed", "failure", "timeout", "opened", "cancelled", "approved", "denied",
"partial", "failed", "idle_only", "immediate",
// Settle teardown could not confirm a stop (design 3d). `timeout` above
// covers the third case. Coarse on purpose: never the task or its error.
"no_stop_control", "rejected",
// Which half of a post-update transaction did not land. `swap` is
// deliberately absent: the app half is already reported by
// `ade_update_install_did_not_land`, so only the brain half is new signal.
Expand All @@ -194,12 +201,15 @@ const SAFE_STRING_VALUES: Partial<Record<string, ReadonlySet<string>>> = {
// widened, so the scope control can never carry free text.
"machine", "project", "account",
]),
provider: new Set(["codex", "openai", "claude", "cursor", "droid", "opencode", "pi", "gemini", "local", "other"]),
provider: new Set(["codex", "openai", "claude", "cursor", "droid", "opencode", "pi", "gemini", "lmstudio", "local", "other"]),
model_family: new Set([
"gpt_5", "openai_reasoning", "claude_sonnet", "claude_opus", "claude_haiku", "cursor", "gemini",
"grok", "local", "other",
]),
duration_bucket: new Set(["under_10s", "under_1m", "under_5m", "under_30m", "under_2h", "over_2h"]),
// Bucketed, never a raw count: a fleet that fails to stop must not become a
// high-cardinality dimension.
count_bucket: new Set(["1", "2_5", "6_plus"]),
route_kind: new Set(["desktop", "web"]),
connection_state: new Set(["connected", "disconnected", "pairing", "direct", "relay", "error"]),
drop_reason: new Set([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,42 @@ describe("product analytics producers", () => {
})).toMatchObject({ provider: "pi" });
});

it("keeps the settle-teardown properties through the sanitizer", () => {
// Both of these were silently dropped when first added: `action` is
// allowlisted separately from the event's key list, and `count_bucket` was
// registered in the key list and the value allowlist but never in the
// string-dispatch set, so it never reached either. The event still shipped,
// just anonymous — which is worse than not shipping, because the dashboard
// looks populated.
expect(sanitizeProductAnalyticsProperties("ade_feature_used", {
feature: "work",
action: "settle_teardown_residue",
outcome: "no_stop_control",
provider: "codex",
count_bucket: "2_5",
})).toEqual({
feature: "work",
action: "settle_teardown_residue",
outcome: "no_stop_control",
provider: "codex",
count_bucket: "2_5",
});

expect(sanitizeProductAnalyticsProperties("ade_feature_used", {
feature: "work",
action: "settle_remote_write_reconciled",
outcome: "partial",
})).toMatchObject({ action: "settle_remote_write_reconciled" });

// The bucket is still a closed set: a raw count must not slip through and
// widen the dimension.
expect(sanitizeProductAnalyticsProperties("ade_feature_used", {
feature: "work",
action: "settle_teardown_residue",
count_bucket: "37",
})).not.toHaveProperty("count_bucket");
});

it("maps automation completion and failed chat turns into canonical bounded outcomes", () => {
const captures: ProductAnalyticsCapture[] = [];
const analytics = settledAnalytics(captures);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ function createInMemoryAdeDb(): { db: AdeDb; raw: Database } {
rebuiltFts: false,
}),
discardUnpublishedChangesForTables: () => {},
setRemoteSettleTupleHandler: () => {},
},
flushNow: () => undefined,
close: () => raw.close(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function createInMemoryAdeDb(): AdeDb {
exportChangesSince: () => [],
applyChanges: () => ({ appliedCount: 0, dbVersion: 0, touchedTables: [], rebuiltFts: false }),
discardUnpublishedChangesForTables: () => {},
setRemoteSettleTupleHandler: () => {},
},
flushNow: () => {},
close: () => {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@ import {
isTrackedAgentCliToolType,
} from "../../../shared/types";
import { isChatToolType } from "../sessions/chatSessionProjection";
import type { SettleAbortedReason } from "../sessions/settlingStateRegistry";

/**
* The abort reasons that mean "something is running right now". Only these make
* a retry wait: the others either resolve on their own or, in the case of
* `teardown_failed`, describe work that is still running and still needs stopping.
*/
const ACTIVITY_ABORTS = new Set<string>(["turn_start", "turn_failed", "attention_requested"]);
const ACTIVITY_ABORTS: ReadonlySet<SettleAbortedReason> = new Set<SettleAbortedReason>([
"turn_start",
"turn_failed",
"attention_requested",
]);
import type { AgentChatSessionSummary } from "../../../shared/types";

function isMergeAtOrAfter(mergedAt: string | null | undefined, enabledSince: string): boolean {
Expand Down Expand Up @@ -264,7 +269,7 @@ export function createPrMergeAutoSettlementService(args: {
}
abortedSessionIds.delete(session.id);
}
const settleResult = args.sessionService.settleSessionsReportingAborts([session.id], {
const settleResult = await args.sessionService.settleSessionsReportingAborts([session.id], {
outcome: `PR #${pr.githubPrNumber} merged`,
settledAt: polledAt,
source: "pr_merge",
Expand All @@ -278,8 +283,10 @@ export function createPrMergeAutoSettlementService(args: {
// ONLY an activity abort waits for the turn to end. `teardown_failed`
// means the stop itself failed while the work kept running — making it
// wait for inactivity would never stop that work again, because the
// work is exactly what it would be waiting on. `lifecycle_changed` and
// `joined_in_flight` are momentary and clear on their own.
// work is exactly what it would be waiting on. `lifecycle_changed`,
// `joined_in_flight` and `remote_lifecycle_changed` are momentary and
// clear on their own; a peer's decision in particular has nothing to
// do with LOCAL inactivity, so waiting on it would be meaningless.
if (settleResult.aborted.some((entry) => ACTIVITY_ABORTS.has(entry.reason))) {
abortedSessionIds.add(session.id);
}
Expand Down
Loading
Loading