FE-545: Local-first storage; npx distribution#43
Conversation
FE-545 Local-first storage; npx distribution
|
🤖 Augment PR SummarySummary: Implements “local-first” project storage by introducing a discoverable Changes:
Technical Notes: The launcher resolves the project from an arbitrary cwd, uses the resolved DB path, and conditionally installs an SPA fallback when 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| "license": "(MIT OR Apache-2.0)", | ||
| "type": "module", | ||
| "bin": { | ||
| "brunch": "./src/server/cli.ts" |
There was a problem hiding this comment.
package.json:16 sets bin.brunch to ./src/server/cli.ts, which npx will execute with plain Node; Node typically can’t run .ts bin entries and cli.ts imports ./launcher.js (which won’t exist without a build). This may cause the distributed CLI to fail to start unless the published artifact includes compiled JS with matching paths.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| const { app } = createApp(DB_PATH); | ||
| // In dev mode, use BRUNCH_DB env var if set, otherwise resolve .brunch/ project | ||
| const dbPath = DB_PATH ?? resolveBrunchProject(process.cwd()).dbPath; |
There was a problem hiding this comment.
src/server/index.ts:8 uses DB_PATH ?? ..., so BRUNCH_DB="" (empty string) will not fall back to .brunch/ resolution and may be passed as an invalid SQLite path (previously || would have defaulted). This can lead to startup failures if an env var is present-but-empty.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| app.use(express.static(DIST_DIR)); | ||
|
|
||
| // SPA fallback: serve index.html for all non-API routes | ||
| app.get('*', (_req, res) => { |
There was a problem hiding this comment.
src/server/launcher.ts:24 installs a catch-all app.get('*', ...) SPA fallback, which will also serve index.html for unknown /api/* GET routes when dist/ exists (masking API 404s). That can break API error handling/debugging in production mode.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| for (let i = 0; i <= MAX_WALK_UP; i++) { | ||
| const candidate = join(current, BRUNCH_DIR); | ||
| if (existsSync(candidate)) { |
There was a problem hiding this comment.
src/server/project.ts:34 treats any existing .brunch path as a valid project; if a file (not a directory) exists at .brunch, dbPath becomes nonsensical and DB init will fail later with a less clear error. It may be worth ensuring the candidate is actually a directory before returning a BrunchProject.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| expect(Array.isArray(res.body)).toBe(true); | ||
| }); | ||
|
|
||
| it('serves static files when dist/ exists alongside the API', async () => { |
There was a problem hiding this comment.
src/server/launcher.test.ts:35 is named as if it verifies static serving, but it never creates a dist/ tree or requests a static asset, so it can’t fail if express.static/SPA fallback breaks. As written it only re-checks the API route.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| expect(res.body).toBeDefined(); | ||
| }); | ||
|
|
||
| it('resolves drizzle migrations when cwd differs from package root', () => { |
There was a problem hiding this comment.
src/server/launcher.test.ts:48 doesn’t change process.cwd() before calling createApp, so it likely would have passed even when migrations were resolved via ./drizzle relative to the test runner’s cwd. As written, it may not actually protect the “cwd differs from package root” invariant it describes.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| - Acceptance: `npx brunch` in a project directory creates `.brunch/`, opens working app; running from subdirectory finds parent `.brunch/`; `BrunchProject` struct exposes root, dbPath, cwd | ||
| - **Verification approach**: inner — launcher/path-resolution tests plus packaged app smoke checks. Outer — packaged manual walkthrough includes seeded closed-project knowledge/export routes using `forced-close-all-phases-closed` and `low-readiness-all-phases-closed`, so the deferred Phase 6 browser coherence pass lands at the real distribution boundary rather than in the pre-distribution refactor thread. | ||
| - Design: `docs/design/LOCAL_STORAGE.md` | ||
| 14. **Local-first storage + npx distribution** `done` |
There was a problem hiding this comment.
memory/PLAN.md:159 (process) — the PR title doesn’t match the repo PR title convention (FE-XXX: ...). See (Rule: AGENTS.md).
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
lunelson
changed the title
lunelson
changed the base branch from
ln/fe-578-ui-refine-expand
to
graphite-base/43
BrunchProject resolution with walk-up discovery (find/init/resolve). Express launcher serves API + static dist/ on one port. Bin entry for npx @hashintel/brunch. Drizzle migrations path resolved via import.meta.url so it works from any cwd. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lunelson
force-pushed
the
ln/fe-545-storage-and-distro
branch
from
6306579 to
0b3dc6a
Compare
1 participant
feat: local-first .brunch/ storage, project resolution, and launcher
BrunchProject resolution with walk-up discovery (find/init/resolve).
Express launcher serves API + static dist/ on one port. Bin entry for
npx @hashintel/brunch. Drizzle migrations path resolved via import.meta.url
so it works from any cwd.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
chore: mark slice 14 done, add project resolution invariant I100
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com