fix(sdk): stop idle ACP sessions re-scanning the session index every 2s (#4689)

packages/coding-agent/CHANGELOG.md

@@ -53,6 +53,8 @@
 - File tools no longer lose a just-written path or leave a 0-byte target when a write fails (#4734). `writethroughNoop` and LSP writethrough now publish through a sibling temp + rename (`writeFileAtomically`) so a permission/IO error cannot truncate the destination; `EACCES`/`EPERM`/`EROFS` surface as an actionable `ToolError` that says the original file is unchanged. Read tries the ACP `readTextFile` bridge when disk stat misses, and bridge failures fail closed: only an explicit `transport_unavailable`/`bridge_unavailable` code falls back to the agent host's disk, while structured denials and raw OS errno values such as `EPERM` do not, so a local read cannot bypass a remote client's access decision. Successful writes invalidate `fileReadCache`. Compaction-state now lists recent successful `write`/`edit`/`apply_patch`/`ast_edit` paths so a long-session compact does not silently drop in-flight file-tool context. This is independent of Windows directory-fsync `EPERM` (#4457) and of workflow-validation compaction (#4560).
 - Closed exact-head review findings on the #4734 atomic write path: the session-local trust boundary is now validated **before** any parent directory is created (a dangling symlink inside a trusted root resolves outside it, so creating parents first materialized an attacker-selected tree outside the sandbox before publication was refused, and the boundary is re-checked after `mkdir -p` follows existing symlinked ancestors); the publication parent is pinned by device/inode rather than realpath string, so a parent unlinked and replaced by a different directory at the same path is detected instead of published into; and the Windows in-place sharing fallback revalidates destination inode identity before mutating by pathname, refusing with `destUnchanged: true`/`not_published` when a concurrent writer substituted a successor during rename backoff. Read's ACP bridge fail-closed policy is now documented accurately: only explicit `transport_unavailable`/`bridge_unavailable` codes fall back to disk, never structured denials or raw OS errno.
 - Hardened the #4734 atomic write path after review: LSP writethrough awaited its `BunFile` write again (an unawaited call escaped the surrounding `try`, so a rejecting write was recorded as published and surfaced as an unhandled rejection), the Windows in-place sharing fallback now writes replacement and rollback bytes at absolute position 0 (`handle.writeFile()` resumes from the handle offset, so a partially accepted replacement left interleaved bytes while the result still reported `destUnchanged: true`), and the module contract no longer claims crash atomicity it does not provide. Publication is documented as last-writer-wins in `docs/tools/write.md`: identity is revalidated before the rename, but `rename(2)` commits against the pathname.
+
+- Fixed idle ACP sessions burning sustained CPU with no active turn (#4689). Every attached session's SessionRouter ran a 2s reconcile that unconditionally re-acquired the machine-global index lock and re-read, re-parsed, and re-checksummed the entire session index, then re-projected every historical row — O(total index history) per 2s tick per live session, forever, scaling a fresh session from ~2% (small index) to a sustained 30-70% of a core on an aged machine and degrading concurrent session creation through lock contention. The index now carries a change stamp (size/mtime/ctime over log + snapshot) so a poller proves "nothing changed" with two stats and reloads append-only changes through the tail reader, and the router's idle tick is just that stamp check plus a cheap transport-revive no-op: the full attach/retire body runs only on index changes, pending adoptions, or a 30s liveness sweep (well inside the index's own 2x60s heartbeat-freshness window). Measured idle CPU on a 15k-row index drops from a sustained ~30% to ~1% mean, with prompts unaffected (the pre-send reconcile is unchanged work when state actually changed). Because a running chat daemon builds its own SessionRouter, a pre-upgrade owner would keep the old hot polling loop: the Telegram daemon generation advances to 172 and its serving epoch to 88 (generation alone does not force replacement), and Discord/Slack generations 67/70 fence the same rollout.
 - Runtime skill discovery now scans `skills.customDirectories`. Session startup already loaded those directories through `loadSkills`, but `discoverRuntimeSkills` and `findRuntimeSkillByName` searched only the canonical project and user roots, so a configured custom skill was invocable by exact name yet absent from every `skill_discovery` search -- usable only by someone who already knew it existed. Both discovery entry points now scan the configured directories at user level (so project-scoped queries exclude them), deduplicated and tilde-expanded the same way `loadSkills` does. Naming a directory is explicit consent, so custom directories are not gated on `skills.trustUserSkills` -- matching the startup rule -- while the `skills.enabled` master switch still suppresses them.
 - A broker that cannot retain its own publication now names the object that withheld authority. The native layer opens `sdk`, `sdk/broker.lock`, `sdk/broker.lock/owner.json`, and `sdk/broker.json` no-follow and reports every refusal as one opaque `Retained broker publication authority is unavailable.`, so `gjc sdk` died with nothing to act on and the precondition could only be learned from the native source — a shared multi-account layout that symlinks the agent directory's `sdk` entry crashed every broker start this way. The failure is still fatal and still rolls back its publication; it now appends the first obstruction (missing entry, symlinked entry, wrong file kind, unreadable entry, or a non-fixed-width `heartbeatAt`) ahead of a bounded agent directory, so the named object survives the 512-character startup-failure reason, and stays verbatim when every precondition holds so a named condition is never invented. Each object is probed with the native's own access mode — the lock record read-only, only the published record read/write — and a file kind is only ever named through the open the native itself refuses, so a layout the native accepts is never reported as an obstruction; the published record is read through the descriptor the no-follow open already verified, never reopened by name. When rollback fails too, the aggregate message now carries the acquisition diagnostic, since the durable startup-failure marker persists only that message.
 - Retained broker publication probing now opens POSIX objects non-blocking, diagnoses exact-buffer malformed records, escapes control and bidi characters in persisted agent-directory diagnostics, and covers native-rejected wrong-kind objects without inventing a condition the native layer accepts.

packages/coding-agent/src/sdk/acp/adapter.ts

@@ -129,6 +129,21 @@ function isLifecycleOperation(operation: string): boolean {
  * Pure ACP-to-SDK adapter. It deliberately owns neither an AgentSession nor an
  * ACP bridge: all session work is performed through authenticated v3 frames.
  */
+/**
+ * Provider leases are renewed only through the maintenance capability (#4689).
+ * An attachment without it must be rejected at the admission boundary with an
+ * explicit migration error: falling back to `send()` would restore the 5s
+ * heartbeat-forced locked index rescan, and accepting it silently would leave
+ * live leases quietly un-renewed until they expire.
+ */
+function assertMaintenanceCapability(attachment: SessionAttachment): void {
+	if (typeof attachment.sendMaintenance === "function") return;
+	throw new AcpSdkAdapterError(
+		"operation_prohibited",
+		"SDK session attachment does not implement sendMaintenance(leaseId); provider leases cannot be renewed. Update the attachment implementation to the current SessionAttachment capability.",
+	);
+}
+
 export class AcpSdkAdapter {
 	readonly #client?: SdkClient;
 	readonly #router?: SessionRouter;
@@ -186,6 +201,11 @@ export class AcpSdkAdapter {
 	acceptAttachment(attachment: SessionAttachment): void {
 		if (!this.#router || attachment.sessionId !== this.#sessionId)
 			throw new AcpSdkAdapterError("invalid_input", "ACP attachment does not match this session adapter.");
+		// Reject an attachment that cannot renew provider leases at the handoff
+		// boundary (#4730 review), not just at start(): acceptAttachment is the
+		// single admission point for the replacement/ready paths too, so a
+		// capability-less replacement can never silently take over live leases.
+		assertMaintenanceCapability(attachment);
 		this.#abortActiveReverseRequests();
 		this.#attachment = attachment;
 		this.#connectionId = undefined;
@@ -204,6 +224,11 @@ export class AcpSdkAdapter {
 	}
 
 	async attachmentReady(attachment: SessionAttachment): Promise<void> {
+		// Guard BEFORE the branch (#4730 review): the same-object path below never
+		// reaches acceptAttachment, so guarding only inside that one arm let a
+		// current attachment without the maintenance capability activate providers
+		// and acquire leases it can never renew.
+		assertMaintenanceCapability(attachment);
 		if (this.#attachment !== attachment) this.acceptAttachment(attachment);
 		else {
 			this.#abortActiveReverseRequests();
@@ -253,6 +278,7 @@ export class AcpSdkAdapter {
 		}
 		if (this.#router) {
 			if (!this.#attachment?.isCurrent()) return;
+			if (this.#attachment) assertMaintenanceCapability(this.#attachment);
 			await this.#activateProviders();
 		} else {
 			await this.#activateProviders();
@@ -578,13 +604,32 @@ export class AcpSdkAdapter {
 		if (!attachment?.isCurrent()) throw new SessionRouterError("pre_send", "SDK session attachment is stale.");
 		await Promise.resolve(attachment.send(frame));
 	}
+	/** Lease heartbeats are idempotent maintenance: they skip the authority reconcile (#4689). */
+	async #sendLeaseHeartbeat(leaseId: string): Promise<void> {
+		if (!this.#router)
+			throw new AcpSdkAdapterError(
+				"operation_prohibited",
+				"Live session sends require the current Router attachment.",
+			);
+		const attachment = this.#attachment;
+		if (!attachment?.isCurrent()) throw new SessionRouterError("pre_send", "SDK session attachment is stale.");
+		// Fail closed when the capability is absent (#4730 review). Falling back to
+		// send() would put the 5s heartbeat back on the locked authority reconcile,
+		// which is the exact idle cost this fix removes.
+		if (typeof attachment.sendMaintenance !== "function")
+			throw new SessionRouterError(
+				"pre_send",
+				"SDK session attachment does not support provider-lease maintenance heartbeats.",
+			);
+		await Promise.resolve(attachment.sendMaintenance(leaseId));
+	}
 
 	async #heartbeatLeases(): Promise<void> {
 		if (this.#closed) return;
 		try {
 			if (!this.#router) return;
 			if (!this.#attachment?.isCurrent()) return;
-			for (const leaseId of this.#leases.values()) await this.#sendSession({ type: "provider_heartbeat", leaseId });
+			for (const leaseId of this.#leases.values()) await this.#sendLeaseHeartbeat(leaseId);
 		} catch (error) {
 			if (error instanceof SessionRouterError && error.phase === "pre_send") return;
 			this.#reportReconnectFailure(error);