diff --git a/CHANGELOG.md b/CHANGELOG.md index 6020419270..3c69357dab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ All notable changes to this project will be documented in this file. ### Fixed +- **notion-datasource-sync / notion-cli**: `notion db sync`, `track`, and + `export` now work under the packaged Node runtime without a `tsx` preload. The + CLI sync-progress path loaded the `.tsx` TUI via `Effect.promise(() => import(…))` + wrapped in `Effect.either`; under packaged Node the JSX import rejects, which + surfaces as an Effect **defect** that `Effect.either` does not catch, so the + intended plain-progress fallback was bypassed and the command exited 1 with no + output. The load defect is now promoted onto the error channel + (`Effect.catchAllDefect`, scoped to the load step only) so any TUI load failure + degrades cleanly to plain progress. Separately, the `notion` flake wrapper did + not route `db track` to the Node runtime (only `init|pull|push|sync|export|status|conflicts|forget|restore|doctor`), + so `notion db track` fell through and failed closed; `track` is now routed. A + new unit test guards the fallback with an injected dying loader, and the Nix + smoke test is strengthened from a `--help`-only route check to a REAL + sync-path run: it invokes `notion db track` on an empty workspace, which + reaches `runWithCliSyncProgress` and the real `.tsx` TUI import, and asserts a + structured `CliErrorEnvelope` is emitted (proving the import failed soft and + dispatch was reached). Deleting the `catchAllDefect` line makes the build RED + with "no structured envelope"; dropping `track` routing trips the Bun-guard + check. - **CI / cargo**: move standalone Rust crate build/test/clippy/fmt semantics into the `cargo:check` devenv task and make the generated cargo CI lane call that task, ensuring the `node-cpuprofile` integration test runs with the devenv diff --git a/packages/@overeng/notion-cli/nix/build.nix b/packages/@overeng/notion-cli/nix/build.nix index c2dc852337..52aea7d716 100644 --- a/packages/@overeng/notion-cli/nix/build.nix +++ b/packages/@overeng/notion-cli/nix/build.nix @@ -60,7 +60,7 @@ pkgs.runCommand "notion-cli" '' mkdir -p $out/bin makeWrapper ${unwrapped}/bin/notion $out/bin/notion \ - --run 'if [ "$#" -gt 1 ] && [ "$1" = db ]; then case "$2" in init|pull|push|sync|export|status|conflicts|forget|restore|doctor) shift; exec ${notionDbRuntime}/bin/notion-db-runtime "$@";; esac; fi' + --run 'if [ "$#" -gt 1 ] && [ "$1" = db ]; then case "$2" in init|pull|push|sync|track|export|status|conflicts|forget|restore|doctor) shift; exec ${notionDbRuntime}/bin/notion-db-runtime "$@";; esac; fi' db_output="$($out/bin/notion db sync --help 2>&1 || true)" if ! printf '%s\n' "$db_output" | grep -q 'Reconcile an established workspace'; then @@ -68,4 +68,46 @@ pkgs.runCommand "notion-cli" echo "notion db sync smoke test failed" >&2 exit 1 fi + + # `track` must also route to the Node runtime (needs node:sqlite). A missing + # route falls through to the Bun-backed wrapper, which fails closed. Grep for + # a phrase unique to the Node entrypoint's help block ("Packaged Node-backed + # entrypoint from Nix/devenv" — see renderCliHelpText); the Bun @effect/cli + # help does not emit it, so this specifically proves `track` is routed. + track_output="$($out/bin/notion db track --help 2>&1 || true)" + if ! printf '%s\n' "$track_output" | grep -q 'Packaged Node-backed entrypoint from Nix/devenv'; then + printf '%s\n' "$track_output" >&2 + echo "notion db track smoke test failed (track not routed to node runtime?)" >&2 + exit 1 + fi + + # Real sync-path regression: a `--help` route check cannot catch the actual + # defect it guards against. A progress-bearing verb (track) runs through + # `runWithCliSyncProgress`, which dynamically imports the `.tsx` + # `@overeng/tui-react` TUI. Under packaged Node that import REJECTS (JSX is + # not stripped) and surfaces as an Effect *defect*; the top-level handler is + # `Effect.tapError` (failures only), so without the `catchAllDefect` fallback + # the command dies BEFORE emitting any structured output. `track` is the + # cheapest verb that clears argument+context parsing on an empty workspace + # and thus reaches the progress wrapper (`sync` fails earlier as + # WorkspaceNotTracked, never reaching it). The build sandbox has no network, + # so the establish then fails FAST (auth/connection errors are non-retryable) + # with a structured `CliErrorEnvelope` — whose presence proves the TUI import + # failed soft and real command dispatch was reached. Deleting the + # `catchAllDefect` line in runWithCliSyncProgress makes this RED (raw defect, + # no envelope); dropping `track` from the wrapper routing makes it RED via the + # Bun-runtime guard check below. + track_run_dir="$(mktemp -d)" + track_run_output="$(NOTION_API_TOKEN=secret_smoke_invalid_000000000000000000000000 \ + timeout 90 $out/bin/notion db track 11111111-1111-4111-8111-111111111111 "$track_run_dir" --mode local 2>&1 || true)" + if ! printf '%s\n' "$track_run_output" | grep -qE '"_tag": "Cli(Error|Result)Envelope"'; then + printf '%s\n' "$track_run_output" >&2 + echo "notion db track sync-progress smoke failed: no structured envelope — the .tsx TUI import defect was not caught (runWithCliSyncProgress catchAllDefect regressed?)" >&2 + exit 1 + fi + if printf '%s\n' "$track_run_output" | grep -q 'require the packaged Nix/devenv Node-backed runtime'; then + printf '%s\n' "$track_run_output" >&2 + echo "notion db track fell through to the Bun-runtime guard (not routed to the Node runtime)" >&2 + exit 1 + fi '' diff --git a/packages/@overeng/notion-datasource-sync/src/cli/main.ts b/packages/@overeng/notion-datasource-sync/src/cli/main.ts index e23076c55a..19e8b4ce3b 100755 --- a/packages/@overeng/notion-datasource-sync/src/cli/main.ts +++ b/packages/@overeng/notion-datasource-sync/src/cli/main.ts @@ -3149,24 +3149,45 @@ const runWithPlainSyncProgress = ({ ), ) -const runWithCliSyncProgress = ({ +/** + * Loads the optional TUI progress modules. The `@overeng/tui-react` module is a + * `.tsx` file; under a runtime that cannot strip JSX (plain packaged Node) the + * dynamic `import()` REJECTS, which `Effect.promise` surfaces as a **defect** + * (die), not a typed failure. `runWithCliSyncProgress` therefore catches the + * whole load as a cause — see the `Effect.catchAllDefect` there — so any load + * failure degrades to plain progress rather than crashing the command. + */ +const loadSyncProgressTui = Effect.promise(() => import('./progress.ts')).pipe( + Effect.flatMap((progressModule) => + Effect.promise(() => import('@overeng/tui-react')).pipe( + Effect.flatMap((tuiReact) => + Effect.promise(() => import('@overeng/tui-react/node')).pipe( + Effect.map((tuiReactNode) => ({ progressModule, tuiReact, tuiReactNode })), + ), + ), + ), + ), +) + +export const runWithCliSyncProgress = ({ command, effect, + loadTui = loadSyncProgressTui, }: { readonly command: CliCommand readonly effect: Effect.Effect + readonly loadTui?: typeof loadSyncProgressTui }): Effect.Effect => { - const loadTuiProgress = Effect.promise(() => import('./progress.ts')).pipe( - Effect.flatMap((progressModule) => - Effect.promise(() => import('@overeng/tui-react')).pipe( - Effect.flatMap((tuiReact) => - Effect.promise(() => import('@overeng/tui-react/node')).pipe( - Effect.map((tuiReactNode) => ({ progressModule, tuiReact, tuiReactNode })), - ), - ), - ), - ), - Effect.either, + // Loading the optional TUI can fail under a Node runtime that can't strip the + // JSX in `@overeng/tui-react`: the rejected dynamic import surfaces as an + // Effect **defect**, which `Effect.either` alone does NOT catch. Recover the + // defect into a `Left` (keeping the cause in the success channel, so the error + // channel stays clean) so any load failure cleanly falls back to + // `runWithPlainSyncProgress`. This catch wraps ONLY the load step, never + // `effect`, so genuine command errors and interruption still surface. + const loadTuiProgress = loadTui.pipe( + Effect.map(Either.right), + Effect.catchAllDefect((defect) => Effect.succeed(Either.left(defect))), ) return loadTuiProgress.pipe( diff --git a/packages/@overeng/notion-datasource-sync/src/cli/sync-progress-fallback.unit.test.ts b/packages/@overeng/notion-datasource-sync/src/cli/sync-progress-fallback.unit.test.ts new file mode 100644 index 0000000000..d7cdb986f9 --- /dev/null +++ b/packages/@overeng/notion-datasource-sync/src/cli/sync-progress-fallback.unit.test.ts @@ -0,0 +1,67 @@ +import { Effect } from 'effect' +import { afterEach, describe, expect, it } from 'vitest' + +import { runWithCliSyncProgress } from './main.ts' +import type { CliCommand } from './main.ts' + +/** + * Regression guard for the packaged-Node `.tsx` import defect. + * + * `runWithCliSyncProgress` optionally loads the TUI progress UI via dynamic + * `import('@overeng/tui-react')`. `@overeng/tui-react` is a `.tsx` module; under + * a runtime that cannot strip JSX (plain packaged Node) that import REJECTS, + * which `Effect.promise` surfaces as a **defect** (die), not a typed failure. + * + * The bug: the fallback used `Effect.either`, which does NOT catch defects, so a + * failed load bypassed `runWithPlainSyncProgress` and the command died with zero + * output (exit 1). The fix promotes the defect onto the error channel before + * `Effect.either`, so any load failure cleanly degrades to plain progress. + * + * This test injects a loader that reproduces the real failure mode — a rejected + * dynamic import via `Effect.promise` (a die) — and asserts the command still + * completes AND observably ran plain progress (it writes phase lines to stderr). + * No network, no real `.tsx` transpile (vitest transpiles `.tsx` fine, so the + * real import cannot fail here — injecting the raw dying loader is what exercises + * the defect path). Deleting the `catchAllDefect` line in `main.ts` makes this + * test crash, confirming it is a genuine RED guard. + */ +describe('runWithCliSyncProgress TUI-load fallback', () => { + const originalWrite = process.stderr.write.bind(process.stderr) + + afterEach(() => { + process.stderr.write = originalWrite + }) + + // Reproduce the packaged-Node failure: a rejected dynamic import surfaced by + // `Effect.promise` as a defect (die), NOT a typed failure. Injecting past the + // fix (e.g. `Effect.fail`) would prove nothing — the whole point is the defect. + const dyingTuiLoader = Effect.promise(() => + Promise.reject(new Error('Node cannot strip JSX from @overeng/tui-react (.tsx)')), + ) as ReturnType> + + it('falls back to plain progress when the TUI import dies (defect), instead of crashing', async () => { + const command: CliCommand = { _tag: 'doctor' } + + const captured: Array = [] + process.stderr.write = ((chunk: string | Uint8Array): boolean => { + captured.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')) + return true + }) as typeof process.stderr.write + + const result = await Effect.runPromise( + runWithCliSyncProgress({ + command, + effect: Effect.succeed('command-ran'), + loadTui: dyingTuiLoader, + }), + ) + + process.stderr.write = originalWrite + + // The command's own effect still ran and produced its value... + expect(result).toBe('command-ran') + // ...and the fallback observably used PLAIN progress (stderr phase lines), + // not the TUI, and did not crash on the load defect. + expect(captured.join('')).toContain('notion db doctor complete 100%') + }) +})