feat(sync): add result history and last-result log#84
feat(sync): add result history and last-result log#84ndycode wants to merge 2 commits intogit-plan/06-docs-frontdoor-cleanupfrom
Conversation
📝 WalkthroughWalkthroughadds persistent sync history tracking for codex cli and live account reload operations. introduces a new sync-history module that writes history as ndjson with concurrency control, plus history loading into codex-cli/sync and history recording hooks into live-account-sync. updates tests to verify history persistence and retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant CodexCliSync
participant SyncHistory
participant FileSystem
Caller->>CodexCliSync: setLastCodexCliSyncRun(run)
activate CodexCliSync
CodexCliSync->>SyncHistory: appendSyncHistoryEntry({kind: "codex-cli-sync", run, recordedAt})
activate SyncHistory
SyncHistory->>SyncHistory: acquireMutex()
SyncHistory->>FileSystem: append to sync-history.ndjson
SyncHistory->>FileSystem: write sync-history-latest.json
SyncHistory-->>SyncHistory: releaseMutex()
SyncHistory-->>CodexCliSync: Promise<void>
deactivate SyncHistory
CodexCliSync-->>Caller: Promise<void>
deactivate CodexCliSync
sequenceDiagram
participant Caller
participant CodexCliSync
participant SyncHistory
participant FileSystem
Caller->>CodexCliSync: getLastCodexCliSyncRun()
activate CodexCliSync
alt in-memory exists
CodexCliSync-->>Caller: CodexCliSyncRun
else needs hydration
CodexCliSync->>SyncHistory: readLatestSyncHistorySync()
activate SyncHistory
SyncHistory->>FileSystem: read sync-history-latest.json
SyncHistory-->>CodexCliSync: SyncHistoryEntry | null
deactivate SyncHistory
CodexCliSync->>SyncHistory: cloneSyncHistoryEntry(entry)
SyncHistory-->>CodexCliSync: cloned entry
CodexCliSync->>CodexCliSync: cache result, set flag
CodexCliSync-->>Caller: CodexCliSyncRun
end
deactivate CodexCliSync
sequenceDiagram
participant Caller
participant LiveAccountSync
participant SyncHistory
participant FileSystem
Caller->>LiveAccountSync: runReload() or via watch/poll
activate LiveAccountSync
alt reload succeeds
LiveAccountSync->>LiveAccountSync: recordHistory("manual"/"watch"/"poll", "success")
LiveAccountSync->>SyncHistory: appendSyncHistoryEntry({kind: "live-account-sync", outcome: "success", reason, recordedAt})
else reload fails
LiveAccountSync->>LiveAccountSync: recordHistory(..., "error", errorMessage)
LiveAccountSync->>SyncHistory: appendSyncHistoryEntry({kind: "live-account-sync", outcome: "error", reason, message, recordedAt})
end
activate SyncHistory
SyncHistory->>SyncHistory: acquireMutex()
SyncHistory->>FileSystem: append to sync-history.ndjson
SyncHistory->>FileSystem: write sync-history-latest.json
SyncHistory-->>SyncHistory: releaseMutex()
SyncHistory-->>LiveAccountSync: Promise<void>
deactivate SyncHistory
LiveAccountSync-->>Caller: void
deactivate LiveAccountSync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes review notesconcurrency concerns: lib/sync-history.ts uses a per-entry mutex ( windows edge cases: sync-history.ts uses missing regression tests:
logic density: lib/sync-history.ts is 210 lines of new logic including file i/o, locking, and error recovery. the test flakiness risk: test/live-account-sync.test.ts uses 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await fs.appendFile(paths.historyPath, line, { | ||
| encoding: "utf8", | ||
| mode: 0o600, | ||
| }); | ||
| await fs.writeFile(paths.latestPath, latestContent, { | ||
| encoding: "utf8", | ||
| mode: 0o600, | ||
| }); |
There was a problem hiding this comment.
Non-atomic two-phase write leaves latestPath inconsistent on failure
appendFile and writeFile are two separate syscalls inside the same lock. if appendFile succeeds but writeFile fails (e.g., windows AV holds the file, disk full, process killed mid-write), the ndjson log grows but sync-history-latest.json is stale or partially written. readLatestSyncHistorySync will silently return old data with no indication the inconsistency exists. lastAppendError does get set, but callers of getLastCodexCliSyncRun have no visibility into it.
the safe fix is to write latestPath via a temp file + rename, which is atomic at the fs level (on posix) and far more resilient on windows:
const tmp = `${paths.latestPath}.tmp`;
await fs.writeFile(tmp, latestContent, { encoding: "utf8", mode: 0o600 });
await fs.rename(tmp, paths.latestPath);windows fs.rename over an existing file can still fail with EPERM when another process holds the target — callers should handle that, but it's still better than a partial in-place write.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/sync-history.ts
Line: 97-104
Comment:
**Non-atomic two-phase write leaves `latestPath` inconsistent on failure**
`appendFile` and `writeFile` are two separate syscalls inside the same lock. if `appendFile` succeeds but `writeFile` fails (e.g., windows AV holds the file, disk full, process killed mid-write), the ndjson log grows but `sync-history-latest.json` is stale or partially written. `readLatestSyncHistorySync` will silently return old data with no indication the inconsistency exists. `lastAppendError` does get set, but callers of `getLastCodexCliSyncRun` have no visibility into it.
the safe fix is to write `latestPath` via a temp file + rename, which is atomic at the fs level (on posix) and far more resilient on windows:
```ts
const tmp = `${paths.latestPath}.tmp`;
await fs.writeFile(tmp, latestContent, { encoding: "utf8", mode: 0o600 });
await fs.rename(tmp, paths.latestPath);
```
windows `fs.rename` over an existing file can still fail with `EPERM` when another process holds the target — callers should handle that, but it's still better than a partial in-place write.
How can I resolve this? If you propose a fix, please make it concise.
| reason, | ||
| path: summarizeWatchPath(targetPath), | ||
| }); | ||
| await this.recordHistory(reason, "success"); |
There was a problem hiding this comment.
History write blocks publishSnapshot — latency risk on Windows
await this.recordHistory(reason, "success") is called inside this.reloadInFlight before the outer finally block calls this.publishSnapshot(). on windows, appendFile on a file that an AV scanner is momentarily holding can stall for hundreds of milliseconds. the snapshot (and therefore any status surface reading getLastLiveAccountSyncSnapshot) won't update until that stall resolves. subsequent runReload calls that check this.reloadInFlight will queue behind this I/O too.
recordHistory should be fire-and-forget here, matching the same pattern used in setLastCodexCliSyncRun (try/catch + no await on the caller side):
// after reloadCount/mtime update, before the try block exits:
void this.recordHistory(reason, "success").catch(() => {});
// and in the catch block:
void this.recordHistory(reason, "error", error instanceof Error ? error.message : String(error)).catch(() => {});since appendSyncHistoryEntry already swallows its own errors, the void+catch is purely defensive.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/live-account-sync.ts
Line: 261
Comment:
**History write blocks `publishSnapshot` — latency risk on Windows**
`await this.recordHistory(reason, "success")` is called inside `this.reloadInFlight` before the outer `finally` block calls `this.publishSnapshot()`. on windows, `appendFile` on a file that an AV scanner is momentarily holding can stall for hundreds of milliseconds. the snapshot (and therefore any status surface reading `getLastLiveAccountSyncSnapshot`) won't update until that stall resolves. subsequent `runReload` calls that check `this.reloadInFlight` will queue behind this I/O too.
`recordHistory` should be fire-and-forget here, matching the same pattern used in `setLastCodexCliSyncRun` (try/catch + no await on the caller side):
```ts
// after reloadCount/mtime update, before the try block exits:
void this.recordHistory(reason, "success").catch(() => {});
// and in the catch block:
void this.recordHistory(reason, "error", error instanceof Error ? error.message : String(error)).catch(() => {});
```
since `appendSyncHistoryEntry` already swallows its own errors, the void+catch is purely defensive.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/codex-cli/sync.ts">
<violation number="1" location="lib/codex-cli/sync.ts:148">
P2: Reading only the global latest history entry can hide the last Codex CLI sync result when a newer `live-account-sync` entry exists.</violation>
</file>
<file name="lib/sync-history.ts">
<violation number="1" location="lib/sync-history.ts:49">
P2: Waiting on pending writes with `Promise.all` can make reads throw when an append fails. Use `allSettled` (or store handled promises) so read paths stay resilient.</violation>
<violation number="2" location="lib/sync-history.ts:174">
P2: Test cleanup uses bare `fs.rm` without retry handling, which can make Windows CI flaky under transient file locks.</violation>
</file>
<file name="test/live-account-sync.test.ts">
<violation number="1" location="test/live-account-sync.test.ts:42">
P2: Use retrying temp-dir cleanup instead of bare `fs.rm` to avoid Windows EBUSY/EPERM/ENOTEMPTY test flakiness.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/codex-cli/sync.ts">
<violation number="1" location="lib/codex-cli/sync.ts:162">
P2: Async history fallback can overwrite a newer in-memory sync run with stale data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .then((entries) => { | ||
| const latestCodexEntry = entries.at(-1); | ||
| if (latestCodexEntry?.kind === "codex-cli-sync") { | ||
| lastCodexCliSyncRun = latestCodexEntry.run; |
There was a problem hiding this comment.
P2: Async history fallback can overwrite a newer in-memory sync run with stale data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/codex-cli/sync.ts, line 162:
<comment>Async history fallback can overwrite a newer in-memory sync run with stale data.</comment>
<file context>
@@ -154,6 +155,14 @@ export function getLastCodexCliSyncRun(): CodexCliSyncRun | null {
+ .then((entries) => {
+ const latestCodexEntry = entries.at(-1);
+ if (latestCodexEntry?.kind === "codex-cli-sync") {
+ lastCodexCliSyncRun = latestCodexEntry.run;
+ }
+ })
</file context>
| lastCodexCliSyncRun = latestCodexEntry.run; | |
| if ( | |
| !lastCodexCliSyncRun || | |
| latestCodexEntry.run.runAt >= lastCodexCliSyncRun.runAt | |
| ) { | |
| lastCodexCliSyncRun = latestCodexEntry.run; | |
| } |
| void readSyncHistory({ kind: "codex-cli-sync", limit: 1 }) | ||
| .then((entries) => { | ||
| const latestCodexEntry = entries.at(-1); | ||
| if (latestCodexEntry?.kind === "codex-cli-sync") { | ||
| lastCodexCliSyncRun = latestCodexEntry.run; | ||
| } | ||
| }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
Background async read can silently overwrite a fresher in-memory result
the fire-and-forget readSyncHistory callback unconditionally overwrites lastCodexCliSyncRun. if a sync run completes (calling setLastCodexCliSyncRun) between the moment this background read is queued and the moment its .then resolves, the fresh in-memory result gets clobbered by the stale on-disk entry. this is an async ordering race unique to node's event loop interleaving.
fix: guard the write with a check for whether lastCodexCliSyncRun has already been set:
| void readSyncHistory({ kind: "codex-cli-sync", limit: 1 }) | |
| .then((entries) => { | |
| const latestCodexEntry = entries.at(-1); | |
| if (latestCodexEntry?.kind === "codex-cli-sync") { | |
| lastCodexCliSyncRun = latestCodexEntry.run; | |
| } | |
| }) | |
| .catch(() => undefined); | |
| void readSyncHistory({ kind: "codex-cli-sync", limit: 1 }) | |
| .then((entries) => { | |
| if (lastCodexCliSyncRun) return; // a real sync already ran; keep it | |
| const latestCodexEntry = entries.at(-1); | |
| if (latestCodexEntry?.kind === "codex-cli-sync") { | |
| lastCodexCliSyncRun = latestCodexEntry.run; | |
| } | |
| }) | |
| .catch(() => undefined); |
no vitest coverage for this race today — a test that interleaves syncAccountStorageFromCodexCli with a simultaneous cold getLastCodexCliSyncRun would reproduce it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/codex-cli/sync.ts
Line: 158-165
Comment:
**Background async read can silently overwrite a fresher in-memory result**
the fire-and-forget `readSyncHistory` callback unconditionally overwrites `lastCodexCliSyncRun`. if a sync run completes (calling `setLastCodexCliSyncRun`) between the moment this background read is queued and the moment its `.then` resolves, the fresh in-memory result gets clobbered by the stale on-disk entry. this is an async ordering race unique to node's event loop interleaving.
fix: guard the write with a check for whether `lastCodexCliSyncRun` has already been set:
```suggestion
void readSyncHistory({ kind: "codex-cli-sync", limit: 1 })
.then((entries) => {
if (lastCodexCliSyncRun) return; // a real sync already ran; keep it
const latestCodexEntry = entries.at(-1);
if (latestCodexEntry?.kind === "codex-cli-sync") {
lastCodexCliSyncRun = latestCodexEntry.run;
}
})
.catch(() => undefined);
```
no vitest coverage for this race today — a test that interleaves `syncAccountStorageFromCodexCli` with a simultaneous cold `getLastCodexCliSyncRun` would reproduce it.
How can I resolve this? If you propose a fix, please make it concise.| const history = await readSyncHistory({ kind: "codex-cli-sync" }); | ||
| const lastHistory = history.at(-1); | ||
| expect(lastHistory?.run.outcome).toBe("changed"); | ||
| expect(lastHistory?.run.summary.addedAccountCount).toBe(1); |
There was a problem hiding this comment.
TypeScript error: accessing discriminated-union member without a type guard
readSyncHistory returns Promise<SyncHistoryEntry[]>. SyncHistoryEntry is a discriminated union of CodexCliSyncHistoryEntry | LiveAccountSyncHistoryEntry. run only exists on CodexCliSyncHistoryEntry, so lastHistory?.run is a compile-time type error (Property 'run' does not exist on type 'LiveAccountSyncHistoryEntry').
the same pattern appears in test/live-account-sync.test.ts lines 162-163 where last?.outcome and last?.reason are accessed without narrowing (those fields only exist on LiveAccountSyncHistoryEntry).
fix by narrowing first:
// codex-cli-sync.test.ts
const lastHistory = history.at(-1);
expect(lastHistory?.kind).toBe("codex-cli-sync");
if (lastHistory?.kind === "codex-cli-sync") {
expect(lastHistory.run.outcome).toBe("changed");
expect(lastHistory.run.summary.addedAccountCount).toBe(1);
}// live-account-sync.test.ts
const last = history.at(-1);
expect(last?.kind).toBe("live-account-sync");
if (last?.kind === "live-account-sync") {
expect(last.outcome).toBe("error");
expect(["poll", "watch"]).toContain(last.reason);
}these will fail tsc --noEmit today.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/codex-cli-sync.test.ts
Line: 844-847
Comment:
**TypeScript error: accessing discriminated-union member without a type guard**
`readSyncHistory` returns `Promise<SyncHistoryEntry[]>`. `SyncHistoryEntry` is a discriminated union of `CodexCliSyncHistoryEntry | LiveAccountSyncHistoryEntry`. `run` only exists on `CodexCliSyncHistoryEntry`, so `lastHistory?.run` is a compile-time type error (`Property 'run' does not exist on type 'LiveAccountSyncHistoryEntry'`).
the same pattern appears in `test/live-account-sync.test.ts` lines 162-163 where `last?.outcome` and `last?.reason` are accessed without narrowing (those fields only exist on `LiveAccountSyncHistoryEntry`).
fix by narrowing first:
```ts
// codex-cli-sync.test.ts
const lastHistory = history.at(-1);
expect(lastHistory?.kind).toBe("codex-cli-sync");
if (lastHistory?.kind === "codex-cli-sync") {
expect(lastHistory.run.outcome).toBe("changed");
expect(lastHistory.run.summary.addedAccountCount).toBe(1);
}
```
```ts
// live-account-sync.test.ts
const last = history.at(-1);
expect(last?.kind).toBe("live-account-sync");
if (last?.kind === "live-account-sync") {
expect(last.outcome).toBe("error");
expect(["poll", "watch"]).toContain(last.reason);
}
```
these will fail `tsc --noEmit` today.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/codex-cli/sync.ts`:
- Around line 139-169: The function getLastCodexCliSyncRun currently does
blocking I/O via readLatestSyncHistorySync and kicks off a fire-and-forget
readSyncHistory, causing event-loop blocking and a race; fix by making the
hydration non-blocking and de-duplicated: either convert getLastCodexCliSyncRun
to async and replace readLatestSyncHistorySync with an awaited async reader (or
readLatestSyncHistory), or introduce a cached promise (e.g.
lastHistoryLoadPromise) that is set when the first load starts and awaited on
subsequent calls; ensure you update/await this promise before returning or
setting lastCodexCliSyncRun, remove fire-and-forget usage of readSyncHistory,
and update callers such as previewCodexCliSync to await the new async behavior
so callers no longer get null spuriously.
In `@lib/live-account-sync.ts`:
- Around line 261-273: The current await this.recordHistory(...) inside
runReload causes reloadInFlight to remain set until history I/O completes;
change recordHistory (and callers in runReload) to be non-blocking by enqueuing
history entries to a bounded in-memory queue serviced by a background worker
instead of awaiting appendSyncHistoryEntry directly—update recordHistory to push
{reason,status,message} into the queue and return immediately, implement a
background flush loop that retries on EBUSY (referencing appendSyncHistoryEntry)
with backoff and emits warnings when writes are slow or queue is near capacity,
and ensure runReload/reloadInFlight logic no longer awaits the write so
subsequent reload triggers aren’t delayed.
In `@lib/sync-history.ts`:
- Around line 38-39: The sync-history.ndjson file (HISTORY_FILE_NAME) can grow
unbounded because readSyncHistory currently loads the entire file; update the
implementation to limit growth by rotating or truncating old entries: modify
readSyncHistory to stream the file and only retain the last N records (or last X
bytes) instead of reading the whole file into memory, and update the append-side
logic (where new entries are written) to trigger rotation/truncation when file
size or line count exceeds a threshold (e.g., roll to a timestamped backup or
rewrite file keeping only the most recent N entries); reference
HISTORY_FILE_NAME and the readSyncHistory/any appendSyncHistory functions to
locate the code to change.
- Around line 65-72: The current in-process mutex implemented in withHistoryLock
(using historyMutex and release) only serializes writes within a single process
but does not prevent concurrent access from other processes; replace this with a
cross-process advisory lock (e.g., use the proper-lockfile package) around the
critical section that writes to the history file (the code that calls appendFile
/ the function wrapped by withHistoryLock) so you Acquire a file lock before
writing and Release it after, or if cross-process safety is not required, add a
clear comment/docstring on withHistoryLock/historyMutex stating the
single-process assumption and the risk on Windows (EBUSY/interleaved writes).
Ensure you update call sites that rely on withHistoryLock to use the new lock
API (acquire/release or try/finally) and handle lock acquisition failures
appropriately.
- Around line 97-104: The appendFile/writeFile sequence in lib/sync-history.ts
(using paths.historyPath and paths.latestPath with latestContent) is non-atomic
and lacks EBUSY retry; extract the EBUSY retry logic from
__resetSyncHistoryForTests into a reusable helper (e.g., writeFileWithRetry or
withEbusyRetry) and use it for both operations, then make the latest update
atomic by writing latestContent to a temporary file (e.g.,
sync-history-latest.json.tmp) and renaming it to paths.latestPath after
fs.appendFile completes successfully so the ndjson and latest JSON stay
consistent; ensure both the append and the temp-write/rename use the retry
helper to handle EBUSY on Windows.
- Around line 130-134: The current code maps every line to JSON.parse and a
single malformed line will throw and abort the whole read; update the parsing so
each line from lines (the variable produced above) is parsed inside a try/catch:
iterate lines, for each call JSON.parse(line) and cast to SyncHistoryEntry
inside the try block, push valid entries into parsed (or a new array), and in
the catch skip the malformed line (optionally log or count skipped entries) so
one corrupt line no longer crashes the entire read operation.
In `@test/codex-cli-sync.test.ts`:
- Around line 844-851: Add two regression tests: one to reproduce the race
between readLatestSyncHistorySync and the async readSyncHistory by writing a
live-account-sync entry via the same APIs used in tests, then call
getLastCodexCliSyncRun twice in rapid succession (after
__resetLastCodexCliSyncRunForTests if needed) and assert both calls return the
live entry; and another that mocks fs.appendFile (or the module wrapper used by
readSyncHistory) to throw an error object with code "EBUSY"/"EPERM" during
history write, then run the sync flow and assert the overall sync completes and
persisted outcome/summary values match expected (e.g., outcome "changed" and
addedAccountCount 1), ensuring you reference and exercise readSyncHistory,
readLatestSyncHistorySync, getLastCodexCliSyncRun, and
__resetLastCodexCliSyncRunForTests in the tests.
In `@test/live-account-sync.test.ts`:
- Around line 19-22: The test creates workDir using process.cwd(), which can
pollute the repo; change it to use the system temp directory instead: replace
process.cwd() in the join call that builds workDir in
test/live-account-sync.test.ts with os.tmpdir() (or tmpdir() if you import {
tmpdir } from 'os'), and add the corresponding import at the top of the file;
keep the rest of the path generation (the "tmp-live-sync" folder and
timestamp/random suffix) the same so cleanup and uniqueness behavior remain
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51df7680-c81b-40cd-8e65-45de11d2f010
📒 Files selected for processing (5)
lib/codex-cli/sync.tslib/live-account-sync.tslib/sync-history.tstest/codex-cli-sync.test.tstest/live-account-sync.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (2)
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/live-account-sync.tslib/codex-cli/sync.tslib/sync-history.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/codex-cli-sync.test.tstest/live-account-sync.test.ts
🔇 Additional comments (8)
lib/codex-cli/sync.ts (2)
124-137: looks goodasync conversion is clean. errors are caught and logged without leaking sensitive data. callers properly await.
171-174: lgtmreset function correctly clears both in-memory state and the hydration guard for test isolation.
lib/live-account-sync.ts (1)
197-213: lgtm with minor note
recordHistorycorrectly builds the entry. note thatpath(line 208) is the raw filesystem path, which may contain the username on some systems. this is persisted to disk. if that's a privacy concern, consider hashing or redacting it. not blocking.test/codex-cli-sync.test.ts (1)
45-46: lgtmtest setup/teardown correctly configures and resets sync history for isolation.
Also applies to: 75-76
test/live-account-sync.test.ts (2)
36-59: nice windows cleanup handlingthe retry loop with exponential backoff for EBUSY/EPERM/ENOTEMPTY is solid for windows filesystem semantics. the debug escape hatch is helpful for local troubleshooting.
116-167: good coverage of error recordingtest verifies spy invocation, file existence, content, and parsed history entry. thorough for the error path.
lib/sync-history.ts (2)
166-196: solid test helpers
__resetSyncHistoryForTestswith EBUSY/EPERM retry andconfigureSyncHistoryForTestsprovide good test isolation. the introspection helpers enable thorough test assertions.
151-164: synchronous i/o
readFileSyncblocks the event loop. already flagged inlib/codex-cli/sync.ts:149context — consider an async alternative.
| export function getLastCodexCliSyncRun(): CodexCliSyncRun | null { | ||
| return lastCodexCliSyncRun | ||
| ? { | ||
| ...lastCodexCliSyncRun, | ||
| summary: { ...lastCodexCliSyncRun.summary }, | ||
| } | ||
| : null; | ||
| if (lastCodexCliSyncRun) { | ||
| return { | ||
| ...lastCodexCliSyncRun, | ||
| summary: { ...lastCodexCliSyncRun.summary }, | ||
| }; | ||
| } | ||
|
|
||
| if (!lastHistoryLoadAttempted) { | ||
| lastHistoryLoadAttempted = true; | ||
| const latest = readLatestSyncHistorySync(); | ||
| const cloned = cloneSyncHistoryEntry(latest); | ||
| if (cloned?.kind === "codex-cli-sync") { | ||
| lastCodexCliSyncRun = cloned.run; | ||
| return { | ||
| ...cloned.run, | ||
| summary: { ...cloned.run.summary }, | ||
| }; | ||
| } | ||
| void readSyncHistory({ kind: "codex-cli-sync", limit: 1 }) | ||
| .then((entries) => { | ||
| const latestCodexEntry = entries.at(-1); | ||
| if (latestCodexEntry?.kind === "codex-cli-sync") { | ||
| lastCodexCliSyncRun = latestCodexEntry.run; | ||
| } | ||
| }) | ||
| .catch(() => undefined); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# check if callers of getLastCodexCliSyncRun can tolerate async
rg -n 'getLastCodexCliSyncRun\s*\(' --type=tsRepository: ndycode/codex-multi-auth
Length of output: 384
🏁 Script executed:
#!/bin/bash
# Check context around line 486 in lib/codex-cli/sync.ts
sed -n '480,495n' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check the implementation of readLatestSyncHistorySync
sed -n '150,160n' lib/sync-history.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check test coverage for lazy-load behavior
sed -n '835,855n' test/codex-cli-sync.test.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Verify if getLastCodexCliSyncRun is called from an async context that could be converted
rg -B5 'getLastCodexCliSyncRun\(\)' lib/codex-cli/sync.ts | head -20Repository: ndycode/codex-multi-auth
Length of output: 394
🏁 Script executed:
#!/bin/bash
# Read the full getLastCodexCliSyncRun function implementation
sed -n '139,175n' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check the readLatestSyncHistorySync implementation
rg -A10 'function readLatestSyncHistorySync' lib/sync-history.tsRepository: ndycode/codex-multi-auth
Length of output: 520
🏁 Script executed:
#!/bin/bash
# Check previewCodexCliSync function signature and whether it's async
rg -B2 'const lastSync = getLastCodexCliSyncRun' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Find readLatestSyncHistorySync to verify readFileSync usage
rg 'readLatestSyncHistorySync|readFileSync' lib/sync-history.tsRepository: ndycode/codex-multi-auth
Length of output: 268
🏁 Script executed:
#!/bin/bash
# Read full getLastCodexCliSyncRun implementation
sed -n '139,175n' lib/codex-cli/sync.ts | cat -nRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check if previewCodexCliSync is async
rg -B5 -A15 'export function previewCodexCliSync' lib/codex-cli/sync.ts | head -30Repository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check readSyncHistory call and fire-and-forget pattern
sed -n '158,167n' lib/codex-cli/sync.ts | cat -nRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check test coverage - what does the test verify?
sed -n '835,860n' test/codex-cli-sync.test.ts | cat -nRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check actual file sizes and line counts
wc -l lib/codex-cli/sync.ts test/codex-cli-sync.test.tsRepository: ndycode/codex-multi-auth
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Read the entire getLastCodexCliSyncRun function using rg
rg -A30 'export function getLastCodexCliSyncRun' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 875
🏁 Script executed:
#!/bin/bash
# Check previewCodexCliSync function
rg -A20 'export function previewCodexCliSync' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check how readSyncHistory is called in the function
rg -B5 'readSyncHistory.*codex-cli-sync' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 209
🏁 Script executed:
#!/bin/bash
# Get full previewCodexCliSync function signature and context
rg -B2 -A40 'function previewCodexCliSync' lib/codex-cli/sync.ts | head -50Repository: ndycode/codex-multi-auth
Length of output: 1259
🏁 Script executed:
#!/bin/bash
# Read the test cases in detail
sed -n '835,875n' test/codex-cli-sync.test.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check what kind of entries could be in sync history
rg 'kind.*codex-cli-sync|kind.*live-account-sync' lib/sync-history.ts | head -10Repository: ndycode/codex-multi-auth
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Check if there are other async calls near getLastCodexCliSyncRun to assess hot path claim
sed -n '480,510n' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Read the test file content to understand coverage
cat test/codex-cli-sync.test.ts | grep -A20 -B5 'getLastCodexCliSyncRun'Repository: ndycode/codex-multi-auth
Length of output: 2125
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for the lazy-load behavior or race condition
rg -n 'lastHistoryLoadAttempted|readLatestSyncHistorySync' test/codex-cli-sync.test.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Verify if __resetLastCodexCliSyncRunForTests exists and is used
rg -n '__resetLastCodexCliSyncRunForTests' lib/codex-cli/sync.ts test/codex-cli-sync.test.tsRepository: ndycode/codex-multi-auth
Length of output: 435
🏁 Script executed:
#!/bin/bash
# Check the __resetLastCodexCliSyncRunForTests implementation
sed -n '171,175n' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check if lastHistoryLoadAttempted is reset
rg -B2 -A5 '__resetLastCodexCliSyncRunForTests' lib/codex-cli/sync.tsRepository: ndycode/codex-multi-auth
Length of output: 230
blocking sync i/o in async hot path and fire-and-forget race
readLatestSyncHistorySync() at lib/codex-cli/sync.ts:149 uses readFileSync (lib/sync-history.ts) and blocks the event loop on the first call to getLastCodexCliSyncRun(), which is called from the async previewCodexCliSync() (lib/codex-cli/sync.ts:486) on every preview request.
the fire-and-forget async call at lines 158-165 creates a subtle race: if readLatestSyncHistorySync() returns a live-account-sync entry (kind check fails at line 154), the async readSyncHistory() is kicked off without awaiting. a second call to getLastCodexCliSyncRun() before that resolves returns null even though history contains a valid codex-cli-sync entry. the test at test/codex-cli-sync.test.ts:849-860 covers the lazy-load behavior but doesn't verify the concurrent-call scenario.
consider converting the entire function to async or caching the hydration promise to avoid both the blocking i/o and the race condition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/codex-cli/sync.ts` around lines 139 - 169, The function
getLastCodexCliSyncRun currently does blocking I/O via readLatestSyncHistorySync
and kicks off a fire-and-forget readSyncHistory, causing event-loop blocking and
a race; fix by making the hydration non-blocking and de-duplicated: either
convert getLastCodexCliSyncRun to async and replace readLatestSyncHistorySync
with an awaited async reader (or readLatestSyncHistory), or introduce a cached
promise (e.g. lastHistoryLoadPromise) that is set when the first load starts and
awaited on subsequent calls; ensure you update/await this promise before
returning or setting lastCodexCliSyncRun, remove fire-and-forget usage of
readSyncHistory, and update callers such as previewCodexCliSync to await the new
async behavior so callers no longer get null spuriously.
| await this.recordHistory(reason, "success"); | ||
| } catch (error) { | ||
| this.errorCount += 1; | ||
| log.warn("Live account sync reload failed", { | ||
| reason, | ||
| path: summarizeWatchPath(targetPath), | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| await this.recordHistory( | ||
| reason, | ||
| "error", | ||
| error instanceof Error ? error.message : String(error), | ||
| ); |
There was a problem hiding this comment.
awaiting history write blocks reload completion
recordHistory is awaited inside runReload, so any delay in appendSyncHistoryEntry (e.g., windows file locks, slow disk) extends the time before reloadInFlight clears. during that window, subsequent poll/watch triggers queue behind it.
the current appendSyncHistoryEntry does not retry on EBUSY — it logs and moves on. on windows, where file locks are more common, you may see sporadic history gaps.
if history write latency becomes a concern, consider fire-and-forget with a bounded in-memory queue or at least logging when writes are slow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/live-account-sync.ts` around lines 261 - 273, The current await
this.recordHistory(...) inside runReload causes reloadInFlight to remain set
until history I/O completes; change recordHistory (and callers in runReload) to
be non-blocking by enqueuing history entries to a bounded in-memory queue
serviced by a background worker instead of awaiting appendSyncHistoryEntry
directly—update recordHistory to push {reason,status,message} into the queue and
return immediately, implement a background flush loop that retries on EBUSY
(referencing appendSyncHistoryEntry) with backoff and emits warnings when writes
are slow or queue is near capacity, and ensure runReload/reloadInFlight logic no
longer awaits the write so subsequent reload triggers aren’t delayed.
| const HISTORY_FILE_NAME = "sync-history.ndjson"; | ||
| const LATEST_FILE_NAME = "sync-history-latest.json"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
unbounded history file growth
sync-history.ndjson grows without limit. readSyncHistory loads the entire file into memory (line 126-129). after weeks/months of usage, this could become large.
consider adding rotation (e.g., keep last N entries or last X bytes) or truncating old entries periodically. alternatively, if only the latest entry is typically needed, consider if the ndjson history is even necessary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sync-history.ts` around lines 38 - 39, The sync-history.ndjson file
(HISTORY_FILE_NAME) can grow unbounded because readSyncHistory currently loads
the entire file; update the implementation to limit growth by rotating or
truncating old entries: modify readSyncHistory to stream the file and only
retain the last N records (or last X bytes) instead of reading the whole file
into memory, and update the append-side logic (where new entries are written) to
trigger rotation/truncation when file size or line count exceeds a threshold
(e.g., roll to a timestamped backup or rewrite file keeping only the most recent
N entries); reference HISTORY_FILE_NAME and the readSyncHistory/any
appendSyncHistory functions to locate the code to change.
| function withHistoryLock<T>(fn: () => Promise<T>): Promise<T> { | ||
| const previous = historyMutex; | ||
| let release: () => void = () => {}; | ||
| historyMutex = new Promise<void>((resolve) => { | ||
| release = resolve; | ||
| }); | ||
| return previous.then(fn).finally(() => release()); | ||
| } |
There was a problem hiding this comment.
in-process-only mutex
lib/sync-history.ts:65-72 serializes writes within a single process, but multiple extension hosts or codex-cli processes can run concurrently. on windows especially, concurrent appendFile calls to the same file can interleave bytes or fail with EBUSY.
consider using proper-lockfile or advisory file locks if cross-process safety is required. if not, document the single-process assumption.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sync-history.ts` around lines 65 - 72, The current in-process mutex
implemented in withHistoryLock (using historyMutex and release) only serializes
writes within a single process but does not prevent concurrent access from other
processes; replace this with a cross-process advisory lock (e.g., use the
proper-lockfile package) around the critical section that writes to the history
file (the code that calls appendFile / the function wrapped by withHistoryLock)
so you Acquire a file lock before writing and Release it after, or if
cross-process safety is not required, add a clear comment/docstring on
withHistoryLock/historyMutex stating the single-process assumption and the risk
on Windows (EBUSY/interleaved writes). Ensure you update call sites that rely on
withHistoryLock to use the new lock API (acquire/release or try/finally) and
handle lock acquisition failures appropriately.
| await fs.appendFile(paths.historyPath, line, { | ||
| encoding: "utf8", | ||
| mode: 0o600, | ||
| }); | ||
| await fs.writeFile(paths.latestPath, latestContent, { | ||
| encoding: "utf8", | ||
| mode: 0o600, | ||
| }); |
There was a problem hiding this comment.
non-atomic write sequence and no EBUSY retry
lib/sync-history.ts:97-104 does appendFile then writeFile. if the process crashes between them, sync-history-latest.json will be stale compared to the ndjson.
additionally, neither call retries on EBUSY. on windows, if another process holds the file, the write fails and is logged but lost. since __resetSyncHistoryForTests already has EBUSY retry logic, consider extracting that into a reusable helper and applying it here.
for atomicity, consider writing to sync-history-latest.json.tmp then renaming.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sync-history.ts` around lines 97 - 104, The appendFile/writeFile sequence
in lib/sync-history.ts (using paths.historyPath and paths.latestPath with
latestContent) is non-atomic and lacks EBUSY retry; extract the EBUSY retry
logic from __resetSyncHistoryForTests into a reusable helper (e.g.,
writeFileWithRetry or withEbusyRetry) and use it for both operations, then make
the latest update atomic by writing latestContent to a temporary file (e.g.,
sync-history-latest.json.tmp) and renaming it to paths.latestPath after
fs.appendFile completes successfully so the ndjson and latest JSON stay
consistent; ensure both the append and the temp-write/rename use the retry
helper to handle EBUSY on Windows.
| const lines = content | ||
| .split(/\r?\n/) | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean); | ||
| const parsed = lines.map((line) => JSON.parse(line) as SyncHistoryEntry); |
There was a problem hiding this comment.
corrupt line crashes entire read
lib/sync-history.ts:134 parses every line with JSON.parse without try/catch. a single malformed line (disk corruption, partial write) throws and the entire read returns [] via the outer catch.
consider parsing each line individually and skipping invalid ones:
🔧 proposed fix
- const parsed = lines.map((line) => JSON.parse(line) as SyncHistoryEntry);
+ const parsed: SyncHistoryEntry[] = [];
+ for (const line of lines) {
+ try {
+ parsed.push(JSON.parse(line) as SyncHistoryEntry);
+ } catch {
+ log.debug("Skipping corrupt sync history line", { line: line.slice(0, 100) });
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lines = content | |
| .split(/\r?\n/) | |
| .map((line) => line.trim()) | |
| .filter(Boolean); | |
| const parsed = lines.map((line) => JSON.parse(line) as SyncHistoryEntry); | |
| const lines = content | |
| .split(/\r?\n/) | |
| .map((line) => line.trim()) | |
| .filter(Boolean); | |
| const parsed: SyncHistoryEntry[] = []; | |
| for (const line of lines) { | |
| try { | |
| parsed.push(JSON.parse(line) as SyncHistoryEntry); | |
| } catch { | |
| log.debug("Skipping corrupt sync history line", { line: line.slice(0, 100) }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sync-history.ts` around lines 130 - 134, The current code maps every line
to JSON.parse and a single malformed line will throw and abort the whole read;
update the parsing so each line from lines (the variable produced above) is
parsed inside a try/catch: iterate lines, for each call JSON.parse(line) and
cast to SyncHistoryEntry inside the try block, push valid entries into parsed
(or a new array), and in the catch skip the malformed line (optionally log or
count skipped entries) so one corrupt line no longer crashes the entire read
operation.
| const history = await readSyncHistory({ kind: "codex-cli-sync" }); | ||
| const lastHistory = history.at(-1); | ||
| expect(lastHistory?.run.outcome).toBe("changed"); | ||
| expect(lastHistory?.run.summary.addedAccountCount).toBe(1); | ||
| __resetLastCodexCliSyncRunForTests(); | ||
| const persisted = getLastCodexCliSyncRun(); | ||
| expect(persisted?.outcome).toBe("changed"); | ||
| expect(persisted?.summary.addedAccountCount).toBe(1); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
missing regression tests for concurrency and windows edge cases
test/codex-cli-sync.test.ts:844-851 validates basic persistence, but per lib/** guidelines:
- no test covers the race when
readLatestSyncHistorySyncreturns alive-account-syncentry and the fire-and-forgetreadSyncHistoryhasn't resolved yet. - no test simulates EBUSY/EPERM during history writes to verify graceful degradation on windows.
consider adding:
- a test that writes a
live-account-syncentry first, then callsgetLastCodexCliSyncRuntwice in quick succession. - a test that mocks
fs.appendFileto throw{ code: "EBUSY" }and asserts the sync still completes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/codex-cli-sync.test.ts` around lines 844 - 851, Add two regression
tests: one to reproduce the race between readLatestSyncHistorySync and the async
readSyncHistory by writing a live-account-sync entry via the same APIs used in
tests, then call getLastCodexCliSyncRun twice in rapid succession (after
__resetLastCodexCliSyncRunForTests if needed) and assert both calls return the
live entry; and another that mocks fs.appendFile (or the module wrapper used by
readSyncHistory) to throw an error object with code "EBUSY"/"EPERM" during
history write, then run the sync flow and assert the overall sync completes and
persisted outcome/summary values match expected (e.g., outcome "changed" and
addedAccountCount 1), ensuring you reference and exercise readSyncHistory,
readLatestSyncHistorySync, getLastCodexCliSyncRun, and
__resetLastCodexCliSyncRunForTests in the tests.
| workDir = join( | ||
| tmpdir(), | ||
| process.cwd(), | ||
| "tmp-live-sync", | ||
| `codex-live-sync-${Date.now()}-${Math.random().toString(36).slice(2)}`, |
There was a problem hiding this comment.
test temp dir placed inside project root
test/live-account-sync.test.ts:19-22 creates workDir under process.cwd() instead of tmpdir(). this can:
- pollute the repo with leftover
tmp-live-sync/dirs if cleanup fails. - interfere with git status / file watchers.
- cause path-length issues on windows with deeply nested test names.
test/codex-cli-sync.test.ts:41 correctly uses tmpdir(). align this test to match.
🔧 proposed fix
+import { tmpdir } from "node:os";
// ...
workDir = join(
- process.cwd(),
- "tmp-live-sync",
+ tmpdir(),
`codex-live-sync-${Date.now()}-${Math.random().toString(36).slice(2)}`,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| workDir = join( | |
| tmpdir(), | |
| process.cwd(), | |
| "tmp-live-sync", | |
| `codex-live-sync-${Date.now()}-${Math.random().toString(36).slice(2)}`, | |
| workDir = join( | |
| tmpdir(), | |
| `codex-live-sync-${Date.now()}-${Math.random().toString(36).slice(2)}`, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/live-account-sync.test.ts` around lines 19 - 22, The test creates
workDir using process.cwd(), which can pollute the repo; change it to use the
system temp directory instead: replace process.cwd() in the join call that
builds workDir in test/live-account-sync.test.ts with os.tmpdir() (or tmpdir()
if you import { tmpdir } from 'os'), and add the corresponding import at the top
of the file; keep the rest of the path generation (the "tmp-live-sync" folder
and timestamp/random suffix) the same so cleanup and uniqueness behavior remain
unchanged.
1 participant
Summary
Summary by cubic
Add durable sync history and a last-result log for
codex-cliand live account sync. Persists results across restarts with safer, hardened reads.New Features
lib/sync-history.ts: append-only NDJSON history and alatestJSON file; in-process lock; waits for pending writes before reads; test-configurable log dir.codex-cli/sync: writes each run to history; on first access loads fromlatest, falling back to recent history if the kind mismatches; logs failures without breaking flow.live-account-sync: records each reload attempt (watch/poll/manual) with outcome, message, path, and snapshot.configureSyncHistoryForTestsand__resetSyncHistoryForTestshelpers.Bug Fixes
Written for commit 07970dd. Summary will update on new commits.
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this PR adds an append-only NDJSON history log and a
latestJSON sidecar for bothcodex-cli-syncandlive-account-sync, with an in-process mutex for serialized writes and a lazy cold-load path on first access. the foundation is sound, but there are a few issues that need attention before merge:getLastCodexCliSyncRun: the fire-and-forgetreadSyncHistorycallback insync.tsunconditionally writes tolastCodexCliSyncRun, with no guard for whether a real sync has already populated it. on a fast startup + immediate sync, the stale disk value overwrites the fresh in-memory result.readSyncHistoryreturnsSyncHistoryEntry[](the discriminated union), so accessing.run(codex-only) or.outcome/.reason(live-sync-only) without a type guard failstsc. this affects both new test blocks.this.currentPath(full filesystem path, including username on windows) is persisted verbatim into the ndjson log and latest file. the existingsummarizeWatchPathhelper should be used instead.sync-history.ndjson;readSyncHistoryreads the entire file before slicing, so evenlimit: 1is expensive on a long-running install.process.cwd(): the change fromos.tmpdir()toprocess.cwd()leavestmp-live-sync/artifacts in the repo root on test failures or whenDEBUG_SYNC_HISTORY=1.Confidence Score: 2/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant Caller participant sync as codex-cli/sync.ts participant las as live-account-sync.ts participant sh as sync-history.ts participant FS as Filesystem Note over sync,sh: normal sync run Caller->>sync: syncAccountStorageFromCodexCli() sync->>sync: reconcile accounts sync->>sh: appendSyncHistoryEntry(entry) sh->>sh: withHistoryLock() sh->>FS: appendFile(sync-history.ndjson) sh->>FS: writeFile(sync-history-latest.json) sh-->>sync: resolved sync-->>Caller: {storage, changed} Note over sync,sh: cold startup — lazy load Caller->>sync: getLastCodexCliSyncRun() [lastCodexCliSyncRun=null] sync->>sh: readLatestSyncHistorySync() [sync] sh->>FS: readFileSync(latestPath) FS-->>sh: content or ENOENT alt latest is codex-cli-sync entry sh-->>sync: CodexCliSyncHistoryEntry sync-->>Caller: CodexCliSyncRun (from latest file) else latest is wrong kind or missing sh-->>sync: null sync-)sh: readSyncHistory({kind,limit:1}) [void/background] Note over sync,sh: ⚠ race: if a real sync runs before<br/>this resolves, stale result overwrites fresh one sync-->>Caller: null (returns immediately) sh->>FS: readFile(historyPath) [full file read] sh-->>sync: entries sync->>sync: lastCodexCliSyncRun = entries.at(-1).run end Note over las,sh: live reload with history Caller->>las: runReload(reason) las->>las: this.reload() las->>sh: await recordHistory(reason, "success") sh->>sh: appendSyncHistoryEntry(entry) sh->>FS: appendFile + writeFile sh-->>las: resolved las->>las: publishSnapshot()Prompt To Fix All With AI
Last reviewed commit: 07970dd
Context used: