Skip to content

fix(cli): allow local session creation on stale Flock sync - #411

Open
UniversePeak wants to merge 2 commits into
LodyAI:mainfrom
UniversePeak:fix/session-flock-sync-best-effort
Open

fix(cli): allow local session creation on stale Flock sync#411
UniversePeak wants to merge 2 commits into
LodyAI:mainfrom
UniversePeak:fix/session-flock-sync-best-effort

Conversation

@UniversePeak

@UniversePeak UniversePeak commented Sep 5, 2026

Copy link
Copy Markdown

Related issue

Closes #398

Problem / pressure

When the machine Flock freshness sync fails during degraded network conditions, MCP/local session creation rejects a project that is already available in the local replica. The remote round trip is a freshness check, not a prerequisite for resolving the local project needed to create the session.

Summary

Keep the existing hard-fail sync helper for callers that require fresh reads, but use a best-effort wrapper for both local-project resolution paths. A failed freshness sync is logged and resolution continues from the local Flock replica.

Before / after

Before After
A Flock freshness timeout/network error aborts local-project resolution and prevents session creation. The failure is logged, local project resolution continues from the replica, and session creation can proceed with locally known inputs.

Test plan

  • corepack pnpm exec vitest run src/commands/session.test.ts --pool=forks --maxWorkers=1 — 65 tests passed.
  • The regression forces syncFlockDocOrThrow to reject with Streams sync failed: network_error and verifies local project resolution still succeeds.
  • git diff --check passed.
  • Full integration verification against a live degraded Streams connection was not run.

Context handoff

Instructions for reviewing agents

  • Review focus: Inspect apps/cli/src/commands/session.ts local-project resolution paths and confirm the best-effort wrapper does not alter agent-config or ACP capability freshness reads.
  • Decisions to challenge: Confirm that CLI and MCP local-project resolution should share this policy because both consume the local replica before session creation.
  • Plausible failures / evidence gaps: The regression covers the resolver boundary with a synthetic network error; a live degraded Streams integration test remains unavailable in this environment.

Authoring context

  • User goal / directives: Fix Issue [Bug] Session create hard-fails on a best-effort Flock freshness sync when the network is degraded #398 so locally resolvable session creation survives transient machine Flock freshness-sync failures.
  • Constraints / non-goals: Keep the change focused; do not make unrelated Flock reads best-effort or add a new MCP offline option.
  • Risk-bearing decisions: Only the two local-project resolution callers switch to best-effort; the original hard-fail helper remains unchanged for other freshness-sensitive reads.
  • Destructive or irreversible behavior: None; the change only logs a failed freshness check and reads the existing local replica.
  • Deliberately not done or tested: No live packet-loss/degraded-Streams integration test was run; the deterministic unit regression covers the failure boundary.
  • Unknowns / confidence: High confidence in the local behavior and regression coverage; live transport behavior still belongs to CI/maintainer verification.

@github-actions github-actions Bot added scope: cli status:needs-pr-attention External PR needs contributor attention before review and removed status:needs-pr-attention External PR needs contributor attention before review labels Sep 5, 2026
@Leeeon233
Leeeon233 requested a review from zxch3n September 6, 2026 00:02

@zxch3n zxch3n left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking #398 on — the direction matches the issue's first suggestion exactly, and the change itself is clean: no new imports, no new dependencies, typecheck passes, and the adjusted test passes locally (CI code checks did not run on this fork PR, so I ran them by hand).

Three things stand before this actually fixes #398, so I'm requesting changes rather than merging it as-is:

1. The create path has a second hard-fail sync one step earlier, and this PR leaves it in place.
resolveCreateContext calls resolveAgentConfigForCreatelistAgentConfigsForMachinesyncMachineFlockDocsForRead(…, 'session.agent-configs') (apps/cli/src/commands/session.ts:2184) before it reaches resolveLocalProjectRefOnMachineOrThrow. In the reported incident that earlier sync happened to succeed (the network was intermittent), which is why the failure surfaced at session.local-projects. Under sustained degradation the create still dies at session.agent-configs, so Closes #398 is not yet true. The same best-effort treatment needs to cover that call.

2. Remote-machine creates get a worse error, not a better one.
machineId can name another machine. When this CLI has no local replica of that machine's Flock doc, the sync is the only source of its projects; after this change the failure becomes Local project not found instead of the sync error. Suggest one of: keep best-effort only when machineId is this machine, or when the project is not found after a failed sync, attach the sync failure to the not-found error so the cause is visible.

3. Test coverage moved rather than grew.
marks local project refs for worktree session creation was renamed and its mock changed to reject, so the success path (sync resolves, useWorktree marking) no longer has a test in that block. Please restore the original test and add the failure-path case as a new one.

If you'd rather land the current scope as a partial step, that's fine too — change Closes #398 to Refs #398 so the issue stays open for (1) and (2).

@UniversePeak

Copy link
Copy Markdown
Author

Addressed the requested changes in ba5253a.

  • Session creation now treats both agent-config and local-project Flock freshness syncs as best-effort, while preserving the hard-fail helper for ACP capability reads.
  • If a failed sync leaves a project unavailable, the resulting error retains the sync cause for remote-machine diagnostics.
  • Restored the worktree success regression and added a separate stale-sync success case.

Verification: session.test.ts — 66 tests passed; git diff --check passed. The PR remains open and is ready for re-review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Session create hard-fails on a best-effort Flock freshness sync when the network is degraded

2 participants