Skip to content

test(agent-core-v2): stop the reconcile loop during the sessionIndex read baseline - #2625

Merged
sailist merged 1 commit into
MoonshotAI:mainfrom
sailist:test/sessionindex-baseline-stop-reconcile
Aug 5, 2026
Merged

test(agent-core-v2): stop the reconcile loop during the sessionIndex read baseline#2625
sailist merged 1 commit into
MoonshotAI:mainfrom
sailist:test/sessionindex-baseline-stop-reconcile

Conversation

@sailist

@sailist sailist commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Related Issue

No linked issue — the problem is explained below. Observed on CI run 30975794688 (PR #2599, unrelated changes), shard test (2).

Problem

FileSessionIndex (read model) > baseline: warm listRecent(limit=20) at 1k vs 10k vs 50k sessions flakes on slow CI runners with AssertionError: expected 2 to be +0 at the expect(op.fsLists).toBe(0) assertion.

The baseline gates warm reads on behavioral work counts, including "zero directory listings inside each counting window". But FileSessionIndex starts a background reconcile loop (RECONCILE_INTERVAL_MS = 60_000) once ready, and every tick unconditionally runs the projector's authoritative scan, which enumerates workspace + session directories — exactly two storage.list calls for the single-workspace test setup. Locally the test finishes in ~12s so the first tick never fires; on a loaded shared runner the test takes ~84s per attempt, so a tick lands inside a counting window on essentially every attempt and its two listings get attributed to the read under test. The existing retry: 1 cannot absorb that — both attempts fail the same way (168s total in the observed run).

What changed

  • Added a stopReconcileLoop() test hook on FileSessionIndex (sibling of the existing reconcileNow / reprojectNow test/ops hooks) that cancels the reconcile IntervalTimer.
  • The baseline test calls it right after prepare(), before any measurement, and the comment block now describes this instead of claiming the retry absorbs a tick.

This makes the gate deterministic rather than relaxing it: after prepare() + stopReconcileLoop(), the only remaining storage.list callers that could fire inside a counting window are the fallback read paths themselves (projector project runs during prepare, reconcile is stopped), so fsLists > 0 can once again only mean a real regression. The reconcile loop itself is production behavior — it repairs drift the best-effort mirror misses (external state.json edits/deletions, dropped flushes) and stays covered by the dedicated reconciliation repairs external edits and deletions of state.json test.

Verified: pnpm vitest run test/app/sessionIndex/sessionIndex.test.ts — 29/29 pass; tsc --noEmit clean. Note the race itself cannot be reproduced locally (local runs finish well under the 60s tick interval); the fix removes the only background listing source, so runtime no longer matters.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, or explained the problem above.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

…read baseline

The baseline gates warm reads on behavioral work counts, including zero
directory listings inside each counting window. The service's background
reconcile loop (60s interval) runs an authoritative scan on every tick —
two storage.list calls — and once a slow CI runner stretches the test past
that interval, a tick lands inside a counting window on every attempt and
is attributed to the read under test, failing fsLists === 0 even with the
retry (observed on run 30975794688: 168s across two attempts, fsLists = 2
both times).

Freeze the loop right after prepare() via a new stopReconcileLoop() test
hook (sibling of reconcileNow/reprojectNow). The only remaining directory
listing sources inside a window are the fallback read paths themselves,
so a non-zero count is deterministically a real regression again.
@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 78d2f01

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Aug 5, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@moonshot-ai/kimi-code@78d2f01
npx https://pkg.pr.new/@moonshot-ai/kimi-code@78d2f01

commit: 78d2f01

@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: 78d2f01ec8

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

Comment on lines +249 to +250
/** Test hook: stop the background reconcile loop, so measurement windows
* contain only the operations under test. */

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 Remove the method-level test-hook comment

This new JSDoc sits beside a method in an implementation file. The scoped packages/agent-core-v2/AGENTS.md comment convention requires comments to live only in the top-of-file /** */ block and never beside functions or methods, so this note should be removed or folded into the file header if it is important.

AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

@sailist
sailist merged commit 4e5f36a into MoonshotAI:main Aug 5, 2026
15 checks passed
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.

1 participant