Conversation
Pure-filesystem detection for git linked worktrees via .git file and commondir heuristics. Includes git execution context resolution that honors -C, --git-dir, --work-tree flags and GIT_DIR/GIT_WORK_TREE/ GIT_COMMON_DIR env overrides.
Adds worktreeMode boolean to AnalyzeOptions and worktree-relaxation discriminant to TraceStep union. Wires the option through the OpenCode plugin entry point.
Refactors analyzeGit to classify rules as local-discard vs shared-state. When SAFETY_NET_WORKTREE=1 and cwd is a linked worktree, local discard commands (checkout --, restore, reset --hard/--merge, clean -f, switch --discard-changes/-f) are allowed. Shared-state commands (push --force, branch -D, stash drop/clear, worktree remove --force) remain blocked. Wires worktreeMode, cwd, and envAssignments through the analysis pipeline: segment, xargs, and parallel.
Wires worktreeMode through all hook entry points (claude-code, copilot-cli, gemini-cli), help text, statusline emoji, doctor environment diagnostics, and explain config. Adds worktree-relaxation step formatting in explain output (both human and Style D).
Adds createLinkedWorktreeFixture and createSubmoduleLikeGitFileFixture test helpers. Tests worktree detection (linked, main, submodule-like, symlinked, malformed), git execution context resolution, env override detection, and worktree relaxation rules (local discard allowed, shared-state blocked, -C resolution, explicit context overrides).
Tests statusline emoji, doctor environment variable listing, help text, explain --json worktree-relaxation step, explain human formatting, and TraceStep type discriminant for worktree-relaxation.
Documents SAFETY_NET_WORKTREE environment variable, which commands are relaxed (local discards) vs remain blocked (shared-state), linked worktree detection heuristics, and statusline emoji.
… AnalyzeOptions Add AnalyzeNestedOverrides interface for propagating env, cwd, and worktreeMode overrides through nested command analysis. Add envAssignments field to AnalyzeOptions so environment context can flow through the analysis pipeline.
…pping Extend stripWrappersWithInfo and stripEnvAssignmentsWithInfo to track the effective cwd when env -C or --chdir is encountered. This allows nested analysis to use the correct working directory for rule checks.
…ested Update analyzeNested in analyzeCommandInternal to accept AnalyzeNestedOverrides, passing effectiveCwd, envAssignments, and worktreeMode to nested analysis so context is preserved across recursion boundaries.
Use stripWrappersWithInfo cwd result to set nestedEffectiveCwd and cwdForRm. Merge env assignments from wrappers into the inherited envAssignments map. Pass overrides when analyzing shell wrapper children and disable worktreeMode for SSH/remote embedded git commands.
…pagation Detect SSH login flags (-S/--sshlogin) to identify remote parallel execution and disable worktreeMode for those commands. Use stripWrappersWithInfo instead of stripWrappers to propagate env/cwd context from child wrappers. Build and pass AnalyzeNestedOverrides through all nested analysis calls.
Use stripWrappersWithInfo instead of stripWrappers to capture env assignments and cwd from child wrappers. Pass the resolved childCwd and merged childEnvAssignments to git analysis.
Change analyzeGitReset to return GitRuleMatch so it can distinguish between ref-moving resets (shared state) and local-only discards. Resets with a ref argument stay blocked even in worktree mode, while bare --hard/--merge without a ref are treated as local discards.
Verify that SAFETY_NET_WORKTREE does not relax protections when: - cwd-changing wrappers redirect to main worktree - xargs/parallel/shell wrapper child commands use env overrides targeting the main worktree - fallback embedded git commands run over SSH - remote parallel execution uses SSH login flags - git reset with ref arguments moves shared refs
…pping Upgrade stripSudo to stripSudoWithInfo that tracks -D directory changes. Add env -S/--split-string parsing that expands split-string tokens before further processing, handling -S val, -Sval, and --split-string=val forms. Fix env -C attached-arg parsing to handle -Cval (without =). Export parseEnvAssignment and GIT_CONTEXT_ENV_OVERRIDES for use in segment-level export tracking.
…ents Track export statements that set GIT_DIR/GIT_WORK_TREE/GIT_COMMON_DIR across semicolon-separated segments, propagating them into subsequent segment analysis. Also propagate segment-scoped envAssignments through analyzeNested so nested commands see the correct env context.
…ox, parallel, and xargs Pass effectiveCwd and envAssignments through interpreter one-liners and BusyBox recursive analysis. In parallel commands mode, forward wrapper env/cwd context via buildCommandsModeOverrides. In xargs git analysis, append a placeholder token when xargs input would be appended as a git argument, enabling ref-moving reset detection. In parallel template mode, expand git command sets with each arg to detect ref-moving resets through appended input.
…e fallback worktree relaxation Move effectiveCwd derivation before wrapper stripping so stripWrappersWithInfo receives the correct base cwd. Use wrapperResult.cwd for downstream cwd derivation to match guard behavior. Disable worktreeMode for fallback embedded git analysis in explain, preventing spurious worktree relaxation for commands like ssh host git clean -f.
… BusyBox, parallel, xargs, and interpreter propagation Add comprehensive test coverage for worktree mode: env -C with terminators and attached args, env -S split-string parsing, sudo -D chdir, export of GIT_DIR/GIT_WORK_TREE across segments, BusyBox wrapper context, parallel commands mode and template arg expansion, xargs appended input for git ref-moving resets, and interpreter wrapper context propagation. Also add explainCommand parity tests for wrapper cwd and fallback worktree relaxation.
… and double-force clean
…ards, sudo chdir, export/typeset, and explain parity
…fy gitdir backlink
…gin in worktree mode
…th fallback Replace direct realpathSync equality checks in worktreeGitdirBacklinkMatches and worktreeConfigMatchesRoot with sameFilesystemPath, which compares by device/inode first for reliability, then falls back to normalized realpath comparison with platform-aware case handling and separator normalization.
…in xargs/parallel tests Wrap otherDir paths with toShellPath to ensure correct path formatting across platforms, particularly Windows where backslash separators differ.
…before invoking git binary Parse .git/config and config.worktree files directly to determine submodule.recurse setting without needing a git binary invocation. Resolves gitdir and commondir from worktree .git files. Includes and includeIf directives conservatively treat as enabling recursive submodules. Falls back to git binary only when local config is absent or inconclusive; returns false instead of true when git binary is unavailable but local config is readable.
…ed result The linked worktree fixture now has its local git config read directly, so git reset --hard is allowed when the local config is readable and the git binary is not invoked (or unavailable).
📝 WalkthroughWalkthroughAdds SAFETY_NET_WORKTREE-controlled "Worktree Mode": per-segment shell/git context and env-assignment tracking, new worktree detection utilities, and optional "worktree relaxation" that permits certain local-discard git operations only when executed inside a verified linked worktree. Changes
Sequence Diagram(s)sequenceDiagram
%%{init: {"themeVariables": {"actorBackground":"rgba(200,220,255,0.5)","actorBorder":"rgba(0,0,0,0.5)"}} }%%
participant User as "User (shell)"
participant Parser as "Shell Parser / stripWrappers"
participant Analyzer as "Analyzer (analyzeCommand)"
participant Worktree as "Worktree Utils (isLinkedWorktree)"
participant FS as "Filesystem / git config"
User->>Parser: submit command tokens + cwd + env
Parser->>Analyzer: tokens, effectiveCwd, envAssignments (per‑segment)
Analyzer->>Analyzer: update shell‑git context across segments
Analyzer->>Worktree: getGitExecutionContext(tokens, cwd)
Worktree->>FS: resolve -C targets, read .git, read linked config
alt linked worktree verified and SAFETY_NET_WORKTREE=1
Worktree-->>Analyzer: { gitCwd }
Analyzer->>Analyzer: getGitWorktreeRelaxation(tokens, envAssignments, worktreeMode)
Analyzer-->>User: emit worktree‑relaxation trace -> allow local discard
else explicit overrides / unknown / not linked
Worktree-->>Analyzer: { hasExplicitGitContext:true } or { gitCwd:null }
Analyzer-->>User: block destructive git operation (no relaxation)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
- Coverage 99.51% 98.51% -1.00%
==========================================
Files 50 52 +2
Lines 5362 6879 +1517
==========================================
+ Hits 5336 6777 +1441
- Misses 26 102 +76 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds Worktree Mode ( The implementation is thorough and well-tested. The one remaining maintainability note is that Confidence Score: 5/5Safe to merge — all hardening paths are fail-closed and the only finding is a minor maintainability duplication with no correctness impact. No P0 or P1 issues found. Previously flagged P1s (missing /usr/local/bin/git and /opt/homebrew/bin/git in TRUSTED_GIT_BINARIES, GIT_CONFIG_AFFECTING_ENV_NAMES triplication, path helper duplication, missing backslash escape handling) are all addressed. The single remaining finding is a P2 duplication of three small helpers across shell.ts and analyze-command.ts. src/core/shell.ts and src/core/analyze/analyze-command.ts share identical isTrackedGitEnvName/isGitConfigEnvName/GIT_CONTEXT_ENV_OVERRIDE_NAMES definitions that should be consolidated. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[analyzeGit called] --> B{worktreeMode enabled?}
B -- No --> C[Return original reason / block]
B -- Yes --> D{match.localDiscard?}
D -- No sharedState --> C
D -- Yes --> E{hasGitContextEnvOverride?}
E -- Yes --> C
E -- No --> F[getGitExecutionContext]
F --> G{gitCwd resolved?}
G -- null or explicit --> C
G -- Valid cwd --> H[isLinkedWorktree]
H -- Not linked --> C
H -- Is linked --> I[isNonRelaxableLocalDiscard]
I --> J{Dynamic args / recurse-submodules / double-force / branch-reset?}
J -- Yes --> C
J -- No --> K[hasRecursiveSubmoduleConfig]
K -- Enabled --> C
K -- Not enabled --> L[Allow relaxation]
Reviews (2): Last reviewed commit: "fix(worktree): handle double-quoted esca..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/helpers.ts (1)
100-157: ⚡ Quick winMark the new fixture exports as internal.
These interfaces/functions are test-only exports, so leaving them undecorated makes Knip treat them as public surface.
♻️ Suggested JSDoc additions
+/** `@internal` Exported for testing */ export interface LinkedWorktreeFixture { rootDir: string; mainWorktree: string; linkedWorktree: string; cleanup: () => void; } +/** `@internal` Exported for testing */ export function createLinkedWorktreeFixture(): LinkedWorktreeFixture { // ... } +/** `@internal` Exported for testing */ export interface FakeGitFileFixture { rootDir: string; cwd: string; cleanup: () => void; } +/** `@internal` Exported for testing */ export function createSubmoduleLikeGitFileFixture(): FakeGitFileFixture { // ... }Based on learnings: "For test-only exports, add
/**@internalExported for testing */JSDoc to satisfy knip".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/helpers.ts` around lines 100 - 157, Add internal JSDoc to mark these test-only exports as internal: annotate the LinkedWorktreeFixture and FakeGitFileFixture interfaces and the createLinkedWorktreeFixture and createSubmoduleLikeGitFileFixture functions with a comment like /** `@internal` Exported for testing */ immediately above each declaration so knip treats them as non-public; update the JSDoc for the symbols LinkedWorktreeFixture, createLinkedWorktreeFixture, FakeGitFileFixture, and createSubmoduleLikeGitFileFixture accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/helpers.ts`:
- Around line 100-157: Add internal JSDoc to mark these test-only exports as
internal: annotate the LinkedWorktreeFixture and FakeGitFileFixture interfaces
and the createLinkedWorktreeFixture and createSubmoduleLikeGitFileFixture
functions with a comment like /** `@internal` Exported for testing */ immediately
above each declaration so knip treats them as non-public; update the JSDoc for
the symbols LinkedWorktreeFixture, createLinkedWorktreeFixture,
FakeGitFileFixture, and createSubmoduleLikeGitFileFixture accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46393664-7699-48ac-aa14-49817c404794
⛔ Files ignored due to path filters (10)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**dist/core/analyze/analyze-command.d.tsis excluded by!**/dist/**dist/core/analyze/parallel.d.tsis excluded by!**/dist/**dist/core/analyze/segment.d.tsis excluded by!**/dist/**dist/core/analyze/xargs.d.tsis excluded by!**/dist/**dist/core/rules-git.d.tsis excluded by!**/dist/**dist/core/shell.d.tsis excluded by!**/dist/**dist/core/worktree.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/types.d.tsis excluded by!**/dist/**
📒 Files selected for processing (32)
README.mdsrc/bin/doctor/environment.tssrc/bin/explain/analyze.tssrc/bin/explain/config.tssrc/bin/explain/format-helpers.tssrc/bin/explain/segment.tssrc/bin/help.tssrc/bin/hooks/claude-code.tssrc/bin/hooks/copilot-cli.tssrc/bin/hooks/gemini-cli.tssrc/bin/statusline.tssrc/core/analyze/analyze-command.tssrc/core/analyze/parallel.tssrc/core/analyze/segment.tssrc/core/analyze/xargs.tssrc/core/rules-git.tssrc/core/shell.tssrc/core/worktree.tssrc/index.tssrc/types.tstests/bin/cli-statusline.test.tstests/bin/doctor/environment.test.tstests/bin/explain/cli.test.tstests/bin/explain/command.test.tstests/bin/explain/format.test.tstests/bin/explain/types.test.tstests/bin/help.test.tstests/core/analyze/edge-cases.test.tstests/core/analyze/parsing-helpers.test.tstests/core/rules-git.test.tstests/core/worktree.test.tstests/helpers.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/rules-git.ts`:
- Around line 609-618: The function effectiveGitConfigEnablesRecursiveSubmodules
currently treats a missing trusted git binary as safe; change its behavior in
effectiveGitConfigEnablesRecursiveSubmodules so that if getTrustedGitBinary()
returns null it returns true (fail-closed/assume unsafe) instead of false, and
make the same fail-closed change in any other branch that currently returns
false when gitBinary is null (see similar logic around where
getTrustedGitBinary() is called later in the file). Also update
getTrustedGitBinary to include standard Windows installation paths ("C:\\Program
Files\\Git\\cmd\\git.exe" and "C:\\Program Files\\Git\\bin\\git.exe") in its
candidate list so Windows-installed git is recognized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f85784a-7a01-4c56-88fa-6223caae7780
⛔ Files ignored due to path filters (5)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**dist/core/path.d.tsis excluded by!**/dist/**dist/core/rules-git.d.tsis excluded by!**/dist/**dist/core/worktree.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (7)
src/core/analyze/analyze-command.tssrc/core/path.tssrc/core/rules-git.tssrc/core/shell.tssrc/core/worktree.tstests/core/rules-git.test.tstests/core/worktree.test.ts
Owner
Author
|
@greptileai review |
…e config enabled When no trusted git binary is found, assume recursive submodule config is enabled rather than disabled, so commands are allowed by default instead of incorrectly blocked. Export effectiveGitConfigEnablesRecursiveSubmodules for testing.
…ive submodule/include config-env tests
…port, command, typeset, readonly, and set
…d config fail-closed tests
…d placeholder arguments
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/core/analyze/edge-cases.test.ts`:
- Around line 429-440: The test 'xargs rm uses wrapper cwd when checking
relative targets' currently uses `rm -rf build` which resides inside `otherDir`
and doesn't prove wrapper-cwd enforcement; update the command passed to
`assertBlocked` to target a path outside `otherDir` (e.g. `../build`) so the
removal would escape the wrapper cwd. Adjust the call to assertBlocked to use
the modified shell string `echo ok | xargs env -C ${toShellPath(otherDir)} rm
-rf ../build` (keeping `projectDir`, `otherDir`, `toShellPath`, and
`assertBlocked` as reference points) so the test correctly verifies escape
detection.
- Around line 548-559: The test 'parallel rm uses wrapper cwd when checking
relative replacements' is using the relative token "build" which lives under the
wrapper cwd so it does not prove the cwd-escape check; update the assertBlocked
command to target a path that actually escapes the wrapper cwd (for example use
toShellPath(join(otherDir, 'build')) or a '../other/build' relative path) and
create that target if needed (mkdirSync(join(otherDir, 'build'))). Modify the
string passed to assertBlocked (the command in the parallel env invocation) to
reference the escaping path and keep the rest of the test (projectDir/otherDir,
assertBlocked, toShellPath, tempDir) intact.
In `@tests/core/worktree.test.ts`:
- Around line 292-301: The test currently treats an unparsable config.worktree
as a valid linked worktree; update the test so malformed core.worktree is
rejected: in the test using createLinkedWorktreeFixture(), getLinkedGitDir(),
and writeFileSync(... 'config.worktree', '[core\n\tworktree ='), change the
assertion on isLinkedWorktree(fixture.linkedWorktree) from toBe(true) to
toBe(false) (and optionally rename the test to reflect rejection) while keeping
the try/finally cleanup intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efdca466-a3b8-45d7-9210-7caef1ab0eda
📒 Files selected for processing (6)
.gitignoretests/core/analyze/analyze-coverage.test.tstests/core/analyze/edge-cases.test.tstests/core/analyze/parsing-helpers.test.tstests/core/rules-git.test.tstests/core/worktree.test.ts
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- tests/core/rules-git.test.ts
Comment on lines
+429
to
+440
| test('xargs rm uses wrapper cwd when checking relative targets', () => { | ||
| const projectDir = join(tempDir, 'project'); | ||
| const otherDir = join(tempDir, 'other'); | ||
| mkdirSync(projectDir); | ||
| mkdirSync(otherDir); | ||
|
|
||
| assertBlocked( | ||
| `echo ok | xargs env -C ${toShellPath(otherDir)} rm -rf build`, | ||
| 'rm -rf outside cwd', | ||
| projectDir, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Use an escaping path if the goal is to prove wrapper-cwd enforcement.
build is still inside otherDir, so this blocked assertion does not actually demonstrate that env -C is being honored. If you want to verify escape detection, switch this to something like ../build.
Suggested adjustment
- `echo ok | xargs env -C ${toShellPath(otherDir)} rm -rf build`,
+ `echo ok | xargs env -C ${toShellPath(otherDir)} rm -rf ../build`,📝 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.
Suggested change
| test('xargs rm uses wrapper cwd when checking relative targets', () => { | |
| const projectDir = join(tempDir, 'project'); | |
| const otherDir = join(tempDir, 'other'); | |
| mkdirSync(projectDir); | |
| mkdirSync(otherDir); | |
| assertBlocked( | |
| `echo ok | xargs env -C ${toShellPath(otherDir)} rm -rf build`, | |
| 'rm -rf outside cwd', | |
| projectDir, | |
| ); | |
| }); | |
| test('xargs rm uses wrapper cwd when checking relative targets', () => { | |
| const projectDir = join(tempDir, 'project'); | |
| const otherDir = join(tempDir, 'other'); | |
| mkdirSync(projectDir); | |
| mkdirSync(otherDir); | |
| assertBlocked( | |
| `echo ok | xargs env -C ${toShellPath(otherDir)} rm -rf ../build`, | |
| 'rm -rf outside cwd', | |
| projectDir, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/core/analyze/edge-cases.test.ts` around lines 429 - 440, The test
'xargs rm uses wrapper cwd when checking relative targets' currently uses `rm
-rf build` which resides inside `otherDir` and doesn't prove wrapper-cwd
enforcement; update the command passed to `assertBlocked` to target a path
outside `otherDir` (e.g. `../build`) so the removal would escape the wrapper
cwd. Adjust the call to assertBlocked to use the modified shell string `echo ok
| xargs env -C ${toShellPath(otherDir)} rm -rf ../build` (keeping `projectDir`,
`otherDir`, `toShellPath`, and `assertBlocked` as reference points) so the test
correctly verifies escape detection.
Comment on lines
+548
to
+559
| test('parallel rm uses wrapper cwd when checking relative replacements', () => { | ||
| const projectDir = join(tempDir, 'project'); | ||
| const otherDir = join(tempDir, 'other'); | ||
| mkdirSync(projectDir); | ||
| mkdirSync(otherDir); | ||
|
|
||
| assertBlocked( | ||
| `parallel env -C ${toShellPath(otherDir)} rm -rf {} ::: build`, | ||
| 'rm -rf outside cwd', | ||
| projectDir, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Same cwd-escape issue in the parallel case.
parallel ... env -C ${otherDir} ... build keeps build under the wrapper cwd, so blocking it doesn’t prove the wrapper-cwd check. Consider using a path that actually escapes the wrapper cwd.
Suggested adjustment
- `parallel env -C ${toShellPath(otherDir)} rm -rf {} ::: build`,
+ `parallel env -C ${toShellPath(otherDir)} rm -rf {} ::: ../build`,📝 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.
Suggested change
| test('parallel rm uses wrapper cwd when checking relative replacements', () => { | |
| const projectDir = join(tempDir, 'project'); | |
| const otherDir = join(tempDir, 'other'); | |
| mkdirSync(projectDir); | |
| mkdirSync(otherDir); | |
| assertBlocked( | |
| `parallel env -C ${toShellPath(otherDir)} rm -rf {} ::: build`, | |
| 'rm -rf outside cwd', | |
| projectDir, | |
| ); | |
| }); | |
| test('parallel rm uses wrapper cwd when checking relative replacements', () => { | |
| const projectDir = join(tempDir, 'project'); | |
| const otherDir = join(tempDir, 'other'); | |
| mkdirSync(projectDir); | |
| mkdirSync(otherDir); | |
| assertBlocked( | |
| `parallel env -C ${toShellPath(otherDir)} rm -rf {} ::: ../build`, | |
| 'rm -rf outside cwd', | |
| projectDir, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/core/analyze/edge-cases.test.ts` around lines 548 - 559, The test
'parallel rm uses wrapper cwd when checking relative replacements' is using the
relative token "build" which lives under the wrapper cwd so it does not prove
the cwd-escape check; update the assertBlocked command to target a path that
actually escapes the wrapper cwd (for example use toShellPath(join(otherDir,
'build')) or a '../other/build' relative path) and create that target if needed
(mkdirSync(join(otherDir, 'build'))). Modify the string passed to assertBlocked
(the command in the parallel env invocation) to reference the escaping path and
keep the rest of the test (projectDir/otherDir, assertBlocked, toShellPath,
tempDir) intact.
Comment on lines
+292
to
+301
| test('accepts unparsable core.worktree config as conservative fallback', () => { | ||
| const fixture = createLinkedWorktreeFixture(); | ||
| const gitDir = getLinkedGitDir(fixture.linkedWorktree); | ||
| writeFileSync(join(gitDir, 'config.worktree'), '[core\n\tworktree ='); | ||
| try { | ||
| expect(isLinkedWorktree(fixture.linkedWorktree)).toBe(true); | ||
| } finally { | ||
| fixture.cleanup(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fail closed on malformed core.worktree.
Accepting an unparsable config.worktree as a linked worktree keeps the relaxation path open even when the repo metadata is corrupted or tampered with. Worktree Mode should only apply to verified linked worktrees, so this should reject instead of falling back to true.
Suggested expectation
- expect(isLinkedWorktree(fixture.linkedWorktree)).toBe(true);
+ expect(isLinkedWorktree(fixture.linkedWorktree)).toBe(false);📝 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.
Suggested change
| test('accepts unparsable core.worktree config as conservative fallback', () => { | |
| const fixture = createLinkedWorktreeFixture(); | |
| const gitDir = getLinkedGitDir(fixture.linkedWorktree); | |
| writeFileSync(join(gitDir, 'config.worktree'), '[core\n\tworktree ='); | |
| try { | |
| expect(isLinkedWorktree(fixture.linkedWorktree)).toBe(true); | |
| } finally { | |
| fixture.cleanup(); | |
| } | |
| }); | |
| test('accepts unparsable core.worktree config as conservative fallback', () => { | |
| const fixture = createLinkedWorktreeFixture(); | |
| const gitDir = getLinkedGitDir(fixture.linkedWorktree); | |
| writeFileSync(join(gitDir, 'config.worktree'), '[core\n\tworktree ='); | |
| try { | |
| expect(isLinkedWorktree(fixture.linkedWorktree)).toBe(false); | |
| } finally { | |
| fixture.cleanup(); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/core/worktree.test.ts` around lines 292 - 301, The test currently
treats an unparsable config.worktree as a valid linked worktree; update the test
so malformed core.worktree is rejected: in the test using
createLinkedWorktreeFixture(), getLinkedGitDir(), and writeFileSync(...
'config.worktree', '[core\n\tworktree ='), change the assertion on
isLinkedWorktree(fixture.linkedWorktree) from toBe(true) to toBe(false) (and
optionally rename the test to reflect rejection) while keeping the try/finally
cleanup intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SAFETY_NET_WORKTREE=1): relaxes local-discard git rules (restore, checkout --, reset --hard, clean -f, switch --force) when the cwd is a verified linked worktree, since linked worktrees are designed as disposable, isolated workspaces.xargs/parallel/ BusyBox, fail-closed on dynamic arguments, recursive submodules, forced branch resets,--git-dir/--work-treeoverrides, and git config-affecting env vars.src/core/worktree.tsmodule for filesystem-based linked worktree detection with gitdir backlink verification,config.worktreevalidation, inode-based path comparison, and symlink-saferealpathresolution.Changes
New Module:
src/core/worktree.ts(+335 lines).gitfile →gitdir:→commondirchain.gitfiles)config.worktreevalidation against actual worktree rootgit -Cchained resolution with symlink-saferealpathrealpathfallback\\?\,\\?\UNC\)Core Analysis Enhancements
src/core/rules-git.ts(+608 lines): Worktree relaxation logic with fail-closed hardening:getGitWorktreeRelaxation()— determines if a blocked git command should be relaxedisNonRelaxableLocalDiscard()— checks for dynamic arguments, recursive submodules, double-force clean, forced branch resetshasRecursiveSubmoduleConfig()— reads local git config files before invoking git binary; checksGIT_CONFIG_COUNT/GIT_CONFIG_KEY_*/GIT_CONFIG_VALUE_*env vars and--config-enveffectiveGitConfigEnablesRecursiveSubmodules()— invokes trusted git binary (/usr/bin/git,/usr/local/bin/git) with sanitized envsrc/core/analyze/analyze-command.ts(+445 lines): Shell git context environment tracking:ShellGitContextEnvState— tracks exported names,set -a(allexport), keyword export modeapplyShellGitContextEnvSegment()— handlesexport,typeset,declare,readonly,builtin,commandbuiltinsgetSegmentGitContextEnvAssignments()— computes per-segment env assignments from accumulated shell stateNAME+=value) tracked for git config env namesAnalyzeNestedOverridespropagated through nested analysis callssrc/core/shell.ts(+261 lines): Wrapper/env stripping with cwd tracking:stripWrappersWithInfo()now returnscwd(null = unknown, undefined = unchanged)sudo --chdir/-D/--logincwd trackingenv -C/--chdir/-S/--split-stringhandlingNAME+=value) preserved for git contextsrc/core/analyze/parallel.ts(+150 lines): Worktree-aware parallel analysis:-S,--sshlogin) and dynamic stdin/placeholder combinations{1},{.},{/},{//},{/.},{#},{%}envAssignmentsandcwdfrom wrapper stripping to child analysissrc/core/analyze/xargs.ts(+32 lines): Worktree-aware xargs analysis:analyzeRmcallsenvAssignmentsthrough child analysissrc/core/analyze/segment.ts(+53 lines): PropagatesenvAssignmentsand wrapper cwd into nested analysis callssrc/types.ts(+11 lines):worktreeModeoption inAnalyzeOptionsAnalyzeNestedOverridestype for env/cwd/worktreeMode propagationworktree-relaxationtrace step typeCLI Surface & Explain
claude-code.ts,copilot-cli.ts,gemini-cli.ts): ReadSAFETY_NET_WORKTREEand passworktreeModeto analysissrc/bin/statusline.ts: 🌳 emoji for worktree modesrc/bin/help.ts: DocumentsSAFETY_NET_WORKTREE=1env varsrc/bin/explain/segment.ts(+117 lines): Worktree relaxation trace steps, env/cwd propagation parity with guard analysissrc/bin/explain/format-helpers.ts: Formatworktree-relaxationtrace stepsrc/bin/explain/config.ts: IncludeworktreeModein explain configsrc/bin/doctor/environment.ts: ReportSAFETY_NET_WORKTREEin doctor outputDocumentation
Tests (~1,600 lines added)
tests/core/worktree.test.ts(+276 lines): Unit tests for linked worktree detection — linked/main/non-repo/submodule/symlinked/malformed/copied/backlink/config.worktree/Windows path normalizationtests/core/rules-git.test.ts(+971 lines): Comprehensive worktree mode tests — bypass/hardening, env propagation (HOME, GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_CONFIG_*), sudo --chdir, export/typeset/declare/readonly, set -a, keyword export, recursive submodules, forced branch resets, dynamic arguments, xargs/parallel propagation, interpreter wrapping, shell wrapper nesting, explain paritytests/bin/explain/command.test.ts(+156 lines): Explain command worktree trace stepstests/core/analyze/edge-cases.test.ts(+30 lines): Platform-aware path formatting in xargs/parallel teststests/helpers.ts(+70 lines):createLinkedWorktreeFixture(),createSubmoduleLikeGitFileFixture(),toShellPath(),withEnv(), worktreeMode env supportTesting
Related Issue
#44 #47
PR Checklist
bun run checkpasses (lint, types, dead code, rules, tests)package.jsonSummary by CodeRabbit
New Features
Documentation
Tests