refactor(utils): dedup backup helpers and document lock/exit conventions by funkadelic · Pull Request #308 · funkadelic/claude-nomad

funkadelic · 2026-06-17T00:21:06Z

Collapses backupBeforeWrite, backupRepoWrite, and backupExtrasWrite (identical bodies differing only by anchor and backup-root prefix) into a single backupUnder core, keeping the three as thin scoped wrappers.
Documents two existing conventions in-place (comment-only): lockFilePath is HOME-keyed because pull/push mutate HOME-scoped state, with the NOMAD_REPO-override concurrency caveat; and resumeCmd's raw process.exit calls are intentional because it is lockless and read-only.
No behavior change.

Summary by CodeRabbit

Refactor
- Consolidated backup snapshot logic into a shared helper to keep backup behavior consistent across scenarios.
Bug Fixes
- Ensured backup handling correctly processes entries where a final path segment begins with "..", preventing mistaken “unsafe path” treatment.
Documentation
- Clarified command error-handling behavior and exit behavior.
- Expanded JSDoc for lockfile path resolution, including HOME-based behavior and concurrency considerations.
Tests
- Added a Vitest case covering the ".."-prefixed path segment backup scenario.

backupBeforeWrite, backupRepoWrite, and backupExtrasWrite shared an identical body differing only by anchor and backup-root prefix. Extract backupUnder and keep the three as thin scoped wrappers, so the force:false collision and relative-bail behavior live in one place.

coderabbitai · 2026-06-17T00:21:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1d12a20f-c4ab-4b75-92c0-4c5b6068df15

📥 Commits

Reviewing files that changed from the base of the PR and between 325c881 and 1e6f169.

📒 Files selected for processing (2)

src/utils.fs.backup.test.ts
src/utils.fs.ts

✅ Files skipped from review due to trivial changes (1)

src/utils.fs.backup.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/utils.fs.ts

📝 Walkthrough

Walkthrough

src/utils.fs.ts consolidates backup snapshot logic by introducing an internal backupUnder(absPath, anchor, destRoot) helper that centralizes existence check, anchored relative-path guard, directory creation, and recursive cpSync with timestamp preservation. Three backup writers delegate to this helper. A regression test validates the ..-prefix edge case. Documentation in src/resume.ts and src/utils.lockfile.ts expands to clarify error handling and lock behavior.

Changes

Backup helper consolidation

Layer / File(s)	Summary
`backupUnder` helper and delegating refactors `src/utils.fs.ts`	Updates Node path imports to include `sep`. Adds `backupUnder(absPath, anchor, destRoot)` centralizing existence check, anchored `..`-prefix guard, parent directory creation, and recursive `cpSync` with timestamp preservation. Refactors `backupBeforeWrite` and `backupRepoWrite` to delegate with `claudeHome()`/`repoHome` anchors and respective destination roots. Refactors `backupExtrasWrite` to delegate with `encodePath(projectRoot)`-suffixed destination root.
Backup logic edge-case test `src/utils.fs.backup.test.ts`	Adds regression test verifying that path segments beginning with `".."` (e.g., `"..config"`) are backed up correctly and are not mistakenly skipped as escape paths.

Documentation clarifications

Layer / File(s)	Summary
Error handling and lock path documentation `src/resume.ts`, `src/utils.lockfile.ts`	Expands `resumeCmd` module documentation to clarify that error paths use `process.exit(1)` in a lockless, read-only context and warn about future locked/mutating step handling. Expands `lockFilePath()` JSDoc to describe HOME-scoped lock resolution at call time and `NOMAD_REPO` concurrency caveats.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

funkadelic/claude-nomad#14: Introduces the original backup primitives backupBeforeWrite and backupRepoWrite that are consolidated in this PR via the new shared helper.
funkadelic/claude-nomad#211: Modifies the same backup writer functions in src/utils.fs.ts, rerouting backup roots through BACKUP_BASE.

Suggested labels

tests, documentation

Poem

🐇 Three backup writers walked alone,
Each copying paths with code of its own.
Now backupUnder brings them to rest,
One guard, one anchor, one copy—blessed!
With docs that explain why we exit and lock,
This warren of backups now clearer than rock! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: refactoring backup helpers to reduce duplication and documenting lock/exit conventions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch refactor/backup-helper-dedup

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-06-17T00:23:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (739676f) to head (1e6f169).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #308   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           90        90           
  Lines         3247      3234   -13     
  Branches       814       810    -4     
=========================================
- Hits          3247      3234   -13

Flag	Coverage Δ
unittests	`100.00% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/resume.ts	`100.00% <ø> (ø)`
src/utils.fs.ts	`100.00% <100.00%> (ø)`
src/utils.lockfile.ts	`100.00% <ø> (ø)`

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 739676f...1e6f169. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/utils.fs.ts`:
- Around line 103-105: The anchor-escape guard in the relative path validation
is insufficient for cross-drive Windows paths where relative() can return
absolute paths, allowing escape of destRoot via join(). Additionally, the check
for ".." is too broad and rejects valid directory names containing ".." like
"..config". Fix this by adding an isAbsolute() check to reject absolute relative
paths, and tighten the ".." check to only match ".." at path-segment boundaries
(e.g., starts with ".." followed by a path separator or is exactly "..") rather
than any string starting with "..". Update the condition on line 104 to include
the isAbsolute() check and make the ".." validation more precise.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6dc6b883-07a3-40be-beeb-70873a9461c2

📥 Commits

Reviewing files that changed from the base of the PR and between 739676f and aa85abd.

📒 Files selected for processing (1)

src/utils.fs.ts

Note in lockFilePath that the lock is HOME-keyed (pull/push mutate HOME-scoped state) and that a NOMAD_REPO override must not be combined with concurrent default-HOME runs. Note in resumeCmd that its raw process.exit calls are intentional because it is lockless and read-only, and that any future locked/mutating step must switch to die()/NomadFatal.

Tighten backupUnder's anchor-escape guard from a bare startsWith('..') to a segment-boundary check (rel === '..' or a '..<sep>' prefix), so a legitimate sibling whose name begins with '..' (e.g. ..config) is backed up instead of skipped. Add a regression test.

sonarqubecloud · 2026-06-17T01:17:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions Bot added the source label Jun 17, 2026

funkadelic self-assigned this Jun 17, 2026

coderabbitai Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread src/utils.fs.ts

funkadelic changed the title ~~refactor(utils): collapse three backup helpers into one core~~ refactor(utils): dedup backup helpers and document lock/exit conventions Jun 17, 2026

github-actions Bot added the tests label Jun 17, 2026

funkadelic merged commit 4c2d59d into main Jun 17, 2026
14 checks passed

funkadelic deleted the refactor/backup-helper-dedup branch June 17, 2026 01:29

funkadelic-release-bot Bot mentioned this pull request Jun 17, 2026

chore(main): release 0.50.3 #300

Merged

coderabbitai Bot mentioned this pull request Jun 29, 2026

fix(remap): match Claude Code's transcript-dir path encoder #360

Merged

Navigation Menu

Repository navigation

refactor(utils): dedup backup helpers and document lock/exit conventions - #308

refactor(utils): dedup backup helpers and document lock/exit conventions#308
funkadelic merged 3 commits into
mainfrom
refactor/backup-helper-dedup

funkadelic commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

codecov Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jun 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

funkadelic commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

codecov Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jun 17, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

funkadelic commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

codecov Bot commented Jun 17, 2026 •

edited

Loading