fix(agent-core-v2): temporarily disable skill-root directory watches and add detailed startup tracing - #3690
liruifengv wants to merge 7 commits into
Conversation
The skill-root watches added in #3608 register recursive fs.watch roots on the entire OS home directory and the whole project root. On Windows these run through the native recursive leg, which delivers every subtree event to the main thread with no OS-level filtering, and each event costs an xstate transition (#3502). This is the prime suspect behind the reported 0.42.0 startup and interactive lag on Windows. Gate both watch sites behind KIMI_CODE_SKILL_ROOT_WATCH (default off) until the watches are redesigned to cover only the skill candidate directories. Skill catalogs still load at startup; new or changed skills are picked up on restart.
🦋 Changeset detectedLatest commit: 936fec2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![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: 5c2639a9f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Disable live watching of skill directories; new or changed skills are picked up on restart. Set KIMI_CODE_SKILL_ROOT_WATCH=1 to re-enable live refresh. |
There was a problem hiding this comment.
When release tooling publishes this entry, it will include two sentences and expose the temporary environment-variable mechanism, although repository policy requires one short user-facing sentence stating only what changed. Fold the relevant user-visible behavior into a single sentence and omit mechanism-level detail.
AGENTS.md reference: AGENTS.md:L86-L87
Useful? React with 👍 / 👎.
…race-and-skill-watch
…rebuild Lock contention (LockError, including cross-shard AggregateError) means another live process holds the shard locks, so closing the cluster and attempting a full wipe-and-rebuild can never succeed and only amplifies IO: it retries all 16 shard locks, reopens the cluster, and re-fans-out index definitions. Contention errors now throw straight through without counting toward the transient-failure escalation. To make contention observable in the field, a throttled (60s) warn logs the cluster stats (lockWaits, writerOpens, readerReopens, incrementalCatchups, catchupFramesApplied, evictions) on contention.
…slow-path timing logs The Windows lag report has three unproven suspects beyond shard lock contention: main-thread event-loop stalls, degraded session-index reads degenerating into full session-tree scans, and slow reconcile ticks. Add direct instrumentation for each: - EventLoopMonitorService (App scope, KIMI_CODE_EVENT_LOOP_MONITOR=1): per-second event-loop delay sampling, warn on stalls >= 250ms with p50/p99/max, plus a 60s summary line. - tick(): duration on the reconciliation-failed warn, plus a slow reconcile warn (>= 2s). - collectAuthoritative(): slow full-scan warn (>= 2s) with session count and index state. - mirror flush failure warn gains durationMs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05d776e52d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isLockContentionError(error)) { | ||
| this.noteLockContention(db, kind, error); | ||
| throw error; |
There was a problem hiding this comment.
When a multi-process lock collision occurs during SessionIndexMirror's cross-shard batch, ClusterDb.batch may commit some shard groups before returning an AggregateError. Exempting every such error from rebuild escalation can leave workspace counters permanently inconsistent: if a summary write committed but its counter write did not, the retry sees the summary as current and emits no counter delta, and SessionIndexProjector.reconcile uses the same old-vs-new delta logic before clearing the dirty mark. Retain a recovery mechanism for partially committed contention failures or make the summary/counter repair idempotent.
Useful? React with 👍 / 👎.
| export function skillRootWatchEnabled(): boolean { | ||
| return parseBooleanEnv(process.env[SKILL_ROOT_WATCH_ENV]) === true; |
There was a problem hiding this comment.
When agent-core is embedded with an explicit bootstrap environment, such as kap-server's public ServerStartOptions.env, this lookup ignores that environment and reads the host process instead. Consequently, passing KIMI_CODE_SKILL_ROOT_WATCH=1 through startServer({ env: ... }) does not enable either watcher, while an ambient value can enable watches even when the supplied environment omits it; resolve the value through IBootstrapService.getEnv like the other engine flags.
Useful? React with 👍 / 👎.
| this.histogram = monitorEventLoopDelay({ resolution: 20 }); | ||
| this.histogram.enable(); | ||
| this.timer.cancelAndSet(() => this.sample(), SAMPLE_INTERVAL_MS); |
There was a problem hiding this comment.
When KIMI_CODE_EVENT_LOOP_MONITOR is enabled and an app scope is later closed or an SDK client is disposed, only the interval timer is registered for disposal; the enabled native IntervalHistogram is never disabled. Closed clients and restarted in-process servers can therefore leave event-loop monitors sampling indefinitely and accumulate diagnostic overhead across lifecycles; register a disposable that calls histogram.disable().
Useful? React with 👍 / 👎.
…behind env vars So the affected machine can run an elimination matrix: - KIMI_CODE_DIAG=1 enables the event-loop stall monitor, the slow reconcile / authoritative-scan warns, and the durationMs fields (default off; KIMI_CODE_EVENT_LOOP_MONITOR is folded into it). - KIMI_CODE_QUERY_STORE_REBUILD_ON_CONTENTION=1 restores the previous behavior where lock contention counts toward the transient-failure escalation and triggers wipe-and-rebuild attempts. Default is the fixed behavior (contention never escalates); the throttled contention stats warn logs in both modes so contention stays observable while toggling.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 936fec283a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| maxMs: Math.round(histogram.max / 1e6), | ||
| }); | ||
| } | ||
| histogram.reset(); |
There was a problem hiding this comment.
When KIMI_CODE_DIAG remains enabled for at least 60 seconds, resetting the histogram after every one-second sample means the summary emitted on tick 60 contains only the immediately preceding second rather than the preceding minute. Delays during the other 59 seconds are omitted, so the p50/p90/p99/max summary can misleadingly report a healthy event loop; retain separate interval-wide statistics or reset the summary histogram only after it is logged.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — the problem is explained below (reported by Windows users on 0.42.0).
Problem
Windows users report that the latest CLI (0.42.0) starts very slowly and feels laggy throughout the session. A
KIMI_STARTUP_TRACE=1capture from an affected machine shows ~2.5s in the win32-only Git Bash probe and 15.3s insidegetWorkspaceTrustInfo(workspace materialization) before the first screen.One of the 0.42.0 changes in this area is #3608, which added recursive file watches on the entire OS home directory and the whole project root to keep the skill catalog fresh. On Windows these run through the native recursive
fs.watchleg (#3502), which delivers every subtree event to the main thread with no OS-level filtering — the home directory is one of the most active trees on a Windows machine (Defender, search indexer, browser caches, OneDrive), so the main thread is continuously interrupted. There is currently no config, env var, or flag to turn these watches off.Separately, the same affected-machine logs show
session index reconciliation failedwarnings recurring every few minutes (164x in one log), each wrappingAggregateError: batch failed on N/M shard(s)/LockError: database is locked by another process: ...\query-store\shard-N. Multiple concurrently running CLI instances share the session-index store at~/.kimi-code/cache/query-store, and the loser's writes keep colliding with the winner's shard locks. Worse, contention failures counted toward the transient-failure escalation, so every 5th failure triggered a full wipe-and-rebuild attempt (16 shard-lock acquisitions + cluster reopen + index fan-out) that could never succeed while the peer was alive — steady background IO churn on top of the retries themselves.What changed
UserFileSkillSource— user-level roots under the OS home directory, andWorkspaceRootSkillSource— the project root) are now gated behindKIMI_CODE_SKILL_ROOT_WATCH(default off) until the watches are redesigned to cover only the skill candidate directories instead of the whole home/project tree. Skill catalogs still load normally at startup; newly added or changed skills are picked up on restart. SetKIMI_CODE_SKILL_ROOT_WATCH=1to re-enable live refresh.startupTracepoints along the workspace-trust path so the remaining startup cost can be split precisely on the affected machine:workspaceTrust:getOrCreate:*/workspaceTrust:read:*(SDK),workspace:catalog:*/workspace:materialize:*(engine). The engine-side tracer lives in agent-core-v2 and shares the same env vars (KIMI_STARTUP_TRACE/KIMI_STARTUP_TRACE_LOG), the same log file, and the same process-start clock as the app-side tracer (the app tracer now uses rawperformance.now()so app and engine labels are directly comparable).LockError, including the cross-shardAggregateErrorshape) no longer count toward the transient-failure limit and never trigger a wipe-and-rebuild; they throw straight through to the caller's existing retry/self-heal path. A throttled (60s) warn now logs the cluster lock stats (lockWaits,writerOpens,readerReopens,incrementalCatchups,catchupFramesApplied,evictions) when contention occurs, so it is directly observable in affected-machine logs. New tests cover the no-rebuild-under-contention behavior and the error classification.KIMI_CODE_SKILL_ROOT_WATCH=1; two new tests cover the default-off behavior for both watch sites.Checklist
gen-changesetsskill.gen-docsskill, or this PR needs no doc update — skill watching was not documented, and theKIMI_CODE_SKILL_ROOT_WATCHkill-switch is temporary.