Skip to content

feat(ee): merge Workspace into the monorepo as a built-in extension - #3455

Open
ToddHebebrand wants to merge 22 commits into
mainfrom
ToddHebebrand/workspace-ee-merge
Open

feat(ee): merge Workspace into the monorepo as a built-in extension#3455
ToddHebebrand wants to merge 22 commits into
mainfrom
ToddHebebrand/workspace-ee-merge

Conversation

@ToddHebebrand

@ToddHebebrand ToddHebebrand commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Moves the Breeze Workspace extension (formerly LanternOps/breeze-workspace,
private) into this repo at ee/workspace/ (@breeze/ext-workspace), loaded
as a built-in extension — statically imported, compiled into the API
image, and registered at boot through the same staged v1 host pipeline
(contribution registry, default-deny gateway, tenancy/RLS tripwires) that
signed runtime bundles use, but with no signing, no artifact verification,
and no source-directory scan. This path is reserved for first-party code
that lives in this repository (currently just ee/workspace); third-party
extensions are unaffected and continue to arrive exclusively as signed
runtime bundles declared in extensions.yaml. ee/ carries the Breeze
Commercial License; everything else in the repo remains AGPL-3.0. The two-repo
split, vendored-SDK tarballs, Ed25519 signing/packing pipeline, live
stock-host conformance suite, machine-local pnpm.overrides hack, and
symlinked extensions/workspace dev flow are all retired for Workspace — see
the deletion/retirement inventory below.

Spec: docs/superpowers/specs/2026-08-11-workspace-ee-merge-design.md
Plan: docs/superpowers/plans/2026-08-11-workspace-ee-merge.md

Deletion / retirement inventory

  • extensions/workspace symlink dev flow — retired. The symlink itself
    is untracked and gitignored (extensions/* in .gitignore, except
    README.md) in every checkout, including the main ~/breeze checkout, so
    there was never anything for this branch to git rm. It does not exist at
    all in this worktree. See the Todd checklist below for removing it from
    disk in the main checkout.
  • Vendored-SDK tarballs / .stubs/ — deleted with the source move
    (Task 1/2); Workspace now depends on @breeze/extension-sdk /
    @breeze/extension-web-sdk via workspace:*, same as every other
    extension in this repo.
  • Packer / sign / stage / validate pipeline usage for Workspace — retired.
    The signed-runtime-bundle platform itself is unchanged and stays in place
    for third-party extensions.
  • Live stock-host conformance suite — retired as a Workspace gate. The
    platform's own conformance coverage (apps/api/src/extensions/packerConformance.test.ts)
    no longer needs Workspace as its reference guinea pig; docs/extensions/build-time-transition.md
    now points to that fixture instead.
  • Root package.json pnpm.overrides for vendored Workspace SDKs, and the
    compose-override ./package.json:/app/package.json mount
    — both were
    already outside git (a machine-local unstaged edit and an untracked compose
    override respectively), so neither is touched by this branch's diff. Left
    as a Todd checklist item (below) rather than editing his tree.
  • LanternOps/breeze-workspace (private repo) — to be archived
    read-only as the history record once this PR merges; not part of this
    branch's diff.

Built-in extension path (what actually changed under the hood)

  • apps/api/src/extensions/builtinExtensions.ts (Task 3) statically imports
    the workspace package's parsed manifest and v1 register entry and feeds
    them through the same staged v1 pipeline as runtime bundles.
  • apps/api/src/index.ts calls loadBuiltinExtensions() at boot, strictly
    between loadSourceExtensions() and the signed-bundle reconcileExtensions()
    call (Task 4); any built-in load failure aborts boot.
  • Migrations: unchanged mechanism — the extension migrator runs
    ee/workspace/migrations/*.sql; the built-in registration hands it a
    resolved migrationsDir.
  • Three Dockerfiles were found to need this wiring, not one:
    docker/Dockerfile.api, apps/api/Dockerfile (the actual published
    release image — .github/workflows/release.yml and
    edge-image-api.yml build from this file, not docker/Dockerfile.api),
    and docker/Dockerfile.api.dev (the hot-reload dev flow). All three now
    bake in ee/workspace's manifest, node_modules, source/build output, and
    migrations, each honoring its own runtime WORKDIR/process.cwd()
    convention for where builtinRegistry.ts's built-in-root resolution looks
    (/app/ee/workspace for the two images whose runner stays at /app;
    /app/apps/api/ee/workspace for apps/api/Dockerfile, whose runner ends
    at WORKDIR /app/apps/api). Each was verified with a real docker build +
    docker run smoke check (Task 5).
  • apps/api/tsup.config.ts's noExternal explicitly lists Workspace's two
    non-@breeze/* third-party runtime deps (mailparser, v9u-smb2) so
    their inline bundling is a documented, guarded contract rather than an
    implicit side effect of apps/api's own dependency list; a new static guard
    test (apps/api/src/config/extensionBundledDependencies.test.ts) catches
    the next genuinely-dangling dependency before it reaches a Docker build.
  • CI (.github/workflows/ci.yml, Task 6): ee/workspace unit tests +
    typecheck run in the test-api job (no DB needed). The integration suite
    needs workspace_* tables, which only come from a real loadBuiltinExtensions()
    boot (nothing in the existing test harness — CI or local — ever ran that
    path before this branch); a new "Boot API once to apply built-in extension
    migrations" step runs tsx src/index.ts with AUTO_MIGRATE=true against
    the job's Postgres service, waits for the loaded built-in "workspace" log
    line (or a CRITICAL failure) with a 180s timeout, then stops it (own
    process group via setsid, bounded shutdown wait with a kill -9
    fallback) — before the ee/workspace integration-test step runs. Both new
    steps are gated if: matrix.shard == 1 since the suite needs exactly one
    DB, not a sharded slice. Note: these steps have never executed in a
    real Actions run — watch this PR's first CI run before merging.

Docs

  • docs/extensions/build-time-transition.md — added a "Built-in (first-party)
    extensions" section documenting the new delivery mode as distinct from
    signed bundles and the deprecated source-directory path. Also reconciled
    now-stale claims found via
    grep -rn "public SDK/host code only\|stock image" docs/extensions extensions/README.md docker/:
    the old "stock image contains public SDK/host code only" line, and the
    file's two now-inaccurate breeze-workspace-as-reference-implementation /
    conformance-reference mentions (Workspace no longer takes the
    source-directory → signed-bundle path, so it no longer fits as that
    guide's worked example; the conformance reference is now
    apps/api/src/extensions/packerConformance.test.ts). docker/Dockerfile.api's
    own stock-image comment was already corrected in Task 5.
  • extensions/README.md — added a note that first-party extensions
    (ee/workspace) do not live in this directory; it is exclusively for
    third-party signed-bundle delivery.

Verification

  • grep -rn "extensions/workspace" --include="*.ts" --include="*.yml" --include="*.yaml" apps packages docker .github | grep -v node_modules — clean, no hits.
  • pnpm --filter @breeze/api exec vitest run src/extensions/ src/config/ — 32 files / 700 tests passed.
  • pnpm lint — 6/6 packages, clean (includes @breeze/ext-workspace's own eslint src).
  • pnpm test:docs-automation (the doc-verify CI's guard) — 5/5 passed.
  • NODE_OPTIONS=--max-old-space-size=8192 pnpm exec tsc --noEmit --project apps/api/tsconfig.json — clean (Task 5's final fix round).

Final whole-branch review

An independent final review (spec-vs-branch, cross-task seams, security, prod
readiness) returned "ready with fixes"; all three fixes landed in 422b38d2a
(CI boot-step process-group kill + bounded wait + 180s window; workspace_crawl_runs.cursor
excluded from tenant export — it's an opaque agent resume token, matching the
sibling crawl_cursor exclusion; ee/workspace/src + migrations mounted
into both docker dev compose flows so in-container tsx sees ee edits).
Recommended follow-up issues (not blocking): (1) built-in installed_extensions
lifecycle polish — record activeVersion only after activation, drop the
redundant first-boot setEnabled, add a mid-pipeline-failure test; (2)
routeNamespace-level collision gate across all three delivery paths (name-only
today, pre-existing gap). One open design question for Todd: should
ee/workspace/manifest.json declare tenancy.installScope: "org" now that
tenant-scoped installs (#3032) exist, or stay server-scoped with org-gating via
workspace_org_settings as before? Current behavior matches pre-merge.

Deployment note: Workspace is opt-in (BREEZE_WORKSPACE_ENABLED)

Built-ins now carry a deployment enable flag; Workspace's is BREEZE_WORKSPACE_ENABLED, default OFF (commit 19534184f). With the flag unset, the API runs no workspace migrations, requires no pgvector, and requires no web bundle — any Postgres 16 image boots clean (one structured builtin_extension_disabled log line). Setting it to exactly "true" loads Workspace through the full pipeline, and then pgvector is required: a plain-postgres DB fails boot with an actionable error naming both remedies (pgvector image, or unset the flag). A previously-enabled deployment that turns the flag off keeps its tables safe — the disabled path publishes the tenancy declaration iff the tables exist, so cascades/exports/sweeps stay correct. Dev composes set the flag; the stock POSTGRES_IMAGE_REF default stays pgvector/pgvector:pg16 so enabling later needs no DB swap. CI covers both paths: shard-1 integration boots flag-on; the compose Smoke Test and both binary-source jobs prove the default-off boot. Verified live in three boots: plain-pg flag-off, pgvector flag-on, and flag-off-with-existing-tables.

Todd checklist

  • Discard the machine-local, unstaged root package.json pnpm.overrides
    edit for vendored Workspace SDKs — dead now that Workspace resolves via
    workspace:* in-repo.
  • Delete the untracked compose-override ./package.json:/app/package.json
    mount — dead for the same reason.
  • Remove the extensions/workspace symlink from the main ~/breeze
    checkout if still present (rm extensions/workspace) — it is untracked
    and gitignored, so nothing in this PR removes it automatically; it just
    becomes a stale, unused symlink once this merges.

🤖 Generated with Claude Code

Todd Hebebrand and others added 15 commits August 11, 2026 13:30
Approved direction: move breeze-workspace into the monorepo at ee/workspace
as a built-in extension under a commercial license; retire the two-repo
packaging seam for first-party code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ace)

Source imported from LanternOps/breeze-workspace @ c5799345b178e74b6d066a02dacb46d55caf0f57.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…irst built-in

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d boot order built-in-aware

Review fixes for the built-in extension loading path:

1. ee/workspace/manifest.json declared 13 orgCascadeDeleteTables with no
   tenancy.orgExportColumns. Publishing that tenancy at boot made
   getExtensionOrgExportColumns() throw "missing an export classification",
   which 500s every org's GDPR right-of-access export. Classify all 13 tables
   against the migration DDL (excluding only workspace_sources.credential_enc,
   crawl_cursor, workspace_content_chunks.embedding and
   workspace_org_settings.dlp_config).

2. State the boot-order contract in loadBuiltinExtensions' doc comment: after
   loadSourceExtensions, strictly BEFORE reconcileExtensions, or the
   reconciler's repo-wide sweep aborts every boot after the first.

3. loadSourceExtensions' repo-wide sweep could not see built-in tables, so a
   legacy source extension with no extensions.yaml refused to boot. Move the
   built-in registry into a leaf module (builtinRegistry.ts, avoiding a
   loader <-> builtinExtensions cycle) and union builtinTenancyDeclarations()
   into the sweep input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o the API image

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eload flow

apps/api/Dockerfile (the actual release/edge image builder, referenced by
release.yml and edge-image-api.yml -- distinct from docker/Dockerfile.api)
had the same missing-manifest/missing-source gap as docker/Dockerfile.api's
first build attempt: apps/api depends on @breeze/ext-workspace but nothing
copied its manifest, node_modules, or source, so pnpm install and the tsup
build (which inlines ee/workspace via noExternal: [/^@Breeze\//]) would
break. Fixed by mirroring this Dockerfile's own bulk-copy style (COPY
packages / COPY ee, not per-package like docker/Dockerfile.api). Its runner
WORKDIR ends at /app/apps/api (unlike docker/Dockerfile.api's /app), so the
ee/ copy destination is apps/api/ee to match builtinRegistry.ts's
process.cwd()-relative fallback -- verified by booting the built image and
confirming ee/workspace/manifest.json resolves relative to cwd.

docker/Dockerfile.api.dev never copied ee/workspace (or its
extension-web-sdk/extension-testkit deps) at all, so the Docker hot-reload
dev flow (docker-compose.override.yml.dev) would ENOENT at import in
builtinRegistry.ts. Fixed by mirroring the file's own existing per-package
manifest+source COPY pattern (same as extension-cli). Verified by building
the dev image and confirming the fromSource resolution path (used when
running via tsx, not the CJS bundle) finds ee/workspace/manifest.json at
the expected /app/ee/workspace.

dockerfileWorkspaceManifests.test.ts is a repo-standing guard that statically
asserts every Dockerfile copies every transitive workspace:* manifest and
source it needs before installing/building. It only knew about apps/* and
packages/* as workspace roots, so it started hard-failing (at module-collection
time, via a top-level expect()) the moment ee/* became a workspace glob and
apps/api gained a workspace:* dependency on @breeze/ext-workspace -- before
any of its Dockerfile-consistency assertions could even run. Taught it ee/*
is a workspace root (matching pnpm-workspace.yaml) rather than weakening any
assertion; the "no other globs" check still fails on an unlisted glob. With
the Dockerfile fixes above, all 26 of its assertions now pass for real,
including the two that caught this exact class of bug.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uarded

apps/api's tsup build bundles the built-in ee/workspace extension's source
via noExternal: [/^@Breeze\//, ...]. Two of ee/workspace's own runtime
dependencies -- mailparser and v9u-smb2 -- are NOT among apps/api's own
dependencies, so neither production Dockerfile's deployed node_modules
provisions them (docker/Dockerfile.api's `pnpm deploy --prod`; apps/api
/Dockerfile's direct node_modules copy both provision only apps/api's own
dependency closure). They were already ending up correctly bundled into
dist/index.cjs, but only as a SIDE EFFECT of tsup's default "external =
apps/api's own declared dependencies" heuristic -- apps/api just doesn't
happen to depend on either package itself. That's an accident, not a
decision: if apps/api ever gained a same-named dependency for an unrelated
reason, the package would silently flip to externalized and the next boot
that hit the code path would crash with "Cannot find module" -- with
nothing in CI to catch it, since the existing Dockerfile-manifest guard
checks COPY completeness, not what tsup actually chooses to bundle.

Make the inlining an explicit, reviewed decision: add 'mailparser' and
'v9u-smb2' to tsup.config.ts's noExternal directly, and add a guard test
(extensionBundledDependencies.test.ts) that fails if a future ee/workspace
runtime dependency lands in neither bucket (an apps/api dependency, or an
explicit noExternal entry) -- or if a noExternal entry goes stale.

Verified empirically against fresh builds of both production images
(docker/Dockerfile.api, apps/api/Dockerfile): require.resolve('mailparser'
| 'v9u-smb2') correctly fails externally (proving both are bundled inline,
not dangling), and node dist/index.cjs boots all the way past every static
top-level import in the whole module graph -- including the full
ee/workspace chain (content/extract.ts's mailparser usage, byteReader.ts's
v9u-smb2 dynamic import) -- to index.ts's line-1002 runtime invocation of
createTerminalWsRoutes, failing only on unrelated remote-WS config
validation, never on module resolution. Confirmed unchanged before/after
this commit (both already passed; this commit makes it durable rather than
fixing an active break).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wires the ee/workspace (@breeze/ext-workspace) test suite into breeze CI:

- test-api job: unit tests + typecheck, next to the extension-sdk steps.
- integration-test job (shard 1 only): boots apps/api once against the
  shard's already core-migrated :5433 DB to apply ee/workspace's built-in
  extension migrations (loadBuiltinExtensions runs only at real server
  boot, not inside the vitest integration globalSetup), then runs the
  workspace integration suite against that DB.
- vitest.integration.config.ts now loads the repo-root .env.test the same
  way apps/api's own integration config does (../../.env.test), instead of
  the standalone repo's now-stale relative path.
- ee/workspace gets its own eslint.config.js + "lint" script (mirroring
  packages/shared's flat config) — `pnpm lint` is `turbo run lint`, which
  only runs a package's `lint` script if it defines one, and ee/workspace
  didn't, so it was silently skipped rather than passing clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rd test

Strict typecheck (NODE_OPTIONS=--max-old-space-size=8192 pnpm exec tsc
--noEmit --project apps/api/tsconfig.json) failed with TS2339 on
tsupConfig.noExternal: defineConfig's return type is `Options | Options[] |
((overrideOptions: Options) => MaybePromise<Options | Options[]>)`, and
Options[] has no `noExternal` property. vitest's transpile-only run never
caught this.

Added resolveTsupOptions(), an explicit type guard that throws (rather than
silently misbehaving) if apps/api/tsup.config.ts ever switches to the
array or factory-function form of defineConfig, and returns the narrowed
Options type otherwise. No ts-ignore/any-casts -- apps/api/tsup.config.ts
genuinely only uses the single-object form today, so this is real
narrowing, not a suppression.

Assertion logic is unchanged (still reads options.noExternal the same way,
just through a properly-typed accessor), so the guard's actual behavior
(including firing on a reverted noExternal, proven in the prior round) is
unaffected -- confirmed by re-running both the strict tsc gate (clean) and
the two guard test files (29/29 passing) plus the full src/config/ +
src/extensions/ suite (700/700).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review finding: under this step's default `bash -e`, an unguarded `kill
"$BOOT_PID"` on the success path (unlike both failure branches, which
already guard with `|| true`) would abort the step with a bare "process
not found" and no log content if the API process exited on its own
between logging the built-in load line and the kill. That makes an
otherwise-correct migration load look like an unexplained CI failure.
Guard it the same way as the failure branches, and dump
/tmp/ee-workspace-boot.log in that case so it stays diagnosable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e built-in path

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… exclusion, dev-compose ee mounts

- CI: boot the API in its own process group (setsid) for the built-in
  extension migration step, kill the whole group instead of just the
  pnpm-exec leader (SIGTERM to the leader alone isn't guaranteed to reach
  the tsx/node descendants, risking a leaked booted API into the next
  step against the same DB shard), bound the final graceful-shutdown wait
  to 30s with a force-kill fallback instead of an unbounded wait, and
  raise the boot timeout from 90s to 180s for cold tsx compiles on
  2-core runners. Log-dump-on-failure preserved on every abort path.

- ee/workspace/manifest.json: exclude workspace_crawl_runs.cursor from
  the tenant export. It's an agent-written opaque resume token (up to
  64KB of device filesystem crawl state); the sibling
  workspace_sources.crawl_cursor is already excluded for the same
  reason, and the extension's own admin projection already hides it as
  internal.

- docker-compose.dev.yml + docker-compose.override.yml.dev: mount
  ee/workspace/src and ee/workspace/migrations into the api dev service
  so in-container tsx hot-reload actually picks up ee/workspace edits
  instead of silently running stale code. Mounts src+migrations only
  (matching the apps/api/src :cached / apps/api/migrations :ro
  convention already used in both files) so the image's
  ee/workspace/node_modules symlink is never shadowed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 11, 2026

Copy link
Copy Markdown

Deploying breeze with  Cloudflare Pages  Cloudflare Pages

Latest commit: 588e8a4
Status: ✅  Deploy successful!
Preview URL: https://8b5a7bbd.breeze-9te.pages.dev
Branch Preview URL: https://toddhebebrand-workspace-ee-m.breeze-9te.pages.dev

View logs

Todd Hebebrand and others added 4 commits August 11, 2026 16:56
…grations require it

The Workspace built-in now runs ee/workspace/migrations/*.sql (including
CREATE EXTENSION vector) at every API boot. docker-compose.yml's postgres
image comes entirely from POSTGRES_IMAGE_REF (no in-file default), and
both .env.example and the Smoke Test job's inline .env pointed it at
stock postgres:16-alpine, so breeze-api failed to start with
"extension \"vector\" is not available". Switch both to
pgvector/pgvector:pg16, matching the pg16 major docker-compose.test.yml
and docker-compose.dev.yml already use. Also documents the requirement
in the production deploy docs table.

Co-Authored-By: Claude Code <noreply@anthropic.com>
ci-smoke-binary-source-local.yml and ci-smoke-binary-source-github.yml
boot @breeze/api directly against a raw postgres:16-alpine service
container. The Workspace built-in now runs ee/workspace/migrations/*.sql
(CREATE EXTENSION vector) at API boot, so both jobs would fail on their
next trigger post-merge. Switch to pgvector/pgvector:pg16, matching the
swap already made for the production compose default and Smoke Test job.

Co-Authored-By: Claude Code <noreply@anthropic.com>
…contextual manifest-load error

resolveBuiltinRoot only tried a dev source-file walk-up and process.cwd(),
so ci-smoke-binary-source-{local,github}.yml (bundled API, plain repo
checkout, cwd=apps/api) found neither: the walk-up lands outside the repo
from apps/api/dist, and cwd/ee/workspace only exists in the Docker image.
loadBuiltinManifest died with a bare ENOENT and no context.

Add a bounded, ordered candidate list (source walk-up, cwd, up to 3 cwd
ancestors) checked against manifest.json itself so an empty mount-point
dir can't shadow the real root. Wrap the manifest read so a miss names
the package, every candidate tried, and the Dockerfile/runtime contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both smoke jobs boot the bundled API with NODE_ENV=production from a
plain checkout. Now that resolveBuiltinRoot finds ee/workspace's
manifest there, the built-in loader's next (deliberate) production
guard fires: it fails hard when ee/workspace/dist/web is missing,
since a real from-source deploy must build it. Add the same
`pnpm --filter @breeze/ext-workspace build:web` step
docker/Dockerfile.api's builder stage runs right after the API build,
to both workflows, right after their existing "Build API" step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

CI status: everything is green (Smoke Test, Test API, all four integration shards incl. the new workspace boot-step + integration suite, Type Check, and the CI Success aggregator) except BINARY_SOURCE=github download path, which is failing identically on main (last four main runs today are red on unrelated selfhost-signing commits — see the workflow history). On this branch that job now gets further than main's failure mode: the API boots cleanly through the built-in workspace load; the 404 is the agent-binary GitHub release manifest lookup, untouched by this PR.

🤖 Generated with Claude Code

Todd Hebebrand and others added 3 commits August 11, 2026 19:58
…fault-off

Built-ins now carry a per-extension enable flag (BuiltinExtension.enableEnvVar);
workspace's is BREEZE_WORKSPACE_ENABLED, default OFF. A disabled built-in runs
no migrations (so plain-postgres deployments boot with no pgvector requirement),
stages nothing, and requires no web dist — it emits one structured skip line and
publishes its tenancy declaration only when its tables already exist from an
earlier enabled boot (existence-probed; publishing absent tables would break
cascade/export SQL, omitting present ones would abort boot at the unaccounted-
tables sweeps). Enabled-path pgvector failures now name the requirement and both
remedies. Dev composes and CI shard-1 opt in; the smoke jobs deliberately
exercise the default-off boot; verified live: plain-pg flag-off, pgvector
flag-on, and flag-off-with-existing-tables boots.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… probe, CI and docs hardening

CRITICAL — a switched-off built-in published its manifest's WHOLE tenancy
declaration whenever ANY of its tables existed. A partial schema is a reachable
state: enabling the built-in on a database that cannot satisfy its migrations
aborts boot mid-sequence (workspace on stock postgres applies three files and
dies on the fourth's CREATE EXTENSION vector), and unsetting the flag then
published thirteen tables when five existed — pointing org-cascade, device-move
and tenant-export SQL at eight relations that were never created.

The existence probe now returns the present SUBSET (existingDeclaredTables,
replacing the boolean anyDeclaredTableExists) and the disabled path publishes a
declaration filtered to it, warning (builtin_extension_partial_schema) with the
present/missing lists when the set is partial. Filtering is safe in both
directions: the unaccounted-table sweeps only examine tables that EXIST, so a
filtered declaration still accounts for all of them, while cascade/export never
name a missing relation. The published declaration also gets the boot-time RLS
tripwire the enabled path applies — orphaned tables still hold tenant rows.

Also:
- probe SQL reads pg_class, not information_schema (whose views are filtered by
  the connecting role's privileges, so an unprivileged table read as ABSENT);
  keeps the binding-safe IN ${sql([...])} list helper
- probe failures rethrow with disabled-path context and { cause }
- sql.end() failures no longer replace the in-flight error (probe + loader)
- the disabled log reports observedValue and a value-strict reason, so
  BREEZE_WORKSPACE_ENABLED=1 stops reading as "not enabled"
- missing dist/web is decided by an explicit existence check instead of catching
  ENOENT around everything, which swallowed real walk/registration faults
- a corrupt built-in manifest names its path and byte length (import-time
  failure; the bare JSON error was the whole diagnostic)
- CI boot step: greps the fatal [CRITICAL] API startup failed sentinel rather
  than a bare CRITICAL a non-fatal worker line can race into; the already-exited
  branch fails on that sentinel and warns otherwise; the process group records
  its own pgid and a survivor after SIGTERM+SIGKILL fails the step rather than
  poisoning the next step's DB shard
- dev composes interpolate the enable flag and mount ee/workspace/manifest.json
  (migrations were mounted, the manifest baked — they drifted silently)
- .env.example, deploy/.env.example and the production deploy docs carry the
  musl/glibc collation caveat for swapping Postgres images over an existing
  volume; deploy/docker-compose.prod.yml wires the flag it now documents

New builtinTableProbe.integration.test.ts drives the real probe SQL against
Postgres — the one query on this path the port-injected unit tests cannot reach,
and the one whose first version passed every unit test while failing live.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bies

First live run tripped the fail-closed assert on dead-but-unreaped
node/esbuild zombies (kill -0 succeeds on zombies; the step's shell cannot
reap grandchildren), and the grace poll watched only the setsid leader,
which exits before its descendants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

Post-review hardening (comprehensive multi-agent review of 3757b9ee2..HEAD)

Four specialized review passes (code, silent-failures, test coverage, comment accuracy) ran over the commits that landed after the original whole-branch review. Outcome, all fixed in 49c60172f + 588e8a459 and verified by a scoped re-review plus this green CI run:

Critical (fixed): a disabled built-in with a partially migrated schema (e.g. enable on plain Postgres → dies at the pgvector migration → operator follows the error's advice and disables) published its FULL tenancy declaration — org-deletion cascades and tenant export would then hit nonexistent tables, aborting GDPR erasure mid-delete. The disabled path now probes which tables exist and publishes a declaration filtered to them, logging a loud builtin_extension_partial_schema warning with present/missing lists and remedies. Reproduced live before and after the fix.

Also fixed: RLS tripwire now runs on the disabled path over the published declaration (previously, disabling the flag silently retired the only RLS check those tables get); CI boot-step CRITICAL greps narrowed to the fatal sentinel; the "API exited on its own" branch now fails on a real startup failure; PGID handling fails closed — and its first live run caught that kill -0 counts zombies, so the shutdown poll now watches the group and treats zombie-only groups as dead; probe/manifest failures carry actionable context; typo'd flag values (True, 1) are named in the skip log; web-asset error handling narrowed to only soft-fail on "never built"; dev composes bind-mount the manifest and parameterize the flag; deploy/ production path documents the flag and receives it; musl→glibc collation/REINDEX upgrade caveat added to .env.example and the deploy docs; new DB-backed integration test pins the real table-probe SQL on every PR.

Deliberate follow-ups (not in this PR): lazy BUILTINS loading so a disabled deployment tolerates a missing/corrupt manifest entirely; a two-builtin mid-pipeline-failure test; installed_extensions lifecycle polish (recordActive-after-activate, drop redundant first-boot setEnabled); routeNamespace-level collision gate across all three delivery paths; kill-signal ESRCH/EPERM discrimination in the CI step.

Release-note item: the disabled path now runs the RLS tripwire, so a deployment with orphaned built-in tables lacking RLS will refuse to boot where it previously booted silently under-protected — intentional fail-closed behavior.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant