HarperFast / HarperFast/harper

Branched databases: `.staging` sibling paths can collide with a legal database name

Open
#2,514 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

Branch materialization derives its scratch paths by dot-suffixing user-chosen names:

- `resources/branchDatabase.ts` — the staging checkpoint is `${branchPath}.staging` and each staging blob root is `${root}.staging`; `removeBlobRoots` also removes `${root}.staging` for every root it is handed.

`schemaRegex` permits `.` in a database name, so `data` and `data.staging` are both legal. An application that branches both has a live branch directory at `/data.staging` — which is exactly the path materialization of the `data` branch treats as its own scratch space: it removes it before staging and renames over it after. The same holds for the blob roots.

This is the sibling of the collision harper#644's removal protocol fixed for its tombstone, where the `.removing` suffix was replaced by `` `removing` `` (a backtick, which `schemaRegex` excludes, so no database name can spell it). The unit test `opens a branch of x without destroying the branch of x.removing` names this class but proves only the removal half.

## Fix direction

Give the staging paths the same protection: a suffix or a control directory no database name can spell (for example `` `staging` `` or `` `staging`/ ``), applied consistently to the checkpoint and the blob roots, plus a test that opens a branch of `x` while a branch of `x.staging` exists and shows the latter untouched. The existing `assertBranchIdentityAvailable`/`isBranchIdentity` handling of the `.staging` identity pair needs to follow the new spelling.

Found by the cross-model pre-push review of the harper#644 removal PR (pre-existing; filed separately as directed).

🤖 Filed by Claude on behalf of Kris

Contributor guide

Open the contributing guide

Research direction

Start in resources/branchDatabase.ts, tracing the staging checkpoint, blob roots, removeBlobRoots, and the assertBranchIdentityAvailable/isBranchIdentity handling. Review the existing “opens a branch of x without destroying the branch of x.removing” test, then add coverage for x and x.staging. Done means checkpoint and blob staging paths cannot collide with legal database names and the x.staging branch remains untouched.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.