fix(explorer): harden mount gate, tests, and watch lifecycle by tonyketcham · Pull Request #232 · FlatbreadLabs/flatbread

tonyketcham · 2026-07-28T09:42:24Z

Keep --open and mount on the shared assets gate, isolate AVA fixtures
with a static-dir override (no package-global index.html renames), use
vite base / for nested SPA routes, toggle explorer on config reload
under --watch, and document the built-assets requirement in the skill.

Co-authored-by: Cursor cursoragent@cursor.com

Depends-On: #231

tonyketcham · 2026-07-28T09:42:26Z

This pull request is part of a Mergify stack:

#	Pull Request	Link
1	feat(explorer): ship Effort Graph visualizer with flatbread start	#230
2	fix(explorer): restore monorepo path aliases for typecheck	#231
3	fix(explorer): harden mount gate, tests, and watch lifecycle	#232	👈

Keep --open and mount on the shared assets gate, isolate AVA fixtures with a static-dir override (no package-global index.html renames), use vite base `/` for nested SPA routes, toggle explorer on config reload under --watch, and document the built-assets requirement in the skill. Co-authored-by: Cursor <cursoragent@cursor.com> Change-Id: I1f5f06aa4e0caba4e517f4608c39e907cb567f45 Co-authored-by: Cursor <cursoragent@cursor.com>

tonyketcham · 2026-07-28T10:01:34Z

Revision history

#	Type	Changes	Reason	Date
1	initial	`7b6d7db`		2026-07-28 10:01 UTC
2	content	`7b6d7db → 58a9a82` (raw)	Drop monorepo build/play:efforts instructions from the packaged public skill; keep contributor tip only in .agents.	2026-07-28 10:01 UTC

Keep .agents/skills/effort-graph/setup.md identical to the package skill source so skills:check passes. Tolerate @parcel/watcher EINVAL on Node 22 when unsubscribe runs after the native handle is gone. Change-Id: Ida461356c8d75fde5b3a7203e587433661d45107

Concurrent AVA fixtures under cwd (.tmp-effort-*, .journal) race inotify_add_watch while liveServer watches the repo root. Ignore those paths, keep watcher callback errors from rejecting the server, and retry subscribe when inotify_add_watch reports a missing path. Change-Id: I8144a16864a00dd5a263a68c8ab08ea7d05b9105

cursor · 2026-07-28T10:27:22Z

@Mergifyio queue

mergify · 2026-07-28T10:27:28Z

queue

☑️ Command disallowed due to command restrictions in the Mergify configuration.

Details

sender-permission >= write

cursor

Verdict: REQUEST_CHANGES

Consensus HIGH on process-global setExplorerStaticDirOverride races in non-serial AVA tests, plus uncovered documented mount-gate contracts (warn-once, assets flip, rejected replaceConfig) on non-test source.

Blocking / highest priority

openPath.test.ts — override set/clear must be fully test.serial (assets-present path is still concurrent).
liveServer.ts replaceConfig wrapper — gate updates on commit, but effort-graph composition stays startup-only; rejected reloads and late enable need stronger asserts.
explorerMount update() — documented warn-once + assets flip under a still-matching preset are untested.

Coverage plan (ordered)

openPath.test.ts — edge:override-serial — all override set/clear cases use test.serial.
explorerMount.test.ts — edge:warn-dedupe — second missing-assets update() stays silent; leave+re-enter warns again.
explorerMount.test.ts — edge:assets-flip — matching preset + remove/restore index.html toggles inactive/active.
liveServerExplorerWatch.test.ts — negative:rejected-reload — rejected replaceConfig leaves server.explorer and / unchanged.
liveServerExplorerWatch.test.ts — pos|neg:effort-graph-after-enable — after late preset add, assert Effort Graph surface / server.effortGraph per intended attach contract.
liveServerExplorerWatch.test.ts — negative:assets-mid-run — empty override with matched preset clears server.explorer.
staticDir.test.ts — positive:override-with-assets — temp dir with index.html asserts true without relying on a prior build.

Reviewer scoreboard

correctness-and-contracts: 4 findings, 4 coverage gaps, signal:HIGH
test-coverage-robustness: 5 findings, 5 coverage gaps, signal:HIGH
cli-and-runtime: 4 findings, 5 coverage gaps, signal:MED
explorer-package: 5 findings, 5 coverage gaps, signal:MED
docs-and-positioning: 3 findings, 3 coverage gaps, signal:MED

Perspectives: correctness, coverage, cli-runtime, explorer-package, docs. Models: grok-4.5 high / composer-2.5.

_{Sent by Cursor Automation: Flatbread PR Review}

cursor · 2026-07-28T10:32:33Z

+test('opens explorer root when preset matches and assets are present', (t) => {
+  setExplorerStaticDirOverride(undefined);
+  if (!explorerAssetsPresent()) {
+    t.fail(
+      'Explorer assets missing. Build @flatbread/explorer first (`pnpm --filter @flatbread/explorer build`).'
+    );
+    return;
+  }
+  t.is(resolveOpenPath(effortGraphContent()), EXPLORER_ENDPOINT);
+  t.is(resolveOpenPath(effortGraphContent()), '/');


severity: HIGH (correctness-and-contracts, cli-and-runtime, explorer-package)

This case clears/sets the process-wide setExplorerStaticDirOverride while other suites may run concurrently. Only the missing-assets case below is test.serial, so explorerAssetsPresent / resolveOpenPath can flake false fail or pass.

Minimal fix: Make every override-mutating case test.serial (one serial block), including this assets-present path and any cross-file consumers.

cursor · 2026-07-28T10:32:33Z

+let staticDirOverride: string | undefined;
+
+/**
+ * Test-only: force `getExplorerStaticDir()` to `dir`.
+ * Pass `undefined` to clear. Not for production callers.
+ */
+export function setExplorerStaticDirOverride(dir: string | undefined): void {
+  staticDirOverride = dir;
+}
+


severity: MED (cli-and-runtime, explorer-package)

Public process-global override has no isolation. Any parallel AVA worker mutating it can corrupt unrelated suites (same root cause as the openPath HIGH).

Minimal fix: Move the override behind a test-only export path, or document it and require serial consumers everywhere it is set/cleared.

cursor · 2026-07-28T10:32:33Z

+  // Explorer SPA first so `/` is the visualizer when a preset matches. Middleware
+  // is registered once; the gate toggles when replaceConfig commits (watch
+  // applyConfig and any direct reload). Inactive → `next()` for `/events` /
+  // `/graphql`.
+  const explorerHandle = mountExplorer(app, config.content);
+  const replaceConfig = reloader.replaceConfig.bind(reloader);
+  reloader.replaceConfig = async (nextConfig) => {
+    const result = await replaceConfig(nextConfig);
+    if (result.status === 'committed') {
+      explorerHandle.update(nextConfig.content);
+    }
+    return result;
+  };


severity: MED (correctness-and-contracts, test-coverage-robustness, cli-and-runtime)

Committed replaceConfig updates the explorer gate, but effort-graph composition / barrier / attach stay startup-only, and rejected reloads are untested. Late preset enable can advertise explorer without effortGraph while callers may assume the gate tracks successful attach.

Minimal fix: On committed reload, recompute/reattach composition (or refuse explorer-on until composition exists and document that), and assert gate// unchanged on rejected.

cursor · 2026-07-28T10:32:33Z

+    t.is(result.status, 'committed');
+    t.true(server.explorer);
+
+    const homeAfter = await fetch(`http://localhost:${server.port}/`);
+    t.is(homeAfter.status, 200);
+    t.true((await homeAfter.text()).includes('__FLATBREAD_EXPLORER__'));
+
+    const gql = await query(server.port, '{ __typename }');
+    t.deepEqual(gql.errors, undefined);
+    t.is(gql.data?.__typename, 'Query');


severity: MED (correctness-and-contracts, test-coverage-robustness)

“Enables explorer when preset is added” only checks SPA HTML + __typename, so a composition/attach gap stays green. Also missing: rejected-reload no-op, assets-mid-run flip, inactive bootstrap fallthrough.

Minimal fix: After enable, assert Effort Graph query surface and/or server.effortGraph (pos+neg per contract); add rejected-reload and assets-override negatives.

cursor · 2026-07-28T10:32:33Z

+        inMissingAssets = true;
+      }
+      active = false;
+      staticMiddleware = null;
+      return;


severity: MED (test-coverage-robustness)

Documented “warn once per missing-assets transition” has no test that a second update() stays silent or that leave+re-enter warns again. Related gap: assets flip under a still-matching preset via override is unproven.

Minimal fix: Serial cases counting console.warn across empty → restored → empty cycles, plus isActive()// HTML off/on when override removes/restores index.html.

cursor · 2026-07-28T10:32:34Z

    try {
      const loaded = await loadFlatbreadConfig(process.cwd());
-      openPath = resolveCliOpenPath(loaded.config?.content);
+      openPath = resolveOpenPath(loaded.config?.content);


severity: MED (cli-and-runtime)

Under --watch, welcome/--open freeze the startup explorer decision while liveServer can later toggle the gate via replaceConfig.

Minimal fix: Re-resolve open path (or query live readiness) before launch/welcome when watch is true, or document explorer links as startup-only.

cursor · 2026-07-28T10:32:34Z

  plugins: [react()],
  root: '.',
-  base: './',
+  base: '/',


severity: MED (explorer-package)

base: '/' emits root-absolute asset URLs that 404 under subpath hosting while the README still implies portable static deploys.

Minimal fix: Restore base: './' for portable hosts, or narrow the README to root-only / Flatbread-at-/ hosting.

cursor · 2026-07-28T10:32:34Z

+pnpm play:efforts   # builds explorer, then flatbread start --watch --open
+```
+
+For UI-only iteration with HMR, run Flatbread and Vite in separate terminals
+(Vite proxies `/graphql` and `/events` to Flatbread on port **5057**, or
+`FLATBREAD_PORT` when set):
+
+```bash
+pnpm exec flatbread start --watch          # terminal 1 — GraphQL on :5057
+pnpm --filter @flatbread/explorer dev      # terminal 2 — SPA on :5173


severity: MED (docs-and-positioning, explorer-package)

HMR docs imply FLATBREAD_PORT / -p and which URL to open, but the Vite terminal does not inherit the forked server’s port and readers may hit stale packaged assets on 5057.

Minimal fix: Document exporting FLATBREAD_PORT in the Vite terminal for non-default ports, say open 5173 for HMR (5057 API-only), and note custom graphql/events paths need extra proxy rules without __FLATBREAD_EXPLORER__.

cursor · 2026-07-28T10:32:34Z

 - **Workspace libraries (watch-only):** `pnpm dev` — runs package `dev` scripts (e.g. `tsup --watch`) for `packages/*`; it does **not** start the Next.js example.
 - **Next.js example:** prefer the flow under [Recommended onboarding](#recommended-onboarding-try-flatbread-in-the-nextjs-example); or `pnpm play` as a convenience alias.
- **Effort Graph explorer:** after `pnpm build`, run `pnpm play:efforts` (`flatbread start --watch --open`). When `flatbread.config.js` uses `effortGraphContent()`, Flatbread serves `@flatbread/explorer` at `http://localhost:5057/` (Apollo sandbox at `/graphql`).
+- **Effort Graph explorer:** run `pnpm play:efforts` (builds `@flatbread/explorer` via `preplay:efforts`, then `flatbread start --watch --open`). When `flatbread.config.js` uses `effortGraphContent()`, Flatbread serves `@flatbread/explorer` at `http://localhost:5057/` (Apollo sandbox at `/graphql`). For HMR on the SPA shell, run `flatbread start --watch` and `pnpm --filter @flatbread/explorer dev` in parallel (Vite on **5173** proxies API routes to **5057**).


severity: MED (docs-and-positioning)

Docs claim explorer is always at http://localhost:5057/ for effortGraphContent(), but mount/--open now require assets and HMR UI is on 5173.

Minimal fix: Add the same “when mounted / assets present” qualifier as setup.md, and point HMR UI at 5173.

This was referenced Jul 28, 2026

feat(explorer): ship Effort Graph visualizer with flatbread start #230

Closed

fix(explorer): restore monorepo path aliases for typecheck #231

Closed

tonyketcham force-pushed the stack/tonyketcham/feat/explorer-package/harden-mount-gate-tests-watch-lifecycle--1f5f06aa branch from 7b6d7db to 58a9a82 Compare July 28, 2026 10:01

tonyketcham mentioned this pull request Jul 28, 2026

feat(explorer): ship Effort Graph visualizer with flatbread start #233

Merged

5 tasks

tonyketcham closed this Jul 28, 2026

cursor Bot reviewed Jul 28, 2026

View reviewed changes

tonyketcham mentioned this pull request Jul 28, 2026

docs(releases): Effort Graph alpha readiness check and release notes #234

Draft

3 tasks

Uh oh!

Uh oh!

Conversation

tonyketcham commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyketcham commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyketcham commented Jul 28, 2026

Revision history

Uh oh!

cursor Bot commented Jul 28, 2026

Uh oh!

mergify Bot commented Jul 28, 2026

☑️ Command disallowed due to command restrictions in the Mergify configuration.

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Verdict: REQUEST_CHANGES

Blocking / highest priority

Coverage plan (ordered)

Reviewer scoreboard

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tonyketcham commented Jul 28, 2026 •

edited

Loading

tonyketcham commented Jul 28, 2026 •

edited

Loading