Skip to content

LIM-2424: L0 multi-workspace (Mode L) + hosted-mode draft - #3

Closed
liminalshruti wants to merge 13 commits into
mainfrom
build-a/L0
Closed

liminalshruti wants to merge 13 commits into
mainfrom
build-a/L0

Conversation

@liminalshruti

@liminalshruti liminalshruti commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

DRAFT. Do not merge, and do not deploy. Stacked on PR #1 (build-a/integration). Sean reviews the diffs, and Shruti approves any merge. No hosting, DNS or accounts until Sean approves on LIM-2424.

L0, multi-workspace, built from Sean's multi-workspace tech spec rev 2: Mode L (loopback) per the §12 "In the cut" column, plus a draft of hosted mode (founder ruling "go C", LIM-2424 e6ac73ed). Each independent tester gets their own isolated workspace.

Deviation from the spec: additive in the cut

The spec says POST /api/workspaces replaces /api/scenario and /new replaces /setup. Here, both are kept: every PR #1 gate and founder ruling runs on /setup. /new and /setup share one form (components/ScenarioForm.tsx), and /setup's DOM stays byte-identical. Retiring /setup is Sean's and Shruti's call.

Commits (on PR #1 head e0d291c)

  • core: lib/config.ts, workspace.ts, locks.ts, errors.ts, startup.ts; instrumentation.ts (pid guard); scripts/serve.sh. The store gets workspace-scoped state, and auth gets a custom token {actor,sid,ws,ep,exp}. The Northstar token and cookie are unchanged: a token without ws takes the old path exactly.
  • routes: a capped body reader and a rate limit. session, state, reset, timer and proposals now use requireWsSession. New routes: /api/workspaces (create), /config and /[id]/login-info. emitAuthority fires only for the Northstar workspace and non-custom scenarios. (This commit's message still mentions proxy.ts and S12; those parts were absorbed by PR LIM-2424: Build A minimum slice #1's proxy.ts during the rebase.)
  • UI: app/page.tsx → components/Home.tsx; /w/[slug]; a Session-ended flow; per-workspace "Restart this scenario". Includes PR LIM-2424: Build A minimum slice #1's width and landing fixes.
  • tenancy: scripts/test-tenancy.mjs, run by test-all.
  • HOSTED=1 hardening (draft):
    • Startup refuses: weak or missing ALIGN_HOSTS/secret/data dir/signup code, default passcodes, an AI key present, capture on.
    • The ALIGN_HOSTS allowlist replaces loopback in proxy.ts and requireJson. It's written once, in lib/config.ts hostAllowed().
    • https only. Capture and the Northstar no-session routes return 404.
    • The cookie is __Host-, Secure, HttpOnly, SameSite=Strict.
    • With HOSTED unset, the localhost behaviour is byte-identical.
  • Fly deploy draft, not applied: Dockerfile, .dockerignore, fly.toml (one machine, volume /data), docs/HOSTING.md. Docker build not verified, because the daemon wasn't running. Retention (Q4) is open for Shruti.
  • fixture: scripts/fixtures/clinical-screening-window.json. Fictional "Halverson Bio" (Clinical Development / Clinical Operations), for the first async tester.
  • /new: create a workspace from the shared form, with a passcodes-shown-once card.

Gates (on 7d2e853)

  • test-all passes every step: Northstar snapshot identical, test-flow identical, test-scenario 42/42, mcp, capture, mapper, test-tenancy 73/73. Tenancy includes the hosted phase H1–H7 and H2: an allowlisted Host gets 200, any other host gets 421.
  • Northstar UI 72/72 identical.
  • Gate C with the landing step: 34/34. Width: 140/140.
  • /new end to end: 27/27 with open create, 28/28 with a signup code. A full instance shows the "full" message.

For Sean's review

  • The lib/auth.ts changes stay within spec §5.
  • ALIGN_CREATE_PER_HOUR (default 5) is new, for tests only. Don't raise it in hosted mode without Sean.
  • Outside the cut: DELETE workspace, rotate and export. A8 simulates delete on disk.

Linear: LIM-2424, LIM-2437


Update: main merged, interview-based fixtures, record changes (Wed 10-21 evening)

  • 4ac86a1: merge of main (B1, e9ef581). No force-push. 4 conflicts were resolved; Sean: please review the lib/store.ts resolution (both sections kept; resetState's consent filter is B1's, verbatim). Consent stays Northstar / machine-level only.
  • 69e871e: fixtures (founder ruling, from the PG interviews):
    • clinical-screening-window.json (Halverson Bio) is a negative control: an operational change to the screening window, 4 → 5 weeks, raised by sites through site partners and approved by Global Development / a VP of Clinical Research. No protocol amendment, ethics step or safety approval. Rollout by investigator letter. One sentence is labelled "Fictional assumption". Role titles only, no person names.
    • New partner-program.json (MercuryCX): Partnerships asks Revenue Operations. Legal's and Product's limits are written into the plan as requirements that aren't settled yet. The scenario is built so the natural move is a conditional counter.
  • 8d63d8a: test-all runs test-scenario against the default fixture and every scripts/fixtures/*.json.
  • 1d30dc7: record changes, custom only. ⚠️ DATA FORMAT CHANGE, needs Sean's approval:
    • The decision record says: "This records what the two teams agreed. It is not approval to launch."
    • A Conditions section shows the last counter's conditions, or its rationale when there are none.
    • The countered banner trims leading punctuation and also shows the conditions.
    • New optional CounterVersion.conditions?: string (≤1000 characters, appended last, custom only). Over the cap or non-text → 400. In Northstar the field is ignored, so the counter shape stays byte-identical, which the snapshot confirms. The owner's form has a "Conditions (optional)" field.
    • Not yet exposed in the MCP decision record.

Gates on 1d30dc7:

  • test-all passes every step: test-scenario 45/45 on each of the 3 fixtures, snapshot identical, test-flow identical, mcp, capture, mapper, tenancy 74/74.
  • test-b1 113/113. Northstar UI 72/72. Gate C 34/34. Width 140/140.
  • MercuryCX conditional run: both records show the not-launch line and the Conditions.

Multi-workspace spec rev 2, section 12 "in the cut", L0a. Additive in the
cut: POST /api/scenario, /setup and reset-to-Northstar are unchanged; custom
workspaces are added alongside (deviation from rev 2's "replaces", ruled by
Mini S0 for PR 3).

- lib/workspace.ts: CustomWsId/WsId, parseCustomWs, atomic create
  (.tmp-<id> then rename) under one creation lock with the workspace cap,
  locked delete with the strict-child check, 80-bit passcodes hashed with
  async scrypt, auth.json read sync with an mtime cache, _events.jsonl.
- lib/store.ts: readState(ws) (Northstar unchanged, including the reseed),
  mutateWs and replaceWorkspaceState. Custom reads never fall back (404 /
  500, nothing written), writes never create directories, 1 MB cap
  excluding audit, audit trimmed to 1000 rows into audit-archive.jsonl.
- lib/auth.ts (spec section 5 only): custom token {actor,sid,ws,ep,exp};
  Northstar token, cookie and passcodes unchanged. requireSession returns
  403 for custom tokens; requireWsSession returns a ctx whose mutate/read
  are bound to its workspace; a signed custom token that is expired,
  revoked or for a deleted workspace is 401 Session ended.
- lib/config.ts, instrumentation.ts, lib/startup.ts: env-only config,
  ALIGN_PUBLIC=1 refused, data/.pid single-server guard.
- scripts/serve.sh: next build && next start on 127.0.0.1 with open create.
- lib/errors.ts and lib/locks.ts: HttpError moved (still exported from
  store) and the per-key lock, so workspace.ts and store.ts don't cycle.

No route uses the new code yet. test-all passes every step (snapshot,
test-flow, test-scenario 37/37, mcp, capture, mapper); Northstar UI 72/72.
Multi-workspace spec rev 2, section 12 "in the cut", L0b plus the L2 create
routes. Additive in the cut: /api/scenario, /setup and reset-to-Northstar are
unchanged.

- session: login with {workspace} is JSON-only, uses async scrypt, returns
  one uniform 401, and has a soft per-workspace backoff (429). GET/POST add
  `workspace` and the team lead's name for custom sessions only.
- state: anonymous is still Northstar; a custom session reads its own
  workspace (no evidence ingest, prep exclusion or registry refresh); an
  ended custom session is 401, never the Northstar view.
- proposals (5), timer, reset: requireWsSession + ctx.mutate. Custom
  bodies are capped at 64 KB and per field (300, text 4000); Northstar
  bodies parse exactly as before. emitAuthority only for Northstar.
  Custom reset is leadership-only and rebuilds from scenario.json.
- POST /api/workspaces (201, no-store, passcodes shown once; creation only
  with ALIGN_ALLOW_OPEN_CREATE=1 or a matching signup code; cap under the
  creation lock; global bucket), GET /api/workspaces/config, GET
  /api/workspaces/[id]/login-info (company and team names only).
- proxy.ts: 421 for a Host outside ALIGN_ALLOWED_HOSTS (default loopback).
- lib/workspace-setup.ts reuses lib/scenario.ts's schema and builder
  without editing it (scenario.ts is pending the optional-fields change).
- test-scenario S12 now accepts 421 (proxy) or 403 (requireJson).
- Not in the spec: ALIGN_CREATE_PER_HOUR overrides the 5/hour bucket so
  the tenancy tests can create more than five workspaces.

test-all passes every step (test-scenario 38/38); Northstar UI 72/72.
Multi-workspace spec rev 2, section 8 (L4 WP8, WP1 Restart), in the cut.
Additive in the cut: / and /setup are unchanged.

- app/page.tsx moved to components/Home.tsx unchanged apart from a
  `workspace` prop; app/page.tsx renders <Home />, and the new
  app/w/[slug]/page.tsx renders <Home workspace={slug} /> in a Suspense
  boundary (cacheComponents).
- useAlign(workspace): Me.workspace; a /w page doesn't poll /api/state
  before sign-in; a session for another workspace (or a custom session on
  /) counts as signed out; a 401 "Your session ended" from any call
  returns to sign-in with that message.
- /w sign-in uses login-info (company and team names, role blurbs, no
  person names) and posts the workspace with the passcode. Sign-in screens
  link to the other workspace this browser is signed in to (server id,
  regex-checked before it goes into the href).
- Leadership in a workspace gets "Restart this scenario" (confirm twice),
  which rebuilds from the saved setup input. The /setup scenario keeps
  "Back to the Northstar demo".

test-all passes every step; Northstar UI 72/72. Browser check of /w: 7/7
(login names only, no pre-login poll, lead name and Restart after sign-in,
no Northstar text, / shows the link, unknown id message).
Multi-workspace spec rev 2, section 11, in the cut (minus 15's limiter cap
and 18). scripts/test-tenancy.mjs starts its own dev servers on the port,
one per config phase (creation closed; signup code with cap 3; open
create), with custom workspaces in a temp ALIGN_DATA_DIR. 56 checks:

- A: marker isolation; workspace in body/query/header ignored;
  login-info names only; tampered, swapped-MAC, expired and epoch-bumped
  tokens; uniform 401 for bad workspace values with nothing written;
  Northstar-only routes 403 for custom tokens; custom flow leaves Northstar
  byte-identical and the reverse; 20+20 parallel writes; deleted workspace
  (rename aside then remove, since DELETE is outside the cut) fails its
  tokens and is not recreated; corrupt state 500 with the file unchanged;
  no plaintext passcodes, 0600 files, never served; a captured Northstar
  session does not reach a new workspace; static import check.
- B: closed, wrong code, cap race (exactly one of two), 415, foreign
  Origin, 413, __proto__, forged Host 421; login 415; Northstar login
  unlimited; custom backoff and recovery; body and field caps; a custom
  accept emits no authority event.
- C: the Northstar API + MCP snapshot is identical with a workspace on
  disk. Also Restart (leadership only, audit kept).

test-all runs it last, after stopping its own server: every step passes.
Founder ruling 10-21 "go C": one hosted server, a workspace per tester.
Without HOSTED, behaviour is unchanged (test-all, Northstar UI 72/72).

- lib/config.ts: HOSTED=1 refuses to start unless ALIGN_HOSTS (no
  loopback), ALIGN_SECRET (32+), ALIGN_DATA_DIR (absolute, outside the
  app), ALIGN_SIGNUP_CODE (12+) and three non-default Northstar passcodes
  (12+) are set, and AI_GATEWAY_API_KEY, ALIGN_CAPTURE and
  ALIGN_ALLOW_OPEN_CREATE are not. Open create is ignored when hosted.
  ALIGN_PUBLIC=1 is still refused.
- proxy.ts + requireJson: the ALIGN_HOSTS allowlist replaces the loopback
  rule only when hosted (421 / 403). X-Forwarded-Proto is trusted only
  when hosted: anything but https gets 403. Capture (/api/sessions/events)
  is 404, and Northstar's no-session routes are 404 unless
  ALIGN_NORTHSTAR_PUBLIC=1.
- state: anonymous GET /api/state is 404 when hosted.
- auth + session: sessionCookie() builds Set-Cookie. Mode L is the same
  string as before; hosted uses __Host-align_session with Secure, and only
  that name is read.
- test-tenancy: new hosted phase (H1-H7): refuses to start with default
  passcodes / no ALIGN_HOSTS / an AI key; loopback and foreign Host 421;
  plain http 403; create needs the code; foreign Origin 403; cookie is
  __Host- + Secure + HttpOnly + SameSite=Strict; the default Northstar
  passcode fails; an unprefixed cookie is not a session; capture 404;
  Northstar no-session routes 404; two workspaces can't see each other.
  71/71. Also checked by hand under next start (production build).
Dockerfile, .dockerignore, fly.toml and docs/HOSTING.md for HOSTED=1 on one
Fly machine with a persistent volume at /data. Nothing is created: no app,
volume, DNS or account until Sean approves on LIM-2424.

Fly because the spec needs exactly one Node process with a persistent disk
(in-process locks, data/.pid): a Fly volume attaches to one machine, the
edge terminates TLS and sets X-Forwarded-Proto, and secrets go through fly
secrets. Serverless is ruled out as written. HOSTING.md covers the env
vars, startup refusals, volume layout, creating tester workspaces, the
host-admin delete path, deploy steps for later, and that
ALIGN_CREATE_PER_HOUR must not be raised in hosted mode without Sean.
The Dockerfile was not built locally (Docker daemon not running); the
production build and HOSTED=1 behaviour were checked with next start.
…st async tester

scripts/fixtures/clinical-screening-window.json: the L2 setup input (with
confirm, like data/scenarios/example-custom.json; drop confirm for POST
/api/workspaces). All fictional: "Halverson Bio", a generic Phase 2 study,
invented lead names, no real company, trial, drug or person and nothing
patent-sensitive. Clinical Development asks Clinical Operations to move from
a 4-week to a 5-week screening window; Operations is measured today on
"Screening completed within 4 weeks at every active site". Minimal form plus
optional fields: a second company measure, an asking-team measure, a second
owning measure, plan title, 2 work items per team.

Validated on the L0 server: POST /api/scenario as leadership 200, POST
/api/workspaces 201, the full custom flow (23 views) with --forbid on the
standard list plus Fernway|Acme|Brightline|Data Platform|SOC 2: clean.
components/ScenarioForm.tsx holds the two-team form moved out of
/setup unchanged (types, defaults, body, issue mapping, fields). /setup
renders byte-identically in every state.

app/new/page.tsx: no sign-in needed. Shows a signup code field when
/api/workspaces/config requires one and a full message at the cap, then
POSTs the form (no confirm) to /api/workspaces. The success card shows
the /w/<id> link and the three passcodes once, each with a copy button,
says to save them now, to hand each to a different person, and that a
browser profile holds one workspace session at a time.
After the rebase onto integration's proxy.ts (7be40ce): Mode L keeps
integration's loopback rule and text/plain 421 byte for byte; with HOSTED=1
the ALIGN_HOSTS allowlist replaces it (lib/config.ts hostAllowed, used by
proxy.ts and requireJson). ALIGN_ALLOWED_HOSTS is dropped: loopback by
default, ALIGN_HOSTS only when hosted. LOCAL_HOSTS stays on the agent-only
routes (directives*, sessions/events), which hosted mode closes with 404.

test-tenancy H2 adds: GET / and POST /api/session with the allowlisted Host
-> 200; with a non-allowlisted Host -> 421 and no Set-Cookie. 73/73.
@liminalshruti
liminalshruti changed the base branch from build-a/integration to main October 21, 2026 23:12

@allsmog allsmog left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sean: in-depth review of PR #3 (L0 multi-workspace + hosted draft) @ 7d2e853

Read-only against e0d291c, nothing run. Four parallel reviews (tenancy core and auth, routes, hosted and deploy, UI and tests), with the main findings re-checked by me against the code. "Verified" means traced through the code path. "Suspected" depends on runtime behaviour nobody executed.

Verdict. The multi-workspace core is sound, with no cross-tenant path found. It can't deploy hosted yet, and it needs a short list of fixes before it merges even for local mode.

Checks that passed

  • Isolation. A workspace token can't read or write another workspace or Northstar on any traced path. requireWsSession binds every read and write to the token's ws, and no workspace-aware route imports mutate or readState or takes a workspace from the body.
  • Rev-1 review H1 to H3 are closed in the code. The CustomWsId split holds, parseCustomWs rejects northstar, and delete does a strict-child check under the lock. Creation access comes from config, not headers. Custom reads never fall back to the Northstar fixture or create directories. Create is atomic.
  • Tokens. The HMAC covers {actor,sid,ws,ep,exp} and is checked in constant time. exp and ep are enforced. An ended session gets 401 Session ended and never Northstar.
  • Passcodes. 80 bits, async scrypt (N=16384), timingSafeEqual, shown once, no-store, never logged. All login failures give one uniform 401.
  • Create. requireJson runs first, the 64 KB cap is enforced on the stream, reserved keys are rejected, and the zod schema is strict. The cap is checked under the creation lock (two racing creates at N-1 give exactly one 201). The signup-code compare is timing-safe.
  • Northstar (Mode L). State path, .secret, env passcodes, token, cookie string and error strings are unchanged, with one edge case below. The Home.tsx and ScenarioForm.tsx moves are pure extractions plus additive code. /setup's DOM is unchanged.
  • XSS. No raw HTML. The only hrefs are built from server ids.
  • Hosted pieces. The exact-match Host allowlist (trailing dot, empty and loopback all get 421) is shared with requireJson. Only an exact X-Forwarded-Proto: https passes. The __Host- Secure cookie is right. fly.toml has no secrets, one machine, auto-stop off, /data mounted.

Blocks any hosted deploy

  1. High: Northstar is still live when HOSTED=1. Verified. / renders the Northstar app, and login() has no hosted check. With a Northstar session, /api/scenario, /api/reset, /api/state (which runs evidence ingest and the mapper refresh) and the rest all work. Test H2 asserts that Northstar sign-in returns 200. This is go-C condition 1. Fix:

    • when hosted, Northstar login and readState("northstar") return 404;
    • the proxy 404s Northstar-only routes and redirects / and /setup to /new;
    • delete northstarPublic;
    • refuse ALIGN_NORTHSTAR_PUBLIC at startup;
    • flip H2.
  2. High: the AI key can ship in the image. Verified. .dockerignore's .env patterns match only the repo root. lib/mapper.ts:31 on this branch reads sparta/walkthrough-lab/projects/team-inbox/.env, and fly deploy uploads the local tree, gitignored files included. That gets around the env-only startup check. Fix:

    • an allowlist .dockerignore (*, then !app !components !lib !public ...) plus **/.env*;
    • apiKey() returns null when hosted (the B1 rebase drops the .env read as well).
  3. High: the startup refusal is the only enforcement. Whether next start exits when register() throws is UNKNOWN. H1 runs under next dev, counts "no 200 in 90 s" as a refusal, and covers 3 of the 10 refusals. Fix:

    • memoize hostedProblems() in the proxy and return 503 while it is non-empty;
    • test every refusal against the real image (docker run).
  4. Medium: a login burst stalls every tenant. Two reviewers verified it independently. checkLoginBackoff runs before the about 50 ms scrypt, and a failure is recorded only afterwards, so N parallel logins all pass. 16 MB scrypt jobs fill the 4-thread libuv pool that fs.promises shares, so state reads for all workspaces queue for seconds. Anyone with a /w/<id> link can do this. Fix:

    • a global semaphore on verifyPasscode (1 to 2 in flight, 429 beyond a short queue);
    • count the attempt before awaiting.
  5. Medium: the backoff works as a lockout and can be reset. Verified (lib/ratelimit.ts, app/api/session/route.ts:28). It's keyed on the raw workspace string only. One wrong try about every 10 s keeps all three roles at 429, correct passcode included. 10,000 junk ids evict the real entry. Fix:

    • record failures only for an id that parses and exists;
    • key on ws plus role, and in hosted mode also Fly-Client-IP (wire up ALIGN_TRUST_PROXY);
    • delay instead of refusing.
  6. Medium: creation can be exhausted.

    • A shared signup code plus a global, in-memory 5-per-hour bucket (reset by a restart), a cap of 25 and no delete path lets one code holder fill the server in about 5 hours.
    • Signup-code guesses are unlimited.
    • The bucket token is spent before the cap check.
    • ALIGN_CREATE_PER_HOUR isn't refused when hosted.

    Fix: single-use invite codes (or one per interview), a failed-code bucket, the token taken inside the lock after the cap check, and that variable refused when hosted.

  7. Medium: the proxy denylist is the only gate for capture and Northstar's no-session routes. Route handlers never check hosted mode, so a future route is open by default. H6 skips /api/evidence/*, /api/mcp, /api/directives/stream, /api/directives/ack and /api/mappings/*/confirm. Fix: a hosted guard in handle() or per route, plus a refuseCustom guard on the Northstar no-session routes, and H6 for every path.

  8. Medium: no delete command (condition 4). deleteWorkspace has no caller, and the manual fly ssh steps skip the lock and the event row. Fly volume snapshots (default retention a few days, suspected) mean rm doesn't purge. Fix: scripts/ws-admin.mjs list | delete <id> | delete --all, and set or state the snapshot retention to match the Nov 6 date.

  9. Medium: no notice on /new (condition 3).

  10. Medium: the image runs as root and ships the whole repo. That includes dev deps, sparta/**, agent/**, scripts/**, data/fixture.json and the test fixtures. Fix: output: "standalone" or npm ci --omit=dev, copy only the build output, and USER node with /data chowned. With item 1, Northstar data doesn't ship at all.

  11. Medium: every gate runs on next dev. Fly ships a production build. Fix: run the hosted phase against next build && next start (ideally the image).

  12. Low:

    • the pid file sits in cwd/data on the ephemeral layer, not the volume;
    • npx runs as PID 1;
    • there's no health check (a default one would get 421 or 403; send Host and X-Forwarded-Proto);
    • the first deploy needs --ha=false;
    • files should be 0600 and directories 0700 (state.json holds private constraints at 0644);
    • when hosted, log only route, status and error name, not console.error(e);
    • suspected: peers on the Fly private network can reach 0.0.0.0:8080 with forged headers, so require Fly-Client-IP.
  13. Condition 6 (live smoke test) is not done, as expected. HOSTING.md also needs fixes:

    • line 25 says Northstar sign-in needs non-default passcodes, which contradicts item 1;
    • line 64 says /new is "being built by S1", but it's in this PR;
    • line 75 says the manual delete is "the same procedure the app uses", but it skips the lock and event row;
    • line 9 says the pid file protects against a second server, but it isn't on the volume;
    • lines 51 and 52 describe safeguards the code doesn't enforce;
    • line 58 leaves out volume snapshots.

Fix before merge (also affects local mode)
14. Medium: CSRF on custom write routes. Verified (lib/http.ts:107-111). bodyFor for a custom workspace parses any content type with no Origin check, and SameSite=Strict doesn't separate localhost ports. A text/plain form from another localhost port can restart a workspace or create a proposal for a signed-in browser. Fix: requireJson(req) inside bodyFor when ws !== "northstar".
15. Medium: one browser can show workspace data at /. Verified (components/useAlign.ts:56-68). The cookie is per origin, so signing in to /w/A in tab 2 makes tab 1's next poll at / render A's names, and tab 1's buttons then act on A. Fix: /api/state returns workspace and actor; on mismatch, call loadMe() and drop the state.
16. Medium: test-tenancy can wipe the live demo. Verified (scripts/test-tenancy.mjs:4, startServer). The header says it refuses a busy port, but it only refuses 3100. Run from the main checkout, it deletes ./data/state.json and data/.secret before any guard. Fix: test-all's worktree and lsof guards.
17. Medium: "Loading Align…" forever. Verified (useAlign.ts:75, :96). loadMe rethrows non-ended errors uncaught, for example a corrupt workspace state or the server being down. Fix: catch them, and show a plain error with retry.
18. Medium (suspected): passcodes can be lost. After "Open the workspace", Back may or may not show the card again (cacheComponents Activity behaviour). Rotation is outside the cut, so a tester who clicked before copying is stuck. Fix: an "I've saved these" step, and open the workspace in a new tab.
19. Low:
- Northstar byte-identity: "workspace" in b throws on a primitive JSON body (app/api/session/route.ts:25). That gives 500 where the base gave 400. Guard with b && typeof b === "object".
- Audit trim before the size check: the archive is appended before the 1 MB check (lib/store.ts:113-119), so rejected writes duplicate rows without limit.
- readAuth: it swallows every error (an EMFILE becomes a false "Session ended"), and a malformed auth.json gives 500.
- Event append: if it fails after the rename, the workspace exists, its passcodes are lost, and it counts toward the cap.
- Northstar tokens: no expiry or mode binding, so a Mode L token minted with a shared secret is valid on hosted. Refuse a shared or default secret.
- UI:
- every login-info failure reads "no longer exists";
- the 560px login form overflows on phones;
- /setup for workspace leadership fails only after the whole form is filled;
- if the /new config fetch fails, the code field disappears;
- a Northstar cookie at /w/A renders nothing helpful.
- Fixture: clinical-screening-window.json isn't labelled fictional on screen, and keeps confirm, so POST /api/workspaces returns 400 for it.

Test gaps to close

  • Spec tests 9, 18 and 20 are missing.
  • A2 (status unchecked), A6 (reverse direction can pass vacuously) and A7 (statuses and counts unchecked) can pass when they shouldn't.
  • H1 covers 3 of 10 refusals. H6 skips routes.
  • There are no UI cases for the pre-login poll, mismatch, session ended or /new.
  • The 72/72 UI snapshot isn't in test-all.
  • The hosted phase should run on a production build.

Spec deviations. Accepted: /setup kept alongside /new, and the ALIGN_HOSTS name. Not accepted for hosted: no per-client limiter key, the pid file off the volume, and no delete path.

Sign-off terms.

  • Local-mode merge (stacked after PR #2): items 14 to 17 and 19's Northstar byte-identity fix are fixed, test-all plus tenancy are green on a production build, and I re-review those diffs.
  • Hosted deploy: additionally items 1 to 11, go-C conditions 1 to 5 met, and the live smoke test (condition 6) posted. The account creation and the deploy still need my separate go.
  • Timing: this is a full night of work. If it isn't in and re-reviewed by Thu 10:00 AM, use the Neo/Meet fallback for testers.

Merge of main e9ef581 (PR #2, B1) into build-a/L0 7d2e853, per Shruti's
ruling: merge, not rebase, so no force-push. The resolution is the one
rehearsed on build-a/L0-b1-dryrun (7f5d147); the resulting tree is
byte-identical to it.

- lib/store.ts: kept both the custom-workspace store (readCustom,
  writeCustom, mutateWs, replaceWorkspaceState) and B1's capture-consent
  helpers; resetState's consent filter is B1's, verbatim.
- app/page.tsx: stays the <Home/> wrapper; B1's two changes are ported into
  components/Home.tsx (sign-in labels without simulations; wait for
  state.me to match the signed-in role).
- components/useAlign.ts: useAlign(workspace) kept, with B1's
  StateView | AnonStateView typing.
- app/setup/page.tsx: B1's AnonStateView import plus the ScenarioForm import.
- scripts/test-tenancy.mjs: A11 grants capture consent (Northstar
  leadership) before the synthetic capture and reads Northstar as
  leadership; it waits for Northstar state to settle instead of for a
  mapping (external mapper off by default); A5 adds the B1 consent and
  session routes, all 403 for a custom-workspace token.

Capture consent stays Northstar / machine-level only: capture writes only
Northstar state, every consent and session route uses requireSession (403
for workspace tokens), hosted mode closes /api/sessions/* (404) and refuses
ALIGN_CAPTURE, and a workspace never holds sessions or consents.
…partner-program fixture

Halverson Bio is now the negative control as described: an operational move of
the screening window from 4 to 5 weeks, raised by sites through their site
partners, approved by a VP of Clinical Research. No protocol amendment, ethics
committee or safety approval; rollout by investigator letter and site partners;
one labelled fictional assumption. Leads are role titles, not names.

MercuryCX (fictional): Partnerships asks Revenue Operations, whose goal is
renewal and retention risk, to back a partner investment program. Legal's
data-access limit and Product's feature limit sit in the plan as unsettled
requirements, so the natural move is a conditional counter, and the plan says
team agreement is not executive launch approval. Two work items per team.
@liminalshruti

Copy link
Copy Markdown
Collaborator Author

@allsmog: this PR now includes main with B1 (e9ef581), through merge commit 4ac86a1. It was pushed normally, with no force.

Please review the lib/store.ts resolution. Both sections are kept: L0's custom-workspace storage (readCustom, writeCustom, mutateWs, replaceWorkspaceState) and B1's consent helpers. resetState's consent filter is B1's, verbatim.

Consent design: consent is Northstar / machine-level only. Every consent and session route needs a Northstar session, so a custom-workspace token gets 403 (tenancy A5). Hosted mode returns 404 for all of /api/sessions/*, and custom workspaces never hold sessions or consents (A11).

Custom workspaces also get B1-6's per-role viewFor allowlist.

Gates on 4ac86a1:

  • test-all: every step passes, including tenancy 74/74 (HOSTED=1 phase 17/17)
  • test-b1: 113/113
  • Northstar UI: 72/72
  • Gate C: 34/34

…ws the owner's conditions

DATA FORMAT CHANGE (needs Sean's approval): CounterVersion gains optional
`conditions?: string` (≤ 1000 chars), appended last, written only in custom
scenarios. POST /api/proposals/<id>/counter accepts `conditions` in custom:
trimmed, empty dropped, non-text or over 1000 -> 400. In Northstar the field is
IGNORED (not rejected), so Northstar's counter shape stays byte-identical
(snapshot gate).

Custom only:
- Decision record: under the headline, "This records what the two teams
  agreed. It is not approval to launch." Then a Conditions section: the last
  counter's conditions, else its rationale.
- Owner's ask-for-changes form: "Conditions (optional)" field.
- Countered banner: shows the conditions; the quoted added fragment drops
  leading ", ." and whitespace.
- test-scenario: F6c (over the cap or not text -> 400, proposal unchanged),
  F6 stores trimmed conditions as the last field, F13 the asking team's state
  carries them after approval. Passes on all three fixtures.
@allsmog

allsmog commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Sean: merge resolution reviewed, at 1d30dc7.

4ac86a1 (main with B1 merged into L0) is correct. All six hand-resolved files keep both sides:

  • lib/store.ts: the workspace store and B1's consent helpers are both there. The reset consent filter is B1's, verbatim. The only lines dropped from main's copy moved to errors.ts or into the workspace-aware readState.
  • /api/state: workspace sessions get B1-6's per-role viewFor and skip ingest. Northstar still runs ingest and the registry refresh.
  • Home.tsx and useAlign.ts carry B1's two sign-in changes and the AnonStateView typing.
  • lib/mapper.ts keeps both guards: custom scenarios never map, and unconsented sessions never map.
  • The machine-level consent design (workspace tokens get 403 on every consent and session route; hosted returns 404) matches what I approved on B1.

Effect on my review: B1 removed the nested Sparta .env read, so item 2 drops to Medium. The .dockerignore allowlist is still needed, so a stray .env can't reach the image.

New bug (1d30dc7), Low: the UI and flow.ts allow 1,000 characters of conditions, but capFields in lib/http.ts caps every workspace string at 300 except text and plan_text. So conditions over 300 characters return 400 in a workspace but work on /setup. Fix: add conditions to the 1,000 exception, and add a test for it.

Still open: every other item in my changes-requested review (hosted blockers 1 and 3 to 11; pre-merge 14 to 17 and the 19 byte-identity fix). I'll re-review as soon as fixes land.

@allsmog

allsmog commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Sean: I'm taking the PR #3 review items myself, starting now. My session is fixing items 1 to 19 from my review, hosted blockers included, in a PR stacked on #4 (which is stacked on this one).

Please hold the mini off those items so the two don't collide. If it already has fixes in progress, push them now and I'll build on top. I'll post the new PR link here, with gates, when the first batch is up.

allsmog added a commit that referenced this pull request Oct 22, 2026
- 1 Northstar is off when HOSTED=1: / and /setup redirect to /new,
  Northstar sign-in is 404, Northstar tokens are ignored, and the
  Northstar store refuses every read and write. ALIGN_NORTHSTAR_PUBLIC is
  gone (refused at startup).
- 2 The AI key can't ship or be used: .dockerignore is an allowlist of
  the app's own source, and apiKey() is null when hosted.
- 3 A refused start exits: `next start` logged a register() error but
  kept serving 500s (checked on the image). instrumentation now exits 1,
  and proxy.ts answers 503 if a refused process is ever kept up.
- 4 Passcode hashing runs at most 2 at a time with 8 waiting (429
  beyond), so a sign-in burst can't stall other workspaces' file reads.
- 5 Login backoff is per workspace, role and client (Fly-Client-IP when
  hosted), counted before the hash, and only for workspaces that exist.
- 6 Hosted creation needs single-use invite codes (ALIGN_SIGNUP_CODES),
  spent under the creation lock after the cap check and stored as
  hashes. Wrong codes are limited per client; the create bucket token is
  taken after the cap check; ALIGN_CREATE_PER_HOUR and the shared
  ALIGN_SIGNUP_CODE are refused when hosted.
- 7 proxy.ts is an allowlist of tester routes: every other path is 404
  when hosted, so a future route is closed by default (H6 walks app/).
- 8 scripts/ws-admin.mjs lists and deletes workspaces (one, or all with
  --yes) the way the app does, safe while serving; snapshots documented.
- 9 /new states the test server, no confidential details, and the
  deletion date (ALIGN_DELETE_BY, required); the passcode card repeats it.
- 10 The image is two-stage: production deps and build output only, no
  data/, sparta/, agent/ or test scripts; the server is PID 1 as node.
- 12 Pid file on the volume (0600); Fly health check with the edge
  headers; Fly-Client-IP required; hosted logs carry only error names;
  `fly deploy --ha=false` documented.
- 13 docs/HOSTING.md rewritten to match, with the go-C smoke test.

Tests: tenancy hosted phase now covers all 19 startup refusals, the
allowlist against every route, Northstar off (incl. a forged token),
invites, per-client backoff, the sign-in burst, the notice and the delete
tool (107/107 on production servers). scripts/test-image.sh checks the
real image (13/13 locally; needs Docker, not in test-all).
test-all 14/14.
@allsmog

allsmog commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Sean: the review fixes are up in #5 (stacked on #4, then on this PR). All of items 1 to 19 are covered except the clinical-screening-window fixture wording, which is Shruti's. It also adds agent connection for local workspaces. test-all is 14/14 (tenancy 115/115 on production servers), scripts/test-image.sh is 13/13 on the real image, and Northstar UI text is 72/72 identical to 850971e. Merge order: #3, then #4, then #5. Deploying still needs my separate go.

liminalshruti added a commit that referenced this pull request Oct 22, 2026
…s, on the record, in MCP and in the agent directive

Founder ruling P1 #3 (LIM-2422 fc5ff7ec): no contradictions. Until leadership approves the launch, every custom surface says the teams agreed and the launch is pending. No decision yet, or "not yet", both count as pending.
- UI: the record badges read "Agreed by both teams · launch pending" (or "· launch approved"), on the teams' record (DecisionView), on leadership's record (OrgView) and in the header (Home). The strip reads "agreed · launch pending".
- UI: the record headline no longer says "is now the shared plan" until the launch is approved. The owner's pre-approval title no longer says "make it the shared plan".
- MCP align_get_accepted_plan: the text adds an "Agreed by both teams · launch pending/approved" line. While launch is pending, run_command, an existing string field, carries the pending sentence. No schema change, so the tools/list listing is unchanged.
- Agent wake-up directive (custom): adds a "Launch: pending" line telling agents to plan only, or "Launch: approved by ...". Sean's REPLAN safety wording is unchanged.
- Northstar paths are untouched.
- Checked with lint, tsc and node --check only; no server until the Next.js upgrade.

Carried onto #7 (f079f76), #7's record wins where both show conditions:
- Record header: #7's amber "Agreed · conditions not checked" badge (real conditions only) beside the launch-pending
  status badge, which replaces #7's "Agreement reached".
- Dropped the record's "Conditions: not confirmed yet" status line (#7's badge and Conditions section say it).
- Dropped the "Conditions from <team>" bullet in the Proposals record summary (#7's record shows the conditions).
- Conditions everywhere else (approve reminder, leadership's record) come from the conditions field only, never the
  reason for asking for changes, matching #7.
@liminalshruti

Copy link
Copy Markdown
Collaborator Author

Landed via #9: merge commit 95c43be on main (Oct 23 03:03Z), whose second parent is Sean-approved 3cd7f8a. This PR's head (1d30dc7) is an ancestor of 3cd7f8a, so all its commits are on main. It was already closed (not merged) when checked at 03:05Z; per the founder ruling on LIM-2424 it was not merged separately.

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.

2 participants