Skip to content

[WRONG BRANCH] fix(command-code): disable repository fsmonitor during git metadata collection #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
luvs01 wants to merge 1 commit into from
Closed

[WRONG BRANCH] fix(command-code): disable repository fsmonitor during git metadata collection #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/adapters/command-code.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ async function gitWorkspaceInfo(cwd: string | undefined): Promise<GitWorkspaceIn
if (cached && Date.now() - cached.collectedAt < WORKSPACE_METADATA_TTL_MS) return cached.value;
const run = async (args: string[]): Promise<string> => {
try {
const { stdout } = await execFile("git", args, { cwd, encoding: "utf8", timeout: 2000, windowsHide: true });
// `git status` may invoke a repository-configured fsmonitor. Disable it for every
// metadata read so inspecting an untrusted workspace cannot launch local commands.
const { stdout } = await execFile("git", ["-c", "core.fsmonitor=false", ...args], { cwd, encoding: "utf8", timeout: 2000, windowsHide: true });
return stdout.trim();
} catch {
return "";
Expand Down
25 changes: 25 additions & 0 deletions tests/command-code-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { afterEach, describe, expect, test } from "bun:test";
import { execFileSync } from "node:child_process";
import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createCommandCodeAdapter } from "../src/adapters/command-code";
import { loginCommandCode, parseCommandCodeCallback, shouldImportLocalCommandCodeAuth } from "../src/oauth/command-code";
import { buildModelsRequest, OAUTH_PROVIDERS } from "../src/oauth";
Expand Down Expand Up @@ -199,6 +203,27 @@ describe("Command Code provider", () => {
expect(body.config.currentBranch).not.toBe("");
});

test.skipIf(process.platform === "win32")("does not execute a repository-configured fsmonitor while collecting metadata", async () => {
const workspace = await mkdtemp(join(tmpdir(), "ocx-command-code-"));
const marker = join(workspace, "fsmonitor-ran");
const monitor = join(workspace, "fsmonitor.sh");
const previousCwd = process.cwd();
try {
await writeFile(monitor, `#!/bin/sh\nprintf invoked > ${JSON.stringify(marker)}\n`);
await chmod(monitor, 0o755);
execFileSync("git", ["init", "-q"], { cwd: workspace });
execFileSync("git", ["config", "core.fsmonitor", monitor], { cwd: workspace });

process.chdir(workspace);
const built = await builtRequest(parsed());
Comment on lines +217 to +218

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- target test section ---'
sed -n '160,245p' tests/command-code-provider.test.ts

printf '%s\n' '--- concurrency and cwd usage in tests ---'
rg -n --glob 'tests/**' 'test\.concurrent|describe\.concurrent|--concurrent|process\.chdir|builtRequest' .

printf '%s\n' '--- test configuration and scripts ---'
rg -n --glob 'package.json' --glob 'bunfig.toml' --glob '*.json' --glob '*.toml' 'bun test|concurrent|test' .

Repository: luvs01/opencodex

Length of output: 50373

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- test runner ---'
sed -n '1,180p' scripts/test.ts
printf '%s\n' '--- package and Bun test config ---'
sed -n '30,48p' package.json
cat bunfig.toml
printf '%s\n' '--- test file setup and helper ---'
sed -n '1,75p' tests/command-code-provider.test.ts
printf '%s\n' '--- adapter definitions and cwd-sensitive calls ---'
rg -n -g '*.ts' 'createCommandCodeAdapter|buildRequest|process\.cwd|process\.chdir|workingDir|cwd' src tests | head -160

Repository: luvs01/opencodex

Length of output: 29147

🏁 Script executed:

#!/bin/bash
set -eu

adapter_file="$(fd -t f -i 'command-code' src | head -1)"
printf '%s\n' "--- adapter file: $adapter_file ---"
wc -l "$adapter_file"
rg -n -C 8 'process\.cwd|execFile|git|workingDir|buildRequest|createCommandCodeAdapter' "$adapter_file"
printf '%s\n' '--- direct test-runner concurrency declarations ---'
rg -n --glob 'tests/**' --glob '!tests/command-code-provider.test.ts' 'test\.concurrent|describe\.concurrent|concurrent' . | head -80

Repository: luvs01/opencodex

Length of output: 17698

🌐 Web query:

Bun test default execution order concurrency: does Bun run tests serially by default, and what does the --concurrent flag change?

💡 Result:

Bun test execution operates under a clear distinction between file-level parallelism and test-level concurrency. Default Execution Order By default, Bun runs test files and the tests within them serially [1][2]. In this standard mode, all tests are executed in a single process, ensuring tests run in the order they are defined within their files [1][3]. The --concurrent Flag The --concurrent flag changes how individual tests within files are executed. When passed, it treats all tests in your suite as if they were declared with test.concurrent, allowing them to run concurrently within the same process [3][4]. Key Distinctions to Note: 1. File-level Parallelism (--parallel): While --concurrent focuses on running individual tests within files simultaneously, the --parallel flag is used to spread multiple test files across different CPU worker processes [1][3][5]. 2. Concurrent Glob Patterns (concurrentTestGlob): You can configure specific test files to run concurrently automatically using the concurrentTestGlob setting in bunfig.toml, which applies the same effect as the --concurrent flag to files matching the provided pattern [4][6]. 3. Execution Control: When using concurrency (via --concurrent, describe.concurrent, or concurrentTestGlob), you can use test.serial to force specific tests to run sequentially if they have dependencies that require order [7]. The default maximum number of tests allowed to run concurrently is 20, which can be adjusted using the --max-concurrency flag [3][7].

Citations:

🏁 Script executed:

#!/bin/bash
set -eu

node - <<'JS'
const original = process.cwd();
const workspace = "/tmp";
let observed;
async function buildRequestLike() {
  const captured = process.cwd();
  await new Promise(resolve => setTimeout(resolve, 25));
  return captured;
}
try {
  process.chdir(workspace);
  const request = buildRequestLike();
  await new Promise(resolve => setTimeout(resolve, 0));
  observed = process.cwd();
  const captured = await request;
  console.log(JSON.stringify({ captured, observed, globalCwdIsWorkspace: observed === workspace }));
} finally {
  process.chdir(original);
}
JS

Repository: luvs01/opencodex

Length of output: 220

Keep this cwd-changing test serial.

process.chdir(workspace) changes the process-wide cwd before await builtRequest(parsed()). Under bun test --concurrent, another test can observe workspace while buildRequest awaits Git metadata collection. Mark this test with test.serial, or pass an explicit cwd to the adapter.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/command-code-provider.test.ts` around lines 217 - 218, Keep the test
containing process.chdir(workspace) serial by changing its declaration to
test.serial, ensuring no concurrent test runs while builtRequest(parsed())
awaits. Alternatively, pass an explicit cwd through the adapter and remove
reliance on the process-wide working directory.

expect(JSON.parse(built.body).config.isGitRepo).toBe(true);
await expect(readFile(marker, "utf8")).rejects.toThrow();
Comment on lines +214 to +220

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- test structure ---'
sed -n '160,240p' tests/command-code-provider.test.ts

printf '%s\n' '--- related symbols and calls ---'
rg -n -C 3 'fsmonitor|fsmonitor-ran|core\.fsmonitor|builtRequest|function parsed|const parsed' tests src

printf '%s\n' '--- candidate test files ---'
git ls-files 'tests/**' | sed -n '1,160p'

Repository: luvs01/opencodex

Length of output: 50373

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- imports and helpers ---'
sed -n '1,95p' tests/command-code-provider.test.ts
rg -n -C 8 'builtRequest|function parsed|const parsed|core\.fsmonitor|fsmonitor' tests/command-code-provider.test.ts src

printf '%s\n' '--- repository metadata ---'
git status --short
git diff --stat
git diff -- tests/command-code-provider.test.ts | sed -n '1,220p'

Repository: luvs01/opencodex

Length of output: 50373

🏁 Script executed:

#!/bin/bash
set -eu

python3 - <<'PY'
import os
import pathlib
import shutil
import subprocess
import tempfile

with tempfile.TemporaryDirectory(prefix="ocx-fsmonitor-check-") as root:
    workspace = pathlib.Path(root)
    marker = workspace / "fsmonitor-ran"
    monitor = workspace / "fsmonitor.sh"
    monitor.write_text(f"#!/bin/sh\nprintf invoked > {str(marker)!r}\n")
    monitor.chmod(0o755)

    def run(*args):
        return subprocess.run(
            args,
            cwd=workspace,
            text=True,
            stdout=subprocess.PIPE,
            stderr=subprocess.PIPE,
            check=False,
        )

    init = run("git", "init", "-q")
    config = run("git", "config", "core.fsmonitor", str(monitor))
    status = run("git", "status", "--porcelain")
    first_marker = marker.exists()
    first_contents = marker.read_text() if first_marker else None

    if marker.exists():
        marker.unlink()
    disabled = run("git", "-c", "core.fsmonitor=false", "status", "--porcelain")
    disabled_marker = marker.exists()

    print("git_version:", subprocess.check_output(["git", "--version"], text=True).strip())
    print("init_rc:", init.returncode, "config_rc:", config.returncode)
    print("status_rc:", status.returncode, "status_stderr:", status.stderr.strip())
    print("status_hook_marker:", first_marker, "contents:", first_contents)
    print("disabled_status_rc:", disabled.returncode, "disabled_hook_marker:", disabled_marker)
PY

Repository: luvs01/opencodex

Length of output: 365

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- fsmonitor override implementation ---'
rg -n -C 10 --glob '*.ts' --glob '*.tsx' 'core\.fsmonitor=false|core\.fsmonitor|git status|status --porcelain|gitStatus|collect.*metadata|isGitRepo' src

Repository: luvs01/opencodex

Length of output: 5392

Add a control run that proves the hook is reachable.

In tests/command-code-provider.test.ts:214-220, run git status --porcelain before builtRequest, assert that fsmonitor-ran exists, remove it, then keep the existing negative assertion. Without this control, the test can pass even when Git never invokes the configured fsmonitor.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/command-code-provider.test.ts` around lines 214 - 220, Update the test
around builtRequest(parsed()) to first run git status --porcelain, assert that
the fsmonitor marker file exists, and remove the marker before continuing.
Preserve the existing builtRequest assertion and final rejection check so the
test verifies both hook reachability and that the subsequent request does not
invoke it.

} finally {
process.chdir(previousCwd);
await rm(workspace, { recursive: true, force: true });
}
});

test("does not advertise an unverified effort for models absent from the official table", async () => {
const built = await builtRequest(parsed("moonshotai/Kimi-K3"));
expect(JSON.parse(built.body).params).not.toHaveProperty("reasoning_effort");
Expand Down
Loading