Skip to content

fix(isolation-core): bound stalled git commands - #8998

Closed
code-yeongyu wants to merge 5 commits into
devfrom
fix/win-ci-isolation-nested-hang
Closed

code-yeongyu wants to merge 5 commits into
devfrom
fix/win-ci-isolation-nested-hang

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Fixes #8997
Refs #8324

Summary

isolation-core could wait forever when a local Git subprocess stopped making progress. The observed Windows failure left git ls-files --others --exclude-standard -z alive until Bun's 30-second test timeout killed it, after which fixture cleanup raced the still-held working directory and failed with EBUSY.

This PR closes the loop at three layers:

  1. every runGit call has a bounded lifetime and typed full-tree timeout teardown;
  2. baseline reads suppress optional index locks, fsmonitor, and persistent untracked-cache state;
  3. because those reads are idempotent, one timed-out attempt is logged and retried once in a fresh process.

Root cause

The log does not expose the internal Git-for-Windows wait that made this one invocation stop progressing. The original product defect was relying on every external Git process to exit eventually; the follow-up risk was that a deadline alone would turn the same stall into a deterministic red. Baseline reads therefore also remove Git's optional background/index acceleration paths and recover once from a transient external stall.

Fix

  • packages/isolation-core/src/git/command.ts:6-24
    • Adds a 120-second default Git command deadline.
    • Adds exported GitCommandTimeoutError with exit code 124 and the configured deadline.
  • packages/isolation-core/src/git/command.ts:48-82
    • Waits for Windows taskkill /T /F.
    • Falls back to direct child termination when taskkill cannot start or exits nonzero.
  • packages/isolation-core/src/git/command.ts:171-207
    • Races child settlement with the command deadline.
    • Terminates the process tree, closes inherited pipes, preserves the typed timeout, and clears the timer.
  • packages/isolation-core/src/git/baseline.ts
    • Applies a 10-second per-attempt deadline, so two bounded attempts plus teardown remain inside the 30-second Windows harness budget.
    • Sets GIT_OPTIONAL_LOCKS=0, preventing optional lock-taking/index refresh side effects.
    • Runs each read with -c core.fsmonitor=false -c core.untrackedCache=false, avoiding a Git-for-Windows fsmonitor daemon/hook and stale persistent untracked-cache state in freshly copied repositories.
    • Logs a typed timeout and retries that same read exactly once with a fresh process. Non-timeout failures and all mutating commands are never retried.
    • Leaves core.fscache unchanged: Git for Windows documents it as per-process bulk lstat caching, not a daemon or persistent index feature.
  • packages/isolation-core/src/git/command.test.ts:79
    • Adds a deterministic real-Git regression: a seven-second alias must terminate at a 50 ms deadline and release its Windows fixture root.
  • packages/isolation-core/src/git/baseline.test.ts
    • Forces the first ls-files attempt to stall through a real Git alias.
    • Proves the second fresh process completes, receives every hardening flag, logs exactly one retry, and leaves .git/index byte-identical.

Git documentation basis

  • GIT_OPTIONAL_LOCKS: false skips optional operations requiring locks, including index refresh side effects.
  • core.fsmonitor: true enables the built-in monitor daemon on Windows/macOS; otherwise it may name a hook command.
  • core.untrackedCache and git-update-index: the cache is persisted in the index and is added/removed when the index is read.
  • Git for Windows core.fscache: bulk-reads and caches directory lstat data inside Git; it does not create a background daemon or persistent cache, so this PR does not disable it.

QA & Evidence

RED

  • Command: bun test packages/isolation-core/src/git/command.test.ts
  • Observed before implementation: the deadline regression completed normally after 7024.47ms; expected GitCommandTimeoutError, received undefined.
  • Artifact: .omo/evidence/20260927-win-ci-isolation-nested-hang/RED.md (local, intentionally untracked)
  • SHA-256: 24b6147ec696ffb6c4ea2ba3381ae7aca008874f1e5b9b2226d287a8b3bdd3e0

Focused GREEN

  • Command: bun test packages/isolation-core/src/git/command.test.ts packages/isolation-core/src/git/baseline.test.ts
  • Observed: 19 pass, 0 fail; includes the command-deadline test and the stall-then-success baseline retry test.

Package suite

  • Command: bun test packages/isolation-core
  • Observed: 156 pass, 0 fail, 7 skip; 163 tests across 24 files.

Static gates

  • bunx tsgo --noEmit -p packages/isolation-core/tsconfig.json — exit 0.
  • bunx biome check <five changed files> — exit 0.
  • git diff --check — exit 0.
  • LSP diagnostics were unavailable because this workspace uses tsgo and has no TypeScript server installation; package tsgo is the authoritative type gate.

Manual module exercise

A minimal Bun driver imported the real module and launched a live seven-second Git alias with a 50 ms deadline:

{"name":"GitCommandTimeoutError","exitCode":124,"timeoutMs":50,"elapsedMs":53}

A second driver imported captureRepoBaseline, forced the first untracked-file read to time out, used the production retry logger, and completed through the fresh process:

[isolation-core] retrying timed-out read-only Git command { args: ["ls-files", ...], timeoutMs: 50 }
{"attempts":2,"elapsedMs":118,"untrackedFiles":["untracked"]}

Non-Windows control

  • Command: bun test packages/isolation-core/src/git/baseline.test.ts --rerun-each 50
  • Observed: 550 pass, 0 fail; 50/50 file runs.

Local QA artifact

  • Artifact: .omo/evidence/20260927-win-ci-isolation-nested-hang/LOCAL-QA.md (local, intentionally untracked)
  • SHA-256: e400fe54024c986c20cfdab1002d6afabddd07525af746dd3fdf990f1f907982

Windows proof

The first three runs were cancelled after lead review expanded the fix. Three replacement windows-latest runs tested updated soak commit 851930d7dc86a18344eb8cfdf4719cc119fa7e25, each repeating packages/isolation-core/src/git 20 times:

Run Repeated-test step Workflow conclusion
36334212006 success success
36334218625 success success
36334225286 success success

Total focused proof: 60 successful Windows iterations, with telemetry upload and job-summary steps also successful in every run.

Risks & residuals

  • The exact internal Git-for-Windows stall is not observable in the failed job after the telemetry artifact was superseded by a later rerun. The process identity and unbounded supervisor path are directly observed.
  • The default 120-second deadline bounds every other runGit caller without constraining normal merge operations to the shorter baseline budget.
  • The only retry is one fresh process after GitCommandTimeoutError, inside the read-only baseline path. No mutating Git command is retried.
  • No platform skip, assertion deletion, polling loop, fixed synchronization sleep, or test-timeout increase was added.

@code-yeongyu

Copy link
Copy Markdown
Owner Author

Focused Windows soaks of the final head (windows-latest, packages/isolation-core/src/git, 20 iterations each): 36334212006 success, 36334218625 success, 36334225286 success.

@code-yeongyu
code-yeongyu marked this pull request as ready for review September 27, 2026 17:02
@code-yeongyu
code-yeongyu force-pushed the fix/win-ci-isolation-nested-hang branch from c5b536c to b6cfd61 Compare September 27, 2026 17:28
@github-actions github-actions Bot added the omo-senpi Changes under packages/omo-senpi label Sep 27, 2026
…out instead of a real stalled git (#8997)

The stand-in stalled process left a Windows grandchild holding the fixture directory past teardown (EBUSY, seen in integration run 36336999917 attempt 2). The deadline and tree teardown of a real stalled git stay covered in command.test.ts; this test now proves only the retry contract.
@code-yeongyu

Copy link
Copy Markdown
Owner Author

Re-soak after the retry-test fix b76fdd0 (the earlier test's real stalled git left a Windows grandchild holding the fixture dir, EBUSY, in integration run 36336999917 attempt 2): windows-latest, packages/isolation-core/src/git, 20 iterations each: 36340075853 success, 36340077334 success, 36340078993 success.

@code-yeongyu

Copy link
Copy Markdown
Owner Author

Superseded by #8961, which lands this fix together with the other win-ci fixes as one merge (all commits of this branch kept, bundles regenerated once over the merged sources). The evidence in this PR (RED/GREEN, focused Windows soaks) still applies; the issue is closed by #8961's Fixes line.

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

Labels

isolation-core Changes under packages/isolation-core omo-senpi Changes under packages/omo-senpi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isolation-core: nested baseline can hang indefinitely when Git stalls

1 participant