Skip to content

perf: cut startup cost across natives, providers, plugins and MCP (W1-W6) - #3846

Merged
Yeachan-Heo merged 32 commits into
devfrom
feat/jcode-research
Aug 7, 2026
Merged

perf: cut startup cost across natives, providers, plugins and MCP (W1-W6)#3846
Yeachan-Heo merged 32 commits into
devfrom
feat/jcode-research

Conversation

@Yeachan-Heo

@Yeachan-Heo Yeachan-Heo commented Aug 5, 2026

Copy link
Copy Markdown
Owner

No description provided.

@Yeachan-Heo
Yeachan-Heo marked this pull request as draft August 5, 2026 07:17

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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");

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 Badge Avoid fs reads for bundled skill content

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");

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 Badge Install the supervisor SIGTERM guard before lazy import

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo marked this pull request as ready for review August 6, 2026 01:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +693 to +694
options.onNotification?.(method, params);
this.emit(entry, { type: "notification", method, params });

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 Badge Dispatch each pooled notification only once

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();

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 Badge Establish artifacts before pruning ephemeral sessions

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 👍 / 👎.

Comment on lines +172 to +174
this.#connectionDisposer?.();
this.#capabilitiesDisposer?.();
this.#malformedDisposer?.();

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 Badge Reinstall SDK transport handlers after restart

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from 38a2867 to 2f2e32c Compare August 6, 2026 08:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +370 to +373
if (
!/\.(?:optional|nullable)\s*\(\s*\)\s*$/.test(field[2]!) &&
!/\.Optional\s*\(\s*\)\s*$/.test(field[2]!) &&
!/(?:Type\.)?Optional\s*\(/.test(field[2]!)

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 Badge Keep nullable plugin parameters required

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 👍 / 👎.

Comment on lines +8372 to +8373
const runtime = await this.#workspaceTreeService.get("first-turn-barrier");
this.#cachedWorkspaceTree = firstWorkspaceTree ? runtime.snapshot : await runtime.refresh();

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 Badge Preserve best-effort workspace scanning in lazy mode

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 👍 / 👎.

Comment on lines +132 to +134
const endpoint = await this.transport.start();
this.#transportStarted = true;
return endpoint;

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 Badge Prevent transport startup from winning after shutdown

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from 2f2e32c to ad2110b Compare August 6, 2026 09:20

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Replace inline backend imports with approved lazy loaders

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 👍 / 👎.

Comment on lines +11732 to +11735
const plan = planToolOutputPrune(branchEntries, {
...DEFAULT_PRUNE_CONFIG,
minimumSavings: overThreshold ? 0 : DEFAULT_PRUNE_CONFIG.minimumSavings,
});

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 Badge Account for artifact references when admitting a prune

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from ad2110b to 5933677 Compare August 6, 2026 11:31

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +1061 to +1063
active = { runtime, revisions, cursors, reconciliation, pending, disposeGate };
try {
await runtime.start();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Register SDK-only sessions with the broker

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 👍 / 👎.

Comment on lines +199 to +202
await filesystem.writeFile(
endpointFile,
JSON.stringify({ version: 1, url, token: input.token, pid: process.pid }),
"utf8",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include the session identity in SDK endpoint records

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 👍 / 👎.

Comment on lines +2462 to +2465
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));

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 Badge Load syntax bindings before one-shot rendering

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 👍 / 👎.

@yazzang-homelab

Copy link
Copy Markdown
Contributor

경고 — 이 PR의 현재 head가 CHANGELOG 전체를 삭제한다

머지하면 안 된다. 확인된 사실:

$ git cat-file -s <이 PR head>:<해당 CHANGELOG 경로>
1

1바이트 — 개행 하나만 남았다. dev의 같은 파일은 312,259 bytes(coding-agent) / 244,785 bytes(ai) / 45,275 bytes(agent)다. 릴리스 이력 전체가 사라진 상태다.

원인은 내 쪽이다

#3932(11:25:32Z 머지)가 .gitattributes에서 packages/*/CHANGELOG.md merge=union을 제거했다. 제거 자체는 근거가 있었다 — union은 충돌을 내지 않고 양쪽을 이어붙여서 이미 릴리스된 섹션에 항목을 조용히 밀어넣고 있었다(#3929, 실측 35건).

그런데 그 결과 리베이스에서 CHANGELOG가 처음으로 진짜 충돌을 내기 시작했고, 그 충돌을 해소하는 과정에서 파일이 비워졌다. 시간대가 명확하다:

시각 (UTC) 사건
11:25:32 #3932 머지 (union 제거)
11:29:29 ~ 11:35:02 #3920 #3697 #3870 #3908 #3887 #3864 #3729 #3869 #3866 #3873작성자 6명, 10개 PR이 전부 1바이트 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 충돌이 나면 양쪽 항목을 모두 ## [Unreleased] 아래에 남기는 것이 올바른 해소다. 이미 릴리스된 ## [X.Y.Z] 섹션은 손대지 않는다. CONTRIBUTING.md의 "Rebasing onto dev" 절에 적어두었다.

푸시 전에 다음으로 자가 점검할 수 있다:

git cat-file -s HEAD:packages/coding-agent/CHANGELOG.md   # 30만 바이트 근처여야 정상

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +86 to +87
if (range !== null) {
content = await file.slice(start, boundedEnd).text();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Cap explicit artifact ranges before reading

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 👍 / 👎.

Comment on lines +76 to +77
const stopResult = current.stop(true);
await Promise.race([stopResult, new Promise<void>(resolve => setTimeout(resolve, 250))]);

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 Badge Cancel the SDK shutdown timeout after a fast stop

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 👍 / 👎.

Comment on lines +717 to +719
void this.#ensureHistoryStorage().then(storage => {
if (storage && this.#history.length > 0) this.#navigateHistory(direction);
this.invalidate();

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 Badge Discard stale asynchronous history navigation

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from 3b5de5c to 9549e2f Compare August 6, 2026 13:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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;

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 Badge Exclude unavailable Telegram tools from discovery

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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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"),

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 Badge Decouple theme file watching from appearance detection

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 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from 099918b to b750886 Compare August 6, 2026 13:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b750886fd2

ℹ️ 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".

this.#sleep = options.sleep ?? delay;
this.#afterLeaseAcquiredForTests = options.afterLeaseAcquiredForTests;
this.cwd = canonicalMCPWorkingDirectory(this.cwd);
this.#pool = options.pool ?? new MCPConnectionPool({ sharedPoolIdleMs: options.sharedPoolIdleMs });

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 Badge Reuse one pool across MCP managers

When multiple MCP managers in the same process connect to a server configured with sharing: "shared", each manager still creates its own MCPConnectionPool, so their keys and leases can never meet and each facade opens a separate physical connection. I checked the production constructors in sdk/session.ts, the MCP loader, and runtime command controllers; they omit options.pool, while only tests inject a common pool. Use a process/session-host-owned shared pool for managers that are expected to honor shared mode.

Useful? React with 👍 / 👎.

Comment on lines +1008 to +1009
if (this.historyStorage) return this.historyStorage;
if (this.settings.get("history.enabled") === false) return undefined;

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 Badge Honor history disablement after storage loads

If history is initially enabled and the editor loads HistoryStorage, changing history.enabled to false later has no effect because this cached-storage return bypasses the setting check; subsequent prompts continue to be written even though history search is disabled. The same issue occurs when the setting is turned off while openAsync() is pending, because its continuation still installs the storage. Recheck the live setting before returning/installing storage and detach persistence when the setting is disabled.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f8e8d1581

ℹ️ 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 loaders: Record<string, Loader> = {
read: session => cached("read", () => import("./read")).then(module => new module.ReadTool(session)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Replace inline imports in the built-in tool loader

This new loader table resolves every built-in through runtime import() calls, despite the repository contract requiring top-level imports only. Besides making dependencies invisible to the static module graph, this leaves compiled-binary loading behavior dependent on runtime activation; use the repository's approved statically imported lazy-loader pattern instead.

AGENTS.md reference: AGENTS.md:L103-L106

Useful? React with 👍 / 👎.

}
if (requireDescription && !frontmatter.description) {
const frontmatter = await readSkillFrontmatter(skillPath);
if (!frontmatter) {

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 Badge Preserve frontmatter-free skills during lazy scanning

When requireDescription is false, a SKILL.md without YAML frontmatter was previously loaded using its directory name and full body, but readSkillFrontmatter() returns null for that valid input and this branch now drops it unconditionally. The Codex, OpenCode, Claude, and agents discovery paths call this scanner without requiring a description, so existing frontmatter-free custom skills silently disappear from discovery.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba7002b832

ℹ️ 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".

bisect: session => cached("bisect", () => import("./bisect")).then(module => new module.BisectTool(session)),
eval: session => cached("eval", () => import("./eval")).then(module => new module.EvalTool(session)),
calc: session => cached("calc", () => import("./calculator")).then(module => new module.CalculatorTool(session)),
ssh: session => cached("ssh", () => import("./ssh")).then(module => module.loadSshTool(session)),

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 Badge Hide SSH until a host configuration exists

With the default tools.discoveryMode="all", this discoverable loader publishes ssh even when the session has no configured SSH hosts. Activating it then makes loadSshTool() return null, which LazyAgentTool surfaces as the generic Tool "ssh" failed to load error; the previous eager factory path omitted the tool in this case. Check host availability before adding the deferred descriptor, or keep SSH eager until hosts are known.

Useful? React with 👍 / 👎.

monitor: session =>
cached("monitor", () => import("./monitor")).then(module => module.MonitorTool.createIf(session)),
cron: session => cached("cron", () => import("./cron")).then(module => module.CronTool.createIf(session)),
recipe: session => cached("recipe", () => import("./recipe")).then(module => module.RecipeTool.createIf(session)),

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 Badge Hide recipes when no runnable tasks exist

Because recipe discovery and recipe.enabled both default on, repositories without a supported runner containing tasks still advertise this deferred tool. On activation, RecipeTool.createIf() detects zero runnable tasks and returns null, so the model receives a generic failed-to-load error for a capability that should not have been discoverable; preserve the factory's task-detection guard before publishing the descriptor.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c77b91fa42

ℹ️ 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 class LazyServiceReentrantDisposeError extends Error {
readonly id: string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Declare the lazy initialization result type explicitly

Replace this ReturnType<>-derived alias with an explicit initialization-result interface. The repository contract prohibits ReturnType<>, and keeping the result shape explicit prevents this service's internal state type from changing implicitly whenever the initializer callback signature is edited.

AGENTS.md reference: AGENTS.md:L104-L104

Useful? React with 👍 / 👎.

return lease;
}

private waitForPendingEntry(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Convert the pool helpers to ES private methods

Convert this and the other newly introduced private pool helpers to #waitForPendingEntry, #closeEntry, and so on. The repository requires ES #private fields rather than TypeScript access modifiers, so the current pool implementation violates the established runtime-private convention throughout the class.

AGENTS.md reference: AGENTS.md:L108-L108

Useful? React with 👍 / 👎.

@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from c77b91f to 0ac498f Compare August 6, 2026 21:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ac498f7dc

ℹ️ 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".

)
required.push(key);
}
return { type: "object", properties, ...(required.length > 0 ? { required } : {}), additionalProperties: true };

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 Badge Preserve strictness in inferred plugin schemas

When a source-inferred plugin declares parameters: z.object({...}).strict(), callName() parses only the initial object() call and this emits additionalProperties: true, ignoring the trailing strict wrapper. The registry consequently advertises extra keys as valid even though the loaded Zod schema rejects them at execution time, so model-generated calls that conform to the published schema can fail validation; preserve supported trailing constraints or reject unsupported chains instead.

Useful? React with 👍 / 👎.

Comment on lines +199 to +203
await filesystem.writeFile(
endpointFile,
JSON.stringify({ version: 1, url, token: input.token, pid: process.pid }),
"utf8",
);

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 Badge Publish SDK endpoints without following existing symlinks

When a resumed session's known endpoint filename has been pre-created as a symlink, this direct writeFile follows it and truncates a file outside .gjc/state; the subsequent chmod also operates on that target. Because the endpoint path is workspace-local, stale or hostile workspace state can therefore corrupt arbitrary user files during SDK startup; publish a mode-0600 temporary file and atomically rename it over the endpoint, as the other SDK discovery writers do.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6014c24a07

ℹ️ 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".

Comment on lines +2244 to +2245
this.#powerAssertionLoad = Promise.resolve()
.then(() => {

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 Badge Cancel pending macOS assertion acquisition on release

When a turn exits synchronously after #beginInFlight()—for example, the preflight cancellation or missing-model checks can throw before the first await#releasePowerAssertion() runs while #powerAssertion is still unset, and this queued continuation subsequently starts the assertion anyway. The orphaned assertion can prevent system sleep until another turn finishes or the session is disposed; keep acquisition synchronous or track cancellation/release state before calling MacOSPowerAssertion.start().

AGENTS.md reference: AGENTS.md:L105-L105

Useful? React with 👍 / 👎.

const known = new Set(existing.map(entry => path.resolve(entry.pluginRoot)));
const discovered: GjcPluginRegistryEntry[] = [];
for (const dirent of dirents) {
if (!dirent.isDirectory() || dirent.name.startsWith(".")) continue;

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 Badge Preserve symlinked plugins during legacy discovery

When an existing user or project plugin is installed as a symlink and has not yet been indexed in registry.json, this filter skips it because Dirent.isDirectory() is false for symlinks. The previous discovery path explicitly accepted isSymbolicLink(), so these valid legacy plugins now silently disappear instead of being migrated and activated; retain symlink candidates and let the compiler's containment and hash validation decide whether they are safe.

Useful? React with 👍 / 👎.

Yeachan-Heo and others added 16 commits August 7, 2026 13:23
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
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
Long artifact migrations can leave the retained Linux lock descriptor reporting a
different inode even though the owner-only pathname still matches the original
lock identity and attempt. Reopen that exact pathname without following links,
verify it before and after release, and keep all other mismatches fail-closed.

Lore-id: 4e9c0cb2
Confidence: medium
Scope-risk: wide
Reversibility: reversible
Tested: managed lock lease and repaired regression suites (34 pass)
Tested: bun run check:ts
Not-tested: injected stale-descriptor recovery is Linux-only locally; affected CI covers it
The 9,999-artifact migration now completes and releases its lock, but Bun can
return a one-shot EFAULT while recursively deleting that unusually large test
tree. Retry only that transient cleanup code and still fail after three attempts.

Lore-id: 0288db21
Confidence: medium
Scope-risk: narrow
Reversibility: reversible
Tested: bun --cwd=packages/coding-agent run check
Not-tested: Linux EFAULT retry locally; affected CI reproduces the boundary
…vailable

Mid-run tool-output pruning must not report a successful prune when eviction
artifacts cannot be established. Ephemeral install failure (or a null manager)
with a non-empty prune plan now returns without committing partial non-tool
prunes, so maintenance continues to compaction and fails closed.

Update the regression fixture to force ephemeral install failure via
SessionManagerTestHooks so it no longer relies on in-memory sessions silently
getting a working tmpdir artifact store.

Lore-id: midrun-fc1
Confidence: high
Scope-risk: narrow
Reversibility: reversible
Tested: bun test packages/coding-agent/test/agent-session-midrun-maintenance.test.ts (18 pass)
Not-tested: full coding-agent shard matrix
The live dev branch advanced Telegram owner postmortem authority while this PR
carried lazy native daemon authority. Publish their union as generation 55,
regenerate both manifests, and remove a duplicate prune fixture introduced while
reconciling the concurrent session-maintenance changes.

Lore-id: f1cd58a4
Confidence: high
Scope-risk: wide
Reversibility: migration-needed
Tested: repaired regression suites (283 pass)
Tested: daemon control and managed lock suites (165 pass)
Tested: Telegram generation contracts and manifest guard
Tested: W1c/W5b module traces and literal catalog gates
Keep #3974 shared-topic-authority gen 55 notes and claim gen 56 for the
#3846 lazy native authority cut. Update generation contract tests.

Tested: telegram-daemon-generation-guard v43 bump verified
The live-base rebase replayed the provider-local relative import after the
compatibility helper moved to the AI package root. Point it at the sibling core
types module so publish and workspace type checks resolve again.

Lore-id: 4b9b1fb0
Confidence: high
Scope-risk: narrow
Reversibility: easy
Tested: coding-agent package check
Tested: AI package check and provider-free core test
@Yeachan-Heo
Yeachan-Heo force-pushed the feat/jcode-research branch from e69f956 to a8851ab Compare August 7, 2026 04:28
@Yeachan-Heo
Yeachan-Heo merged commit b832794 into dev Aug 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8851ab248

ℹ️ 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".

Comment on lines +18 to +19
initialize: async () => {
switch (resolveMemoryBackendId(settings)) {

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 Badge Reinitialize memory service when the backend setting changes

After session startup initializes this lazy service, changing memory.backend through the interactive settings selector cannot change its cached value. For example, a session started with local continues injecting and running the local backend after the user selects off, whereas the previous per-operation resolver observed the current setting. Reset or replace the service when this setting changes, or make the facade dispatch against the live backend selection.

Useful? React with 👍 / 👎.

settingsInstance.get("colorBlindMode"),
settingsInstance.get("theme.dark"),
settingsInstance.get("theme.light"),
settingsInstance.get("syntaxHighlighting.enabled"),

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 Badge Apply syntax-highlighting changes to the live theme

When an interactive user changes syntaxHighlighting.enabled in the settings selector, the new value is persisted but rendering does not change because the theme module copies it into syntaxHighlightingEnabledState only during this startup call, and the selector's change handler has no case that updates that state. Consequently the new appearance toggle has no effect until the CLI is restarted; wire the setting change to a live setter or theme reinitialization.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants