perf: cut startup cost across natives, providers, plugins and MCP (W1-W6) - #3846
perf: cut startup cost across natives, providers, plugins and MCP (W1-W6)#3846Yeachan-Heo wants to merge 27 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a28670d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function readBundledContentSync(entry: BundledGjcSkillCatalogEntry): string { | ||
| const sourcePath = sourcePathForBundledEntry(entry); | ||
| try { | ||
| return readFileSync(sourcePath, "utf8"); |
There was a problem hiding this comment.
When this runs from a compiled Bun executable, sourcePath resolves under /$bunfs/...; literal text imports are available there, but readFileSync/Bun.file against that path returns ENOENT. Because content on the exported bundled default definitions and embedded skills now calls this getter, any compiled-binary caller that still reads definition.content/skill.content instead of the new async loadContent() loses the bundled workflow skill body. Keep the text behind the generated import loaders/cache rather than reading the source path synchronously.
AGENTS.md reference: AGENTS.md:L112-L122
Useful? React with 👍 / 👎.
| } | ||
| if (isManagedOwnerSupervisorArgv(argv)) { | ||
| if (argv.length === 1 && argv[0] === MANAGED_OWNER_SUPERVISOR_ARG) { | ||
| const { runManagedOwnerSupervisor } = await import("./gjc-runtime/managed-owner-supervisor"); |
There was a problem hiding this comment.
For the internal --internal-managed-owner-supervisor invocation, this lazy import leaves a startup window before managed-owner-supervisor.ts's module-scope SIGTERM capture is installed. If the pane close flow sends SIGTERM while the CLI is still loading or before this branch imports the module, the supervisor exits via the default handler instead of preserving bootstrapSigtermPending and relaying/observing the exact child termination, so managed-owner recovery loses its trusted terminal evidence. Keep a tiny early handler or load the supervisor guard before normal CLI initialization.
AGENTS.md reference: AGENTS.md:L101-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a28670d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| options.onNotification?.(method, params); | ||
| this.emit(entry, { type: "notification", method, params }); |
There was a problem hiding this comment.
When a shared MCP entry receives a notification, this calls the first acquirer's onNotification directly and then emits the same event to every lease, including that first lease. Its manager therefore processes tools/list_changed, resources/list_changed, and similar events twice, causing duplicate refresh requests and duplicate consumer callbacks whenever it owns the physical connection; route notifications exclusively through the lease fan-out.
Useful? React with 👍 / 👎.
| const plan = planToolOutputPrune(branchEntries, { | ||
| ...DEFAULT_PRUNE_CONFIG, | ||
| minimumSavings: overThreshold ? 0 : DEFAULT_PRUNE_CONFIG.minimumSavings, | ||
| }); | ||
| const artifactManager = this.sessionManager.getArtifactManager(); |
There was a problem hiding this comment.
For a non-persistent/SDK session with no session file and no previously saved artifact, getArtifactManager() returns null; consequently nothing is added to published, the committed plan is empty, and stale tool outputs are never pruned. Long ephemeral sessions therefore fall through to compaction or context overflow even though ensureArtifactManager() can create the supported temporary artifact store; establish that store before publishing prune candidates.
Useful? React with 👍 / 👎.
| this.#connectionDisposer?.(); | ||
| this.#capabilitiesDisposer?.(); | ||
| this.#malformedDisposer?.(); |
There was a problem hiding this comment.
When the same SessionSdkSessionRuntime is stopped and then started again, these one-time disposers permanently remove the connection-close, capability-negotiation, and malformed-frame handlers, while start() never registers replacements. The restarted runtime consequently retains reverse leases after disconnects, ignores negotiated capabilities, and stops returning structured malformed-frame errors; either make the runtime explicitly terminal after stop or recreate these subscriptions on every start.
Useful? React with 👍 / 👎.
38a2867 to
2f2e32c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f2e32c536
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| !/\.(?:optional|nullable)\s*\(\s*\)\s*$/.test(field[2]!) && | ||
| !/\.Optional\s*\(\s*\)\s*$/.test(field[2]!) && | ||
| !/(?:Type\.)?Optional\s*\(/.test(field[2]!) |
There was a problem hiding this comment.
For plugin tools whose schema is inferred from source, a declaration such as z.string().nullable() is classified as optional here. Nullable fields may contain null, but their presence is still required, so the published schema permits the model to omit a value that the plugin's runtime validator rejects; the static expression parser also reduces this chained expression to the underlying string schema and therefore rejects the valid null value. Treat only optional wrappers as removing a property from required, while preserving nullability in the property schema.
Useful? React with 👍 / 👎.
| const runtime = await this.#workspaceTreeService.get("first-turn-barrier"); | ||
| this.#cachedWorkspaceTree = firstWorkspaceTree ? runtime.snapshot : await runtime.refresh(); |
There was a problem hiding this comment.
With workspaceTree.mode="lazy", any native scan error or filesystem failure from the service now escapes this first-turn barrier and rejects prompt() instead of omitting the workspace tree as the fallback branch and the previous implementation do. Because LazyService retains the failed state, subsequent prompts keep rejecting as well, making an optional context scan permanently disable the session; catch scan/service failures here and continue with an undefined tree.
Useful? React with 👍 / 👎.
| const endpoint = await this.transport.start(); | ||
| this.#transportStarted = true; | ||
| return endpoint; |
There was a problem hiding this comment.
If stop() runs while transport.start() is pending, the transport shutdown path waits for that start and then closes the newly created server, but this continuation still sets #transportStarted to true and returns its endpoint. The start caller can therefore receive a URL whose endpoint file/server has already been removed, while the runtime incorrectly reports the transport as started; coordinate start and stop with a generation or shutdown flag so an in-flight start cannot publish success after shutdown begins.
Useful? React with 👍 / 👎.
2f2e32c to
ad2110b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad2110b2fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const localBackend: MemoryBackend = { | ||
| id: "local", | ||
| async start(options) { | ||
| return (await import("./local-backend")).localBackend.start(options); |
There was a problem hiding this comment.
Replace this await import() path, along with the other method-level imports in these backend facades, with the repository's approved top-level/static loading pattern. The repository contract explicitly prohibits inline imports, and these calls make the memory backend's dependency and compiled-binary loading behavior depend on runtime method execution rather than the statically inspectable module graph.
AGENTS.md reference: AGENTS.md:L103-L106
Useful? React with 👍 / 👎.
| const plan = planToolOutputPrune(branchEntries, { | ||
| ...DEFAULT_PRUNE_CONFIG, | ||
| minimumSavings: overThreshold ? 0 : DEFAULT_PRUNE_CONFIG.minimumSavings, | ||
| }); |
There was a problem hiding this comment.
When below-threshold pruning is near minimumSavings, this plan is built without artifactRefMaxChars, so admission assumes the replacement has no artifact URI; the method then appends the published URI and can commit even after actual savings fall below the configured minimum, because the optional commit gate checks only cache-reset cost. This can trigger a history rewrite and provider-cache reset for a prune that the documented minimum-savings gate should have rejected; build the committed plan with the same artifact-reference budget used by the preflight estimate or recheck the final savings against the minimum.
Useful? React with 👍 / 👎.
ad2110b to
5933677
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5933677db4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| active = { runtime, revisions, cursors, reconciliation, pending, disposeGate }; | ||
| try { | ||
| await runtime.start(); |
There was a problem hiding this comment.
When notifications are disabled—the default for ordinary top-level sessions—createAgentSession selects this SDK-only extension, but startup ends after runtime.start() and never calls the available runtime.registerWithBroker(...) path. Unlike the previous notification-host path, no host_registered record reaches SessionIndex, so broker-backed session.list, SDK attach, and relay commands cannot discover these otherwise-live endpoints.
Useful? React with 👍 / 👎.
| await filesystem.writeFile( | ||
| endpointFile, | ||
| JSON.stringify({ version: 1, url, token: input.token, pid: process.pid }), | ||
| "utf8", |
There was a problem hiding this comment.
For sessions using the new WebSocket transport, this endpoint record omits sessionId. Broker endpoint resolution explicitly rejects records unless endpoint.sessionId === record.sessionId (sdk/broker/broker.ts:784-791), and lifecycle readiness applies the same check, so even after the SDK-only host is indexed, broker attach/readiness treats its endpoint as stale. Publish input.sessionId in the discovery JSON, matching the existing endpoint contract.
Useful? React with 👍 / 👎.
| const bindings = nativeThemeBindings; | ||
| loadNativeThemeBindings(); | ||
| const validLang = bindings && lang && bindings.supportsLanguage(lang) ? lang : undefined; | ||
| if (!bindings) return code.split("\n").map(line => theme.fg("mdCodeBlock", line)); |
There was a problem hiding this comment.
On non-macOS hosts and in non-interactive/print mode, theme initialization does not preload the native bindings. The first code block therefore captures undefined, starts an asynchronous load, and immediately returns unhighlighted text; print mode has no subsequent render, so syntax highlighting remains absent despite syntaxHighlighting.enabled defaulting to true. Await the binding load before one-shot rendering or trigger a rerender when it completes.
Useful? React with 👍 / 👎.
경고 — 이 PR의 현재 head가 CHANGELOG 전체를 삭제한다머지하면 안 된다. 확인된 사실: 1바이트 — 개행 하나만 남았다. 원인은 내 쪽이다#3932(11:25:32Z 머지)가 그런데 그 결과 리베이스에서 CHANGELOG가 처음으로 진짜 충돌을 내기 시작했고, 그 충돌을 해소하는 과정에서 파일이 비워졌다. 시간대가 명확하다:
전환 비용을 예고하지 못한 건 내 잘못이다. 미안하다. 복구git fetch origin
git checkout origin/dev -- packages/coding-agent/CHANGELOG.md # 해당 패키지 경로로
# 그 다음 ## [Unreleased] 아래에 이 PR의 항목만 다시 추가
git add packages/coding-agent/CHANGELOG.md
git commit --amend --no-edit # 또는 새 커밋앞으로 리베이스에서 CHANGELOG 충돌이 나면 양쪽 항목을 모두 푸시 전에 다음으로 자가 점검할 수 있다: git cat-file -s HEAD:packages/coding-agent/CHANGELOG.md # 30만 바이트 근처여야 정상 |
5933677 to
3b5de5c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b5de5cbe7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (range !== null) { | ||
| content = await file.slice(start, boundedEnd).text(); |
There was a problem hiding this comment.
For an artifact larger than 16 MiB, an explicit open-ended or wide query such as ?range=0- enters this branch and materializes the entire file, completely bypassing the ceiling applied to bare reads. Large spilled artifacts can therefore allocate hundreds of MiB or more and crash the session; clamp every requested range to a bounded maximum before calling text().
Useful? React with 👍 / 👎.
| const stopResult = current.stop(true); | ||
| await Promise.race([stopResult, new Promise<void>(resolve => setTimeout(resolve, 250))]); |
There was a problem hiding this comment.
When current.stop(true) resolves before 250 ms, Promise.race drops the timeout promise but does not cancel its timer. Bun keeps that timer alive, so an otherwise-complete SDK shutdown can hold process exit open for the remainder of the 250 ms window; use a cancellable timeout and clear it when the stop settles.
AGENTS.md reference: AGENTS.md:L108-L109
Useful? React with 👍 / 👎.
| void this.#ensureHistoryStorage().then(storage => { | ||
| if (storage && this.#history.length > 0) this.#navigateHistory(direction); | ||
| this.invalidate(); |
There was a problem hiding this comment.
When history has not loaded yet, pressing Up starts this asynchronous load; if the user types or otherwise changes the draft before it resolves, the continuation still calls #navigateHistory(direction) and replaces the newer draft with an old prompt. Capture the editor/document state at the initial keypress and only navigate if it is still unchanged when loading completes.
Useful? React with 👍 / 👎.
3b5de5c to
9549e2f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9549e2fe72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const maxDepth = session.settings.get("task.maxRecursionDepth") ?? 2; | ||
| return maxDepth < 0 || (session.taskDepth ?? 0) < maxDepth; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
When tools.discoveryMode uses its default all value and Telegram notifications are disabled, telegram_send falls through to this unconditional true, so createTools() publishes a deferred LazyAgentTool without running TelegramSendTool.createIf(). The model can then discover and activate the tool, but its first execution loads a null implementation and fails with the generic “failed to load” error. Mirror the provider-enabled predicate from tools/telegram-send.ts in descriptor availability so the discovery catalog does not advertise an unavailable capability.
Useful? React with 👍 / 👎.
PR #3846 altered protected Telegram lifecycle symbols (defaultProcessReference, exactUnlinkNotificationFile, process-incarnation authority). The daemon generation guard requires a strict generation bump for these changes. Generation 54 covers the startup-cost refactoring that moved these symbols off the eager import path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 099918b3cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "initTheme:final", | ||
| deps.initTheme ?? initTheme, | ||
| isInteractive, | ||
| isInteractive && settingsInstance.get("theme.watchFiles"), |
There was a problem hiding this comment.
When an interactive user sets theme.watchFiles=false, this passes false as initTheme's general enableWatcher argument. That argument also gates startSigwinchListener() and the macOS appearance observer in theme.ts, so disabling custom-theme file reloads unexpectedly disables automatic dark/light theme detection and subsequent terminal appearance updates. Keep appearance monitoring enabled for interactive sessions and gate only the file watcher with this setting.
Useful? React with 👍 / 👎.
The RSS harness writes per-commit baselines and per-run reports under .gjc/rss-checkpoints/. These are host-specific measurements (hundreds of KB of sampled process-tree RSS) that differ on every machine and every run, so they are runtime state rather than source. Lore-id: a7c31e08 Constraint: the harness must keep resolving .gjc/rss-checkpoints/<commit>.json -- ignore the directory, do not relocate it Confidence: high Scope-risk: narrow Reversibility: easy Tested: git check-ignore resolves the canonical baseline and the last-run report Not-tested: CI runners that may expect a committed baseline
…ation adapters Hosting an SDK session pulled the Telegram/Discord/Slack adapters, and through them a native dependency, onto the startup module graph even when notifications were inactive. Splitting the bus into a session runtime (control/query/replay via SessionSdkHost) plus adapters behind a LazyService keeps the adapters off the graph until an eligibility check actually activates one. Lore-id: 4f19b2ac Constraint: SDK control/query/replay contract must stay byte-identical -- surface parity is manifest-pinned Constraint: adapters must still activate under every currently-activating condition with identical event ordering Rejected: leaving adapters statically imported and gating only at call time | the native import already happened by then Rejected: package-name dynamic import for the adapters | crashes bun --compile; only statically-traceable in-function require() survives Confidence: high Scope-risk: wide Reversibility: reversible Directive: keep adapter access behind LazyService; a bare top-level import silently re-adds the native to startup Tested: module-trace deny of the adapter modules and their native on an idle session; surface parity manifest; hostile-error paths Not-tested: live delivery against real Telegram/Discord/Slack endpoints
…across facades Every session opened its own child per MCP server, and the manager talked to the physical connection directly, so lifecycle, retry, restart and credential scope had no boundary to live on. This introduces a connection pool with lease-backed facades: tools, resources, prompts, catalog refreshes and prepared slash/TUI commands all route through the owning lease and fail their live-state check once it is released. Sharing is honored for tools-only stdio and for HTTP/SSE, where one physical MCP session and one callback stream serve every lease with per-lease demultiplexing. Endpoint identity follows C5 exactly: raw path and query are preserved (trailing slash, reserved and unreserved percent encoding, parameter order, duplicate keys, empty vs absent), only universally equivalent forms collapse, and an Authorization-bearing entry must declare a non-secret binding kind and scope before it may share. Restart is owned by the pool: exactly one facade reconnects, every surviving lease is rebound, and each physical entry carries a monotonic generation that both acquire-to-register paths verify immediately before registration so a retired entry can never be published. Lore-id: 91d4e7b3 Constraint: never replay an in-flight tools/call -- recovery may reconnect, but the failed call must surface typed to its calling lease Constraint: exactly one restart owner per shared entry; peers rebind rather than racing a second reconnect Constraint: never hash an Authorization value into the pool key Rejected: retrying the shared call after recovery | duplicates server-side side effects on an uncertain response Rejected: best-effort replacement events without a durable generation | a lease acquired just before a rotation has no listener and pins a retired entry Rejected: sharing unbound Authorization headers | second facade silently reuses the first transport's credential across tenants Confidence: high Scope-risk: wide Reversibility: reversible Directive: any future acquire-to-register path must re-check pool.isCurrentLease() immediately before registration Tested: two facades in one process over one child; release/idle reap; -32601 sampling and elicitation confinement; crash isolation; roots union add/remove on the wire; endpoint partition and equivalence; second rotation inside the acquire-to-register window on both paths Not-tested: two top-level AgentSessions sharing a pool -- runtime-scope isolation is a later milestone and stays unauthorized
Forty modules imported @gajae-code/natives at the top level, so the addon was resolved and mapped during startup even for `gjc --help`. Each call site now takes the binding through an in-function require at the point of use, which keeps the addon off the startup graph without changing any behavior once a native call actually happens. Lore-id: 6b820fd1 Constraint: the require specifier must be a literal string in the function body -- createRequire and package-name import() both crash bun --compile Rejected: a single lazy wrapper module re-exported everywhere | the wrapper itself becomes a static edge and the addon loads again Confidence: high Scope-risk: wide Reversibility: easy Directive: never add a top-level `import ... from "@gajae-code/natives"`; the help/idle trace gate denies it Tested: module-trace help scenario with @gajae-code/natives denied; compiled binary --version and --smoke-test Not-tested: non-darwin-arm64 native variants
The package root barrel pulled every provider module, so importing a single type dragged the whole provider set onto the graph. A /core subpath exposes the runtime-neutral surface (descriptors, auth storage, shared types) while providers stay reachable only through their own subpaths and the root barrel, which external consumers still depend on. Lore-id: 2e5aa93c Constraint: keep the root barrel intact -- external consumers import from it and must not break Constraint: subpath requires must work under bun --compile, which is why ./* and ./providers/* carry require conditions Rejected: pruning the root barrel to force everyone onto /core | silently breaks published consumers Confidence: high Scope-risk: moderate Reversibility: reversible Tested: packages/ai suite env-scrubbed; dual-package identity checked absent; providers denied on the help and idle traces Not-tested: downstream repos consuming the published tarball
…kage root Consumers inside coding-agent imported from @gajae-code/ai, which meant every one of them re-attached the provider set to the module graph. These call sites now use @gajae-code/ai/core, leaving provider construction to the paths that genuinely build a provider. Lore-id: 8c40df67 Constraint: packages/coding-agent/src/** must not import bare @gajae-code/ai -- enforced by a biome noRestrictedImports override and an import-gate test Confidence: high Scope-risk: wide Reversibility: easy Directive: new coding-agent code imports @gajae-code/ai/core; the gate test fails the build otherwise Tested: ai-core-import-gate test; help and idle module traces with packages/ai/src/providers/** denied Not-tested: nothing material -- this is a mechanical specifier change
AgentTelemetryConfig conflated span emission with usage and cost accounting, so a spans-disabled run still resolved span attributes on every step. Splitting them means resolveAttributes is never called and attributes stay undefined under spans:false, while usage and cost hooks keep firing and the run collector still reports real summaries with exactly-once onRunEnd across success, failure and tool-bearing runs. Lore-id: d5e37a10 Constraint: usage and cost hooks must keep firing with spans off -- billing depends on them Constraint: onRunEnd fires exactly once per run, span or no span Rejected: keeping one flag and skipping only the exporter | attribute resolution was the actual cost Confidence: high Scope-risk: moderate Reversibility: easy Tested: OTEL suite asserting attributes undefined under spans:false, plus exactly-once success/error/tool coverage; packages/agent suite Not-tested: a live OTLP collector
…thority Plugin tools, subskills and prompt appendices were resolved from more than one place, so a path validated by the registry could be replaced before it was used. Registry v2 becomes the sole authority: it checks identity, scope and digest, confines paths by realpath under the plugin root, honors disabled and quarantined state, and rejects path-only persisted records. Legacy roots migrate only inside the locked registry transaction, and auto-migration never executes plugin code. Every final use is now verified at the moment of use. Tool and subskill imports re-check their exact reference immediately before importing, injection carries the verified bytes rather than re-reading the file, and prompt appendices hash their body -- file-backed and inline alike -- before it can reach the prompt, failing closed with a typed runtime_mismatch on drift. Lore-id: 7a1c9e54 Constraint: one runtime path per plugin entry -- a failed v2 load must never fall back to v1 Constraint: the legacy loader must be unreachable from the published package, not merely absent from the barrel Rejected: validating once at registry load and trusting the path later | leaves a mutation window before every import and render Rejected: dropping the loader from the barrel only | the ./extensibility/* export wildcard still resolved its subpath Confidence: high Scope-risk: wide Reversibility: migration-needed Directive: any new plugin-supplied content must be digest-verified at final use, not at discovery Tested: mutation-between-validation-and-use races for tools, subskills, system and agent appendices; forged delimiter and tag injection; packed-consumer resolution failure for both legacy loader spellings; preview/apply classification matrix Not-tested: third-party plugins in the wild that depend on v1 record shapes
…alidation The legacy batch adapter had drifted from the native path: thrown steps lost their failureCode and failureIndex, timeoutMs was ignored so a non-terminating action ran unbounded, and batch dispatch reached the native seam without the pre-dispatch bounds check that the single-action path applies. A batch could therefore act on coordinates the single path would reject. Lore-id: 3fb6c082 Constraint: batch and single dispatch must agree on failure shape, deadline and coordinate validation Rejected: treating the divergence as legacy-path tolerance | it silently widened what a batch may do Confidence: high Scope-risk: narrow Reversibility: easy Tested: batch step failure mapping, deadline cancellation, and COMPUTER_COORD_INVALID on the batch path; the seven-case enforcement suite Not-tested: real display hardware -- the controller is driven through its test seam
The tool index imported every implementation to expose descriptors, so listing tools pulled their whole dependency set. Descriptors now live apart from implementations behind a generated catalog, letting discovery read metadata without constructing anything. Lore-id: 5d92ae3b Constraint: the generated catalog must stay in sync with its generator -- the literal-catalog trace gate enforces it Confidence: high Scope-risk: moderate Reversibility: easy Directive: change the generator, then regenerate; do not hand-edit tool-catalog.generated.ts Tested: literal catalog gates for tools and skills; tool-discovery initial-tools suite Not-tested: nothing material beyond the catalog gates
Session construction eagerly built artifact and history storage even for runs that never touched either. Both are now created on first use, which keeps their dependencies off the startup path. Lore-id: c6e14b09 Constraint: storage identity must stay stable across the deferral -- a session that later writes must land in the same place it would have Confidence: high Scope-risk: moderate Reversibility: easy Tested: agent-session suites covering concurrency, handoff, mid-run maintenance, detached bash, todos and replay Not-tested: resume from sessions written by older builds
…p path Follow-through for the laziness milestones across the surfaces the earlier commits exposed: MCP capability and CLI entry points, skill discovery and defaults, eval executors, and the settings and schema surfaces that describe them. Each moves construction behind first use or narrows an import so the startup graph stops paying for work a given invocation never performs. Lore-id: 0b7fd253 Constraint: compatibility defaults must not shift -- workspaceTree.mode stays eager and startup.networkPrewarm stays true in this change Rejected: flipping the defaults here to bank the win | that is a separate authorized milestone and would change behavior without review Confidence: medium Scope-risk: wide Reversibility: reversible Tested: coding-agent source suites; help full-deny, help+idle bun:sqlite and provider deny, and literal catalog trace gates; check:schemas Not-tested: every downstream consumer of the touched settings shapes
…ubpath Two boundaries needed manifest and lint enforcement rather than convention. Biome now rejects bare @gajae-code/ai imports inside packages/coding-agent/src, and the coding-agent export map null-blocks the legacy GJC plugin loader subpath -- both spellings, ordered before the recursive ./extensibility/* wildcard that would otherwise resolve it. packages/ai carries require conditions on ./* and ./providers/* so subpath requires resolve under bun --compile. Lore-id: e94a5b71 Constraint: null export entries must precede the wildcard; after it they are dead Constraint: the root barrel stays intact for external consumers Rejected: relying on review to catch bare @gajae-code/ai imports | the graph regression is invisible without a gate Confidence: high Scope-risk: moderate Reversibility: easy Tested: child-process package-subpath resolution failure for both loader spellings; packed SDK smoke at root 381 / sdk 39; restricted-import lint Not-tested: consumers already importing the legacy loader subpath -- they break by design
The laziness work needed measurable gates rather than reviewer judgement. verify-module-trace asserts which modules a scenario may load and pins the literal tool and skill catalogs; verify-rss-checkpoints measures whole-process-tree RSS at an explicit barrier and compares against a per-commit baseline with floor enforcement and schema-bound re-scope records. Omitting --baseline with --compare resolves only the canonical .gjc/rss-checkpoints/<commit>.json and otherwise fails closed with a typed BaselineDefaultMissing naming that path. S6 is emitted as deferred with its authorization reason instead of being measured or fabricated. Lore-id: b30c8fd4 Constraint: never fall back to <commit>.last-run.json -- a non-baseline run overwrites it, so comparing against it makes the gate oscillate between pass and fail on identical invocations Constraint: no implicit --allow-baseline-drift; drift stays an explicit opt-in Rejected: same-commit last-run fallback | observed flipping exit 0 and FAIL across back-to-back identical runs Rejected: fabricating an S6 measurement to fill the table | the scenario needs a daemon that does not exist yet Confidence: high Scope-risk: moderate Reversibility: easy Directive: stable-tree RSS at the barrier is the deciding metric; sampled peak stays diagnostic Tested: harness-gates covering canonical resolution, refusal of same-commit and foreign last-run fallbacks, malformed baselines, deferred S6 shape, and explicit --baseline precedence Not-tested: CI hardware -- S1 and S7 vary widely under load on this host
The session-runtime extraction also flipped the broker's no-cleanup session.delete fallback from an idempotent ok to invalid_input, which breaks the manifest-pinned adapter disposition contract (AD-*-G07) for every machine adapter. Deleting an unknown session stays a successful no-op. Lore-id: 7c41a9e2 Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun test packages/coding-agent/test/sdk-adapter-dispositions.test.ts (576 pass)
src/tools/tool-catalog.test.ts imported ../../scripts/generate-tool-catalog, so the publish type check emitted a stray scripts/generate-tool-catalog.d.ts into the package and the next `biome check .` failed on it. The test belongs with the other tool tests, where importing scripts/ is already normal. Lore-id: b83d5510 Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun test packages/coding-agent/test/tools/tool-catalog.test.ts (3 pass)
chat-daemon-control.ts moved its native process and unlink access behind lazy bindings, so the semantic declaration digests the guard pins had to be regenerated with --write-manifest. Lore-id: 5a2f77c1 Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun scripts/telegram-daemon-generation-guard.ts --validate-current-tree
`biome check .` was clean on dev and reported 225 errors on this branch: unformatted files, unsorted imports, and refactor residue — dead `import type * as native` aliases, an unused DiffQueryError copy in sdk/bus, an orphaned #ensureDir, unused native type aliases, and a cancelPendingEntry parameter no caller needs. findParametersExpression kept a while-assign loop that could only ever run once, and safeIsInstanceOf shadowed the global `constructor`. Lore-id: 9d2c04b7 Confidence: high Scope-risk: wide Reversibility: reversible Tested: biome check . (clean) Not-tested: no behavior change intended beyond the dead-code removals
Deferring @gajae-code/natives behind an async accessor added a microtask yield inside startSession before it registers in sessionStartPromises, so two concurrent `/notify on` calls each built a runtime and the loser threw "Lifecycle SDK startup was cancelled". require() is synchronous, so the accessor does not need to be async to stay lazy. Lore-id: 3e6fb18d Constraint: the native must stay off the startup module graph -- keep the in-function require Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun test packages/coding-agent/test/sdk-host-wiring.test.ts (78 pass)
Deferring discoverable tools advertises them from the descriptor alone, so every factory guard that used to drop a tool at creation had to move into availableFor. Without it a headless session advertised `ask` and only failed at call time; the same held for `checkpoint`/`rewind` in subagents, `irc` without an agent registry, `github` without the gh CLI, and `cron` under CLAUDE_CODE_DISABLE_CRON. fetch's html-to-markdown accessor cached the bound export rather than the module, freezing the first-seen implementation for the process. Lore-id: 1f7ad64c Constraint: availability must stay cheap -- no heavy imports on the descriptor path Rejected: eager materialization for conditional tools | reloads exactly what the deferral removed Confidence: high Scope-risk: medium Reversibility: reversible Tested: bun test packages/coding-agent/test/tools packages/coding-agent/src/tools (1719 pass) Not-tested: telegram_send availability still resolves at load; its guard needs the notification snapshot
The self-test expected moving continueStalledGjcTeamWorkers after stale-claim reconciliation to fail, but exactTeamRuntimeSendKeysRanges validated only the continuation function and ignored its monitor call site. Pin one continuation call before one reconciliation call inside monitorGjcTeam, and keep the direct Bun.spawnSync adversarial fixture syntactically valid. Lore-id: c61a9df4 Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun packages/coding-agent/scripts/verify-gjc-sdk-canonicalization.ts --self-test
The PR changes public package surfaces and runtime behavior across the AI, agent, coding-agent, TUI, and utils workspaces. Record the core entrypoint, telemetry fast path, lazy native loading, MCP/plugin/runtime changes, and the correctness fixes under each package's Unreleased section. Lore-id: 6f7430ad Confidence: high Scope-risk: narrow Reversibility: reversible Tested: git diff --check
The RSS harness test read three ignored `.gjc/rss-checkpoints` files that existed only in the author's worktree, so CI could never run M6. Commit the minimal immutable W1c identity evidence under scripts/fixtures and read it there instead. The descriptor availability matrix also assumed a Darwin-arm64 computer backend and an unset cron-disable variable. Derive those expected exclusions from the same platform and environment predicates as the descriptor. Lore-id: e3f8256a Confidence: high Scope-risk: narrow Reversibility: reversible Tested: CLAUDE_CODE_DISABLE_CRON=1 bun test packages/coding-agent/src/tools/descriptors.test.ts Tested: bun test scripts/harness-gates.test.ts
Deferred modules changed first-use timing and captured several exports too early, breaking syntax highlighting, clipboard spies, memory startup joins, pruning, MCP leases, broker validation, and test-only inspection of lazy tools. Keep startup graphs lazy while restoring the observable contracts at feature use. Lore-id: 7c51fd9a Confidence: high Scope-risk: wide Reversibility: reversible Tested: focused interactive, SDK, broker, pruning, memory, and workflow-gate suites (262 pass) Tested: W1c and W5b module traces Tested: bun run check:ts
Cached native bindings bypassed live identity adapters, while long-lived lock descriptors could retain stale bytes or report a transient Linux release mismatch. Resolve bindings at feature use, make descriptor writes complete and truncating, and allow only an independently verified exact-identity release fallback. Lore-id: 98bf5a24 Confidence: medium Scope-risk: wide Reversibility: reversible Tested: SDK broker lifecycle suites (75 pass) Not-tested: Linux 9,999-artifact migration locally; CI runner is authoritative
Lazy exact-unlink and process-incarnation bindings change protected lifecycle authority, so older resident daemons must not continue serving as current. Advance the generation and refresh the guarded manifest and assertions. Lore-id: 6a7d3e11 Confidence: high Scope-risk: narrow Reversibility: reversible Tested: Telegram generation guard current-tree validation Tested: bun run check:ts
Discord and Slack share the exact unlink and process-incarnation lifecycle symbols moved behind lazy native bindings in this branch. Advance both serving generations and refresh the protected manifest so resident older daemons are replaced. Lore-id: 31cb749e Confidence: high Scope-risk: narrow Reversibility: reversible Tested: bun test packages/coding-agent/test/daemon-control.test.ts Tested: bun scripts/telegram-daemon-generation-guard.ts --validate-current-tree
099918b to
b750886
Compare
2 participants
Summary
Startup and steady-state memory work for the
gjcCLI, delivered as plan milestones W1–W6. The theme throughout: stop paying at startup for work an invocation never performs, and give the MCP connection layer a real lifecycle boundary.Measured on this host with the harness added in this PR,
S6deferred by contract:deferred— requires W7/W8 authorization and daemon implementationS1/S2/S7 vary widely run-to-run on a loaded machine; the gate compares against a per-commit canonical baseline rather than treating any single run as authoritative.
What changed
sdk/bussplits into a session runtime (SessionSdkHost) plus Telegram/Discord/Slack adapters behind aLazyService, so hosting a session no longer drags the adapters and their native dependency onto the startup graph.@gajae-code/nativesimport to an in-functionrequireat the point of use.@gajae-code/ai/core— a subpath that excludes provider construction, with ~124 coding-agent call sites migrated onto it. The root barrel is untouched for external consumers.spans: falsenow performs zero span and attribute work while usage and cost hooks keep firing.verify-module-tracepins which modules a scenario may load;verify-rss-checkpointsmeasures process-tree RSS against a per-commit baseline with floor enforcement.Notable correctness fixes
computerbatch path (fix(computer)) — the legacy batch adapter had drifted from the native path: thrown steps lostfailureCode/failureIndex,timeoutMswas ignored, and batch dispatch skipped the pre-dispatch bounds check the single-action path applies. A batch could act on coordinates the single path rejects.tools/callafter transport failure; peer facades left on a retired entry after restart; a replacement dropped when the receiving manager was reconnecting an unrelated server; and a second rotation landing inside the acquire-to-register window on both admission paths./notify on|off— the W1b branch dereferencedruntime.sessionunguarded; fixed to pass through when there is no session. Concurrent/notify oncalls also remain single-flight after lazy native loading, rather than creating competing runtimes.Design decisions worth review
tools/callsurfaces typed to its calling lease and is never resent — recovery reconnects and rebinds peers, but does not retry the call. Retrying risks duplicating a server-side side effect after an uncertain response.Authorization-bearing HTTP entry without a non-secret binding kind/scope is refused (MCP_AUTH_BINDING_REQUIRED) rather than sharing a transport across credentials. The secret never enters the pool key.--baselinewith--compareresolves only.gjc/rss-checkpoints/<commit>.jsonand otherwise fails closed. An earlier attempt allowed falling back to the same-commit.last-run.json; because a non-baseline run overwrites that file, the gate flipped between pass and fail on back-to-back identical invocations.Verification
Final head
b750886fd:bun run check:tsPASS · repository Biome PASS · repaired regression matrix 262/262 · daemon control suite 161/161 · portable descriptor/harness gates 38/38 · MCP/plugin/harness suites 98/98 ·packages/agent692/692 in a clean HOME · module traces (help/idle W1c, help W5b, literal tool and skill catalogs) PASS · Telegram/chat generation guard PASS · SDK canonicalization self-test PASS · exactbun scripts/verify-rss-checkpoints.ts --all --compareexit 0.Not in scope
W7–W9 (daemon, attach, compatibility-default flips) are deliberately absent.
workspaceTree.modestayseager,startup.networkPrewarmstaystrue, and MCP sharing defaults toper-session, so this PR changes no user-visible defaults.S6staysdeferreduntil that work is authorized.Known pre-existing failures
test/session-storage.test.ts(34) andtest/session-manager-resume-readonly.test.ts(4) fail identically on a clean worktree at the merge base — established bygit worktree addat pristine upstream and diffing failing case names, not just totals. Neither file is touched here.