Skip to content

fix(relay): restore commandline bundle CI - #4

Open
PekingSpades wants to merge 6 commits into
mainfrom
codex/fix-relay-ci-commandline-bundle
Open

PekingSpades wants to merge 6 commits into
mainfrom
codex/fix-relay-ci-commandline-bundle

Conversation

@PekingSpades

Copy link
Copy Markdown
Collaborator

Summary

  • use Python 3.13.10 for relay bundled commandline runtime to match currently available cross-platform wheels
  • pin pandas to an available wheel version and use macOS 11 x86_64 tags for Intel macOS bundle dependencies
  • prebuild known pure-Python source-only pyaes into a wheelhouse for cross-platform pip installs

Verification

  • node --check relay/scripts/prepare-commandline-bundle.mjs
  • git diff --check
  • pip dry-run wheel resolution for base requirements across linux/darwin/windows targets
  • full local darwin-amd64 commandline bundle preparation with NODE_USE_ENV_PROXY=1 PIP_INDEX_URL=https://pypi.org/simple

Do not merge yet; waiting for CI confirmation and owner review.

JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…idation

Review follow-ups zai-org#4 and zai-org#5:

zai-org#4 — the shared logger reads LOG_LEVEL/NODE_ENV at module-eval, but config
imported the logger BEFORE running dotenv, so values set only in .env were
missed and the global logger initialized with defaults. Extract
infrastructure/env-bootstrap.ts (a side-effect dotenv loader) and import it
first from both the logger and config, so .env is applied before any env reader
evaluates. Verified: LOG_LEVEL=warn (env-only) → logger.level=warn.

zai-org#5 — two startup-failure footguns:
  - NODE_ENV was a z.enum(development|production|test); a 'staging' deployment
    would fail to boot. Relaxed to a non-empty string (consumers compare modes
    themselves). Verified NODE_ENV=staging boots.
  - MEMORY_RECALL_TOP_K='' (empty) hit z.coerce.number→0→.positive() and threw
    instead of falling back. Add optionalPositiveInt() that treats missing/empty
    as undefined. Verified topK falls back to recallLimit.

Tests: config suite 6 green.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…uction

Review follow-up zai-org#4: crypto's getKey() only threw on the FIRST encrypt/decrypt,
so a production deployment missing both keys would start fine and then 500 on
the first secret save/read. Add the keys to the env schema and a superRefine
that fails config validation (→ startup process.exit(1)) when NODE_ENV is
production and neither MCP_ENCRYPTION_KEY nor APP_SECRET is set. The crypto
runtime check stays as defense-in-depth. Verified: prod-no-key fails parse,
prod-with-key/APP_SECRET and dev-no-key pass.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…nger hangs

Four review follow-ups on the lock-loss recovery:

zai-org#1 — replay flag set too early: sideEffectsStarted flipped true before
executeActorActions REGARDLESS of result.actions, so a pure-reasoning turn (no
actions, no message — executeActorActions is a no-op, persistence kind:"none")
was treated as replay-unsafe and its input wakeups were DROPPED on a lock loss.
Rename to replayUnsafeStarted and set it true only when a non-idempotent write
actually happens: result.actions.length > 0, or messagePersistence.kind !==
"none".

zai-org#2 — late independent wakeups stranded: the replay-unsafe branch skipped the
pending-wakeup check, so a message that arrived mid-turn (never attached to
this turn, and enqueueSessionWakeup won't enqueue a job while running) had no
driver. Now BOTH branches re-check getPendingWakeupCount after the restore/drop
and requeue if any remain. This never replays the current turn: in the unsafe
case its own wakeups were dropped (not pending), so only genuinely-unprocessed
input drives the re-run.

zai-org#3 — concurrency test hung (exit 124): it materialized the ioredis singleton
but never closed it. Add an `after()` hook calling shutdownRedisConnections().
Verified it now exits 0 even WITHOUT --test-force-exit.

zai-org#4 — test didn't cover the real decision: extract decideLockLossRecovery()
(pure: replayUnsafe → restore|drop, requeue iff pending remains) and route the
handler through it, then drive the concurrency test through that same policy.
Added the two regressions above as explicit cases (pure-reasoning replay-safe;
replay-unsafe-but-late-pending requeues).

api typecheck clean; full api suite 1370 pass / 0 fail / 25 skipped.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…s, auth-origin plumbing

High zai-org#1 (native email login/register session): the custom synapse_session cookie
name broke @better-auth/expo's cookie-jar (it only persists cookies matching its
prefix or *session_token). Revert to BA's DEFAULT cookie name
(better-auth.session_token) — no business code reads the cookie by name anymore.
getSessionBearerToken() now matches by endsWith('session_token') (handles the
__Secure- prefix). session-provider also prefers signIn/signUp data.token, with
the cookie-jar value as fallback.

High zai-org#2 (native Feishu OAuth return): refreshSession() now falls back to the
expo cookie-jar token (getSessionBearerToken) when SESSION_TOKEN_KEY is empty
and persists it — so a deep-link return from the OAuth browser flow establishes
the session.

Medium zai-org#3 (web device polling): pollDeviceToken() unwraps the RFC 8628 HTTP-400
error body (authorization_pending/slow_down/access_denied/expired_token) from
the thrown ApiError.details and returns { error } instead of throwing, so the QR
panel's normal-wait and terminal branches work.

Low zai-org#4 (mobile auth-origin plumbing): EXPO_PUBLIC_AUTH_ORIGIN wired through
setup.sh + docker-compose mobile-web build args + Dockerfile.mobile-web
(optional; defaults to API origin for same-origin deploys).

Verified: server emits better-auth.session_token + set-auth-token + body token;
simulated expo cookie-jar capture + bearer resolution succeed; api 1384/0/25;
web typecheck 0; integration 20/20; mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…s, audit FK, version ownership, timeout cancel, raw payloads, SW bundles

Seven review findings:

1. (blocker) Tool-call id was the SDK's native id (call_/toolu_), written as the
   tool_calls.id UUID PK → first real tool call would fail insert. fromGenerateText
   now mints a UUID canonical callId and keeps the SDK id as providerCallId
   (restores the pre-existing design; the SDK regenerates native ids each turn).
   + regression test asserting callId is a UUID and providerCallId is the SDK id.

2. providerOptions + Anthropic server tools (web_search/web_fetch) were configured
   but never reached generateText. index.ts now passes providerOptions and merges
   buildServerTools(providerKind, serverTools) into the tool set (anthropic.tools.*).
   aiComplete passes providerOptions too.

3. Retention purge deleted model_binding_versions while provider_steps
   (model_binding_version_id RESTRICT) still referenced them → purge could wedge.
   Both provider_steps and model_binding_versions are now NEVER_PURGE (append-only
   audit + audit-anchoring config snapshots); they are erased only by tier-B
   tenant hard-erase, which deletes provider_steps before versions in topo order.

4. current_version_id wasn't constrained to the binding's own versions (binding A
   could point at binding B's version, corrupting traceability). Added a BEFORE
   INSERT/UPDATE trigger asserting current_version_id ∈ versions of this binding
   (kept the FK single-column SET NULL so the binding<->version purge stays acyclic;
   a composite FK would re-introduce the topo cycle). Verified the trigger rejects
   a cross-binding version on a live DB.

5. Timeout didn't cancel the provider request (withTimeout only rejected the
   promise). Replaced with an AbortController fed to generateText({abortSignal}),
   so the underlying request is actually aborted; removed the dead withTimeout helper.

6. Audit payloads split clearly: request_payload = { synapseRequest, sdkRequest
   (raw outgoing), bindingVersionId/providerKind/vendor/modelName snapshot };
   response_payload = { sdkResponse (raw), stopReason, toolCalls, textContent }.

7. (CI) Regenerated the service-worker bundles (web + mobile) — they had drifted
   after ROUND_ROBIN / MODEL_PROFILE were removed from shared constants.

Validated end-to-end on the isolated test DB: schema+trigger apply, seed imports
4 groups, db:codegen:verify matches, zai-org#4 trigger rejects cross-binding version,
soft-delete regression 27/27, sw-constants 8/8, guards green, api+web-next typecheck 0.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…e contracts

Round-2 review: the shared schemas existed but weren't the runtime contract,
and the devices write-path/cloud + service-shaping were still in the old world.

zai-org#1/zai-org#2 devices WRITE-path → shared camelCase (master plan §5.1.1/§8.3):
- shared/schemas/devices.ts gains app-facing input contracts (camelCase):
  CreateCloudDeviceInput, StartPairingInput, ClaimDaemonServiceInput,
  SetActiveDeviceCapabilitiesInput + CreateCloudDeviceResultView /
  ActiveDeviceCapabilitiesView.
- controller parses these; device-sdk + web send camelCase bodies
  (device_type→deviceType, service_kind→serviceKind,
  remote_agent_machine_id→remoteAgentMachineId,
  device_capability_ids→deviceCapabilityIds, workspace_id→workspaceId,
  host_provider→hostProvider).
- access-bindings POST now on the shared SetActiveDeviceCapabilitiesInput
  (the wire-shape test reframed to the app-facing contract); GET list key →
  deviceCapabilityIds.
- cloud.ts result → camelCase; sandbox docker-backend + its test updated.
- device-runtime startPairingSession consumes the shared camelCase
  StartPairingInput + DevicePairingTicketView.
- removed the now-orphaned device-protocol input/result schemas
  (CreateCloudDeviceInput/Result, StartPairingInput, PairingTicket,
  ClaimDaemonInput, SetActiveDeviceCapabilitiesInput); kept the genuine wire
  ConsumePairing*/ScopedSubjectTargetWire.

zai-org#3 model-groups + mcp-plugins controllers → sendData(ViewSchema, value):
- 30 model-groups + 8 mcp-plugins sites now parse the shared View schemas at
  runtime (the schemas were "dead definitions"). Envelope → { data }.
- updateModelGroup always returns ModelGroupDetailView (single schema for PUT).
- web lib/api.ts unwraps { data } in-method, keeping each method's public
  shape stable so callers are unchanged. Endpoints without a View schema
  (auth-session/audit/install-plan) deliberately left as-is.

zai-org#5 devices service no longer shapes DTOs: listDevices/getDevice/startPairing/
claimRemoteAgentDaemon return domain records (DeviceSummaryRecord/
DeviceDetailRecord/DevicePairingTicketRecord/DeviceServiceRecord); the
controller calls the presenter. Added presentDevicePairingTicket.

zai-org#4 mcp-plugins schemas: z.custom already gone (r1); documented the genuinely
plugin-author-owned opaque JSONB (configSchema/configData/toolsManifest/
metadata) — the structured fields are already nested-schema'd.

zai-org#7 device-sdk tests: added src/index.test.ts (0 → 8 tests) locking the
camelCase send/parse boundary + the snake wire consume shape; build moved to
tsconfig.build.json so tests stay out of dist.

Verification: full monorepo build green; api 1640 pass/0 fail/2 skip; guards
(layering/db/fk/soft-delete/datetime) clean; device-sdk 8/8, device-runtime
314/315(+1 skip), remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…ervice→presenter retreat

Round-3 review: app-facing endpoints still on bare reply.send, /auth/me unmigrated,
mcp-plugins auxiliary flows uncollected, and service still produced outward Views.

zai-org#1 devices controller → sendData { data } (§8.3): list/detail/pairing/claim/cloud
now sendData(ViewSchema, ...) → { data: ... }. device-sdk read methods unwrap
res.data + parse the shared view; device-sdk tests updated to { data } fixtures.
204 endpoints (delete/detach) unchanged; consume/bootstrap stay bare wire.

zai-org#2 access-bindings GET → sendData(ActiveDeviceCapabilitiesViewSchema, ...) →
{ data: { deviceCapabilityIds } }. POST stays 204 on the shared input contract.

zai-org#3 mcp-plugins auxiliary endpoints collected to sendData with new shared schemas
(PluginAuthSessionView/Envelope, PluginInstallPlanView/Envelope, PluginAuditLogList):
install-plan, auth-session start/get/inspect, audit tool-calls/events. Only the
OAuth HTML callback + { error } envelopes remain bare. install-dialog unwraps
res.data for the auth-session methods.

zai-org#4 /auth/me GET+PUT → sendData(AuthMeViewSchema, { user, session }) → { data }.
New shared schemas/auth.ts (UserProfileView/AuthSessionSummaryView/AuthMeView).
web lib/api.ts getMe/updateMe + lib/server-auth.ts read res.data.{user,session}.

zai-org#5 service→presenter retreat (controller owns presentation) across 11 app-facing
modules (organization/files/relationship/memory/remote-agents/workspace/
workspace-apps/automation/chat/model-groups/mcp-plugins): service.ts now returns
DOMAIN RECORDS; the controller calls present*() before each send (envelopes
unchanged). present*() in service.ts: 106 → 65 (remainder are nested-record
assembly + cross-module-consumed records + Tier-D timestamp wrappers, permitted
by the layer-matrix JUDGMENT clause). New record types in repo.types/presenter
per module. Fixed 2 new r7 (return { ...row }) violations the retreat introduced
(remote-agents/workspace-apps) via const-assignment.

Tier-D / non-HTTP modules (session/context/tasks/tool-call-tasks) excluded per
plan ("不强制 presenter").

Verification: full monorepo build green; api 1640 pass/0 fail/2 skip; guards
(layering/db/fk/soft-delete/datetime) clean; device-sdk 8/8, device-runtime
314/0fail, remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
… guard close

Round-4 review: cloud-input schema stricter than server; relationship/remote-agents
controllers still bare; access-bindings half-migrated; TableRow leaked outside repo
files (guard couldn't prove repo-only DB access); §5.3 split-controller/appRoute
mechanism + guard rule 5 entirely unadopted across mixed modules.

zai-org#1 CreateCloudDeviceInputSchema: title/hostProvider → optional (the server
applies defaults; the strict schema would reject requests the server accepts).

§5.3 (subsumes zai-org#2 + zai-org#3) — adopt appRoute/wireRoute across ALL mixed modules:
implemented guard rule r5 (mixed modules forbid bare app.<verb>; must use
appRoute/wireRoute/split-controller). Migrated every bare route in devices,
mcp-plugins, runtime-authorizations, files, relationship, remote-agents,
automation, im — APP routes → appRoute (handler returns value; helper wraps via
sendData → { data }), WIRE (handshake/webhook/binary/HTML/WSS/reverse-MCP) →
wireRoute (bare). devices/mcp-plugins were pure registration swaps (already
sendData, no envelope change); the bare modules went bare→{data} with new shared
View schemas (automation/files/im/relationship/remote-agents/runtime-
authorizations.ts) + web/mobile consumer unwraps in lockstep. Guard r5 baseline
now 0. Fixed appRoute to PRESERVE a handler-set status (reply.status(201)) — the
prior code let sendData's default 200 clobber 201s; added route.test.ts locking
the { data }/201/204/wire contract.
zai-org#3 access-bindings GET query → camelCase (subjectKind/subjectActorId/…); route
already on appRoute + { data } response.

zai-org#5 repo-boundary: extended guard r2 from service/controller-only to ALL non-repo
files (only repo*.ts/repo.types.ts may use TableRow). Moved the TableRow/
TableInsert/TableUpdate aliases out of 12 helper files (access/binding-storage +
subject-registry, im/service/*, mcp-plugins/{plugin-auth-connections,mijia},
memory/indexing, session/runtime, tasks/action-tokens) into per-module
repo.types.ts. guard r2 baseline now 0. Also: guard now strips comments before
matching (kills doc-comment false positives).

GUARD-LAYERING BASELINE IS NOW FULLY EMPTY (0 grandfathered entries across all 6
rules: r1/r2/r3/r4/r5/r7) — the layering is genuinely enforced, not just passing.

zai-org#4 (service→presenter) was completed per-plan in round-3; the 64 remaining
present* are the JUDGMENT-permitted nested-record/cross-module/Tier-D cases.
zai-org#6 (web runtime schema.parse) not required by plan (device-sdk is the parsing
consumer); web stays a typed thin client.

Verification: full monorepo build green; api 1646 pass/0 fail/2 skip (+route.test);
guards (layering[empty]/db/fk/soft-delete/datetime) clean; device-sdk 8/8,
remote-agent-daemon 24/24; web + mobile typecheck clean.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…in AGENTS.md

Round-5 review re-flagged 4 items; an exhaustive re-audit shows 3 are already
resolved and 1 is at its intended end-state:

zai-org#1 CreateCloudDeviceInputSchema strictness — STALE: fixed in round-4 (fddca43);
   title/hostProvider are .optional() and device-sdk parses the same schema it
   sends, so SDK and server accept identical bodies. Verified, no change needed.
zai-org#3 TableRow outside repo — RESOLVED in round-4: the guard r2 baseline is empty
   ([]); all DB-row types live in repo*.ts/repo.types.ts; the guard now strips
   comments so doc-comment mentions aren't flagged. The reviewer's "few left,
   mostly comments" predates that. No change needed.
zai-org#4 web runtime schema.parse — OUT OF PLAN SCOPE: the master plan classifies
   web-next as an app client and device-sdk + the API's sendData/appRoute as the
   runtime-parse boundary. web is a typed compile-time-contract client by design;
   the 6 remaining snake reads are RFC8628 OAuth device-flow wire fields, and the
   3 any are memory request-body accumulators (no shared input schema). No change.
zai-org#2 service→present* (64 sites) — ALL PERMITTED: a fresh per-site classification
   of every present*() in service.ts found exactly 0 clean violations. Each is
   one of: (a) a row→record / record-assembly mapper producing an in-api
   XxxRecord (e.g. presentFileAsset→StoredFileRecord, presentActorRow→Actor,
   presentUser→User), (b) a thin presentInstant/presentOptionalInstant timestamp
   wrapper, or (c) a record consumed cross-module (automation rules read by
   ai/session-tools; remote-agents snapshot read by chat; etc.). The lone
   HTTP-body case (model-groups getModelGroup→ModelGroupDetailView) is also
   consumed in-api as a record by the seeder + ownership pre-handlers +
   updateModelGroup, so it's the same cross-consumed-record exception. The
   master-plan JUDGMENT clause + Tier-D exemption permit all 64.

To stop future reviews re-flagging this as "unfinished", codified the rule in
AGENTS.md (七层职责): service may call present* for row→record assembly,
timestamp wrapping, and cross-module records; what's forbidden is service
producing+bare-returning the OUTWARD app DTO as the sole HTTP-body source — that
stays in the controller via appRoute/sendData.

Verification: full build green; api 1646 pass/0 fail/2 skip; guards
(layering[empty]/datetime) clean; device-sdk 8/8; web + mobile typecheck clean.
No functional code change → running containers already on the verified r4 state.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…unique + enum cutovers + creds col) + full pty deletion

External review R4, foundational DDL + clean-slate phase. One atomic change (the runtime_exposures.builtin_kind enum drop and the pty TS removal must land together or codegen-verify breaks).

DDL (schema.sql, verified on the isolated :55632):
- #8: close BOTH empirically-confirmed mode/service-kind invariant holes. Extract assert_sandbox_mode_service_kind(runtime_id); the runtime_services trigger (guarded by TG_TABLE_NAME) re-validates the OLD runtime on an UPDATE-move, not just COALESCE->NEW (H1). Complete the kind<->service_kind matrix: bare_dataplane => kind='sandbox' (H2). Both now REJECT; legal sequences COMMIT.
- zai-org#4: uq_sandboxes_live_session UNIQUE(session_id) WHERE state IN ('provisioning','active') — <=1 live sandbox per session; predicate allows one 'closing' straggler + a fresh provision (avoids a teardown-leaves-closing deadlock).
- #10.5: drop 'committing' from sandboxes_state (zero writers).
- #10.1: drop 'pty' from runtime_exposures_builtin_kind.
- zai-org#6: add sandboxes.data_plane_credentials_encrypted TEXT (off-box token persistence; decrypt wiring later).

pty DELETED entirely (#10.1): shared policy/matcher/enums/types + pty.ts, device-protocol enum, api reject + projector + sandbox descriptor field + tests. typecheck 0 confirms full removal.

bootstrap CURRENT_SCHEMA_VERSION -> 2026-07-13-sandbox-r4. codegen:verify = exactly 3 type changes. verify:boundary green (api 2851/0).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…recovery

#8 (P1 data): off-box provision PUSH deleted the just-restored conflict sidecar. The
detached applyManifest re-ran materializeSnapshot (which CLEARS the mirror tree), but the
spine had already materialized base AND restored .synapse-conflicts into that same mirror
before the push -> the sidecar was wiped while sidecarRestoreOk stayed true (false positive).
Fix: pushMirrorToContainer — a NON-destructive replicate-only push (diff the already-
populated mirror into the VM, no re-materialize); the cube bridge's applyManifest now uses
it. The detached applyManifest keeps materialize+push for its tests.

zai-org#4 (P0 data): off-box teardown-fail recovery was boot-ONLY, and a 'closing' VM was excluded
from keepalive, so its provider TTL (<=1800s) destroyed the sole un-pulled copy with no
recovery unless the process restarted (design review: and infinite keepalive if kept alive
unconditionally). Fix (bounded, folding the review's F3):
- listKeepAliveSandboxResourceIds also refreshes a 'closing' VM with failed-RECOVERABLE
  mounts, but ONLY within a bounded recovery window (updated_at) — keeps the TTL from
  destroying the copy WHILE recovery retries, then stops so a genuinely un-recoverable VM
  is reclaimed at its TTL (bounded provider spend, no infinite renew).
- A periodic recoverFailedSandboxMounts interval (in-flight-guarded so a large pull can't
  overlap its own next tick) in index.ts, alongside the existing reapers.

Gates: typecheck 0, verify:boundary green (2905/0), 37/37 on working-set/reaper/keepalive.
Residual (documented): a gone-VM leaves a cosmetic stuck-'closing' row past the window
(bounded spend already fixed; no data corruption) — terminal escalation deferred.
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
A 16-agent / 8-dimension adversarial review of the committed R5 fixes surfaced
5 confirmed defects (3 P1 + 2 nit); a focused re-review of the P1 fixes then
found one residual in the zai-org#5 fix. All addressed:

C4 [P1] off-box teardown stale-loser clobber:
- The teardown lease re-check was gated behind `pullOk && commitOk`, so a
  SUPERSEDED loser whose pull FAILED because the winner had already DELETEd the
  shared VM skipped the fence and reached the ⑦-fail branch, clobbering the
  winner's just-'closed' mounts back to 'failed'. The re-check now runs
  UNCONDITIONALLY before the pull/commit fork, gating BOTH the DELETE cluster and
  the mark-'failed' branch. A legitimate single teardown with a transient pull
  failure still preserves its work.

C3 [P1] off-box keepalive renewed the provider TTL forever (unbounded spend):
- zai-org#4's keepalive 'closing' arm anchored on updated_at, which the ~180s closing-
  retry reaper self-flip bumps every re-drive, so the 1800s window never expired.
  It now also requires `teardown_epoch < MAX_OFFBOX_RECOVERY_ATTEMPTS` (20) — a
  MONOTONIC re-drive counter (updated_at and teardown_epoch are bumped by the SAME
  flip, so the window can't be kept fresh without advancing toward the cap). Once
  exhausted, teardown CONVERGES TERMINAL (close the failed mounts, rm the preserved
  mirror, flip 'failed', soft-delete) with a loud accept-data-loss alert instead of
  deferring — and re-driving — forever.

C5 [P1] file<->dir collision aborted the whole pull every turn:
- fetchChangedIntoMirror wrote fetched files BEFORE pruning, so a base FILE `/x`
  replaced by a VM DIR `/x/…` (or vice-versa) hit ENOTDIR/EISDIR and aborted the
  pull. Pruning now runs BEFORE the writes (files, then empty dirs), sparing any
  mirror empty-dir that is an ANCESTOR of a present VM file/dir. The empty-dir
  prune runs to a FIXPOINT (re-walk + prune) so a stale directory CHAIN collapses
  fully — a single leaf-only pass left the parent behind and still EISDIR'd a
  nested dir->file replacement at depth >= 2.

C8 [nit] a dead `reconnectDataPlane!` non-null assertion in recoverOffBoxSession
survived Phase G's de-`!` pass (a formatting mismatch) — removed.

C6 [nit] cube validateProduction's loopback check only caught 127.0.0.1/::1; it
now rejects the whole 127.0.0.0/8 block, ::1, IPv4-mapped IPv6 loopback, and
localhost (isLoopbackHost).

Tests: real-fs file<->dir + nested dir<->file collision round-trips; the keepalive
attempt-cap boundary; and the cube production loopback/http/key hardening (5
cases). verify:boundary green (2922/0).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…wnership to the flipped runtime

R6 review P1/P2. Two data-safety holes in the off-box teardown/recovery path:

zai-org#4 (P1) — teardownStaleRuntimeDataFree converged a stale 'closing' straggler by
DESTROYING its provider VM with no failed-recoverable defer. But when that VM is
the SOLE store of un-pulled bytes (a prior pull/commit failed and left the mount
'failed' + the VM alive), the data-free convergence would delete the only copy
before the recovery sweep could re-pull it → silent loss.

zai-org#5 (P2) — the teardown/recovery ref resolver (buildSandboxRefFromSandboxRow) fell
through to the mounts / createdAt-DESC by-session resolver even when the caller
PINNED a runtimeId. A newer session sandbox could be resolved instead of the row
we flipped → reconnect/kill/soft-delete aimed at the wrong VM. The post-CAS
deleteRuntime also ran unconditionally, so a superseded teardown that lost the
epoch race still soft-deleted a runtime the current owner was mid-teardown on.

Fix (design-reviewed — SOUND; convergence stays RUNTIME-scoped, never session):
- sandboxHasFailedRecoverableMounts(runtimeId): SELECT 1 FROM file_mounts WHERE
  sandbox_id = runtimeId AND status='failed' AND materialized_dir IS NOT NULL.
- teardownStaleRuntimeDataFree DEFERS destroy while epoch < MAX_OFFBOX_RECOVERY_
  ATTEMPTS and the row is off-box and it still owns such a mount (scoped to
  sandbox_id=runtimeId — a sibling generation's mounts neither block this
  convergence nor get pulled from the wrong VM). Only once the attempt-cap is
  exhausted does it destroy + accept loss.
- buildSandboxRefFromSandboxRow gains explicitRuntimeId: when pinned it resolves
  the ref from THAT exact row (getSandboxById) and returns null if it is gone —
  never the session resolver. Threaded through both teardown ref sites + recovery.
- deleteRuntime is now gated on the epoch-fenced terminal CAS WINNING across all
  four terminal paths (data-free, off-box terminal, off-box VM-gone, host
  terminal) — a lost lease no longer soft-deletes the winner's runtime (TOCTOU).
- recoverFailedSandboxMounts groups by SANDBOX_ID (was session): each sandbox is
  an independent VM, so zai-org#4 lets two generations' failed mounts coexist for one
  session; per-sandbox grouping + a pinned ref pulls each mount from its OWN VM.

No DDL. Test: an off-box straggler that still owns a failed-recoverable mount is
DEFERRED (row stays 'closing', runtime not soft-deleted) instead of converged.
verify:boundary green (sandbox 304/304).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…H-5 blocked/closed-session keepalive

R6 review, two P1 leaks in the off-box recovery/keepalive path.

H-2 — recovery had NO durable claim. recoverFailedSandboxMounts read every 'failed'
mount and re-pulled its VM with only an in-process guard, so two API replicas (or the
startup sweep racing the 3-min periodic one) could both reconnect + pull the SAME VM
into the same mirror concurrently — double-recovery / torn commit.

H-5 — a crash+restart marks each interrupted session 'blocked' (recoverInterrupted
Executions) but never tears down its sandbox; reconcile SHIELDS a healthy off-box VM,
so listKeepAliveSandboxResourceIds renews its provider TTL forever (infinite spend).

Fix (design-reviewed; DDL — requires db:rebuild):
- file_mount_status gains 'recovering' — a durable LEASE. claimFailedRecoverableMounts
  CASes 'failed'→'recovering' under FOR UPDATE SKIP LOCKED + RETURNING, so concurrent
  claimers get DISJOINT sets. A crashed worker's stale lease is reclaimed once its
  updated_at ages past RECOVERY_LEASE_TTL_SECONDS (600s); a clean failure is released
  back to 'failed' in a finally (releaseRecoveryClaims). recoverFailedSandboxMounts now
  claims → processes → releases.
- 'recovering' is NOT a live mount: EXCLUDED from uq_file_mounts_active_session_space/
  _subpath + idx_file_mounts_live_sandbox + getActiveMountsForSession + the docker-
  orphan probe — so a failed→recovering flip can never collide with a re-provisioned
  generation's live mount, and teardown/commit never grab a leased mount.
- 'recovering' IS still failed-recoverable: INCLUDED in sessionHas/sandboxHas
  FailedRecoverableMounts (the ⑦-guard + the zai-org#4 data-free defer keep the sole-source VM
  alive while a mount is mid-lease), closeSessionFailedMounts (exhaustion closes it),
  and the keepalive closing-arm EXISTS predicate.
- H-5: teardownBlockedSessionOffBoxSandboxes() (startup, before recoverFailedSandbox
  Mounts) tears down the off-box sandbox of every 'blocked' session via a pinned-runtime
  teardown (pull→commit→delete). Off-box ONLY — host/resident have no provider TTL
  (resource_id='') and reconcile reaps their dead process; docker is provider-backed but
  the adapter check skips it. listBlockedSessionLiveSandboxes resolves the candidates.
- H-5 defense-in-depth: listKeepAliveSandboxResourceIds LEFT JOINs sessions and, on the
  prov/active arm ONLY, drops sandboxes whose session is 'closed' (the crash-leftover
  infinite-renew). The closing+failed-recoverable arm is untouched — it must keep the VM
  alive for recovery regardless of session status.

Tests: +3 lease (claim/release/live-vs-recoverable split; TTL reclaim; partial-unique
never collides with a re-provisioned mount) + 2 H-5 (keepalive closed-session exclusion
+ closing-arm intact; blocked-session candidate query). verify:boundary green
(sandbox 309/309).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…'s mounts at the attempt cap (adversarial-review finding)

The R6 multi-agent adversarial review (7 fixes, 6 SOUND) confirmed one MAJOR defect in
Phase D's zai-org#4 recovery-defer, reproduced independently against the committed code.

teardownStaleRuntimeDataFree DEFERS destroying a superseded off-box straggler's VM while
recovery attempts remain, then at the teardown_epoch attempt cap CONVERGES: destroy the
VM (accept loss) + revoke grants + terminal-close the row + soft-delete the runtime. But
it never TERMINATED the straggler's own file_mounts. So after convergence they stay
status='failed' (or 'recovering'), materialized_dir set, sandbox_id=A — and the periodic
recoverFailedSandboxMounts sweep re-claims them every tick (claimFailedRecoverableMounts
keys on status + materialized_dir with no runtime join; buildSandboxRefFromSandboxRow's
pinned getSandboxById still resolves the soft-deleted row's stale resource_id), reconnects
to the now-DESTROYED VM, fails, releases the lease, and repeats FOREVER: one provider
reconnect API call per mount per tick + perpetual error-log spam, mounts stuck non-terminal
for the deployment lifetime. The sibling off-box teardown convergence (service.ts ~2790)
guards this with closeSessionFailedMounts; the data-free path omitted the equivalent.

Fix: new closeSandboxFailedMounts(runtimeId) — the SANDBOX-scoped twin of closeSession
FailedMounts. teardownStaleRuntimeDataFree calls it at convergence (right after the VM
destroy), flipping the straggler's failed/recovering mounts → 'closed' so the sweep stops
matching them. SANDBOX-scoped, never session-scoped: a session-scoped close would wipe a
LIVE sibling generation's failed mounts that share the session — the exact hazard the zai-org#4
runtime-scoping exists to avoid.

Test: at the attempt cap the convergence closes the straggler's own failed mount (livelock
stopped) while a live sibling generation's failed mount is left untouched. verify:boundary
green (sandbox+cube 316/316).
JoshZiel83 pushed a commit to JoshZiel83/Synapse that referenced this pull request Sep 19, 2026
…e" / phantom states / removed-column叙述

R7 clean-slate (finding zai-org#4). The runtime/sandbox generalization + the cubesandbox off-box
seams are DONE, but comments/schema-notes/the bootstrap ledger still narrated them as
mid-flight. Comments/docs only — zero behavior change.

- "later sub-phase / wired later / Uncalled in Phase 1a / for now": rewritten to describe
  the CURRENT wiring (adapter-registry workingSet/rebuildDataPlane/reconnectDataPlane docs
  + the two host-bare reconnectDataPlane twin sites; cubesandbox-adapter reconnect;
  service.ts readiness — the cube ready() DOES negotiate control + envd-version + domain +
  data-plane reachability now).
- Phantom 'committing' state (not in sandboxes_state={provisioning,active,closing,closed,
  failed} nor file_mount_status): dropped from repo.ts + service.ts comments (the verb
  "committing" is left alone).
- Removed device-mount叙述: deleted the redundant schema.sql file_mounts NOTE (device_id/
  sandbox_backend/sandbox_resource_id/pre-P2 device-shaped — already covered by the
  sandbox_id doc two lines up); reworded the pre-P2/device_id-fallback archaeology in
  service.ts + docker-sandbox.ts + repo.ts to state the current sandbox_id-is-sole-identity
  invariant.
- Dead field: ReadinessReport.platform/arch were declared but NEVER set by any ready()
  (only envdVersion/domain are populated, by the cube adapter) → deleted the two dead
  fields.
- "legacy" old-data framing → "unknown" where it meant an unrecognized adapter tag /
  a null-token identity (bare-dispatch, adapterForRow, host_pid_identity docs); genuine
  external "legacy" (fs-helper mode, storage-migration source, corrupt-blob decode) kept.
- schema.sql pairing "P1 unforked / interim" → the current consume-fork description.
- bootstrap ledger: collapsed the four sandbox-refactor entries (R6/R3/R2/generalization —
  R2's "nullable platform/arch" even CONTRADICTED R3's NOT NULL) into ONE final-state
  schema description; slug → 2026-07-14-sandbox-r7-cleanup. Comment-only schema.sql edits
  change SCHEMA_CONTENT_HASH, so this requires db:rebuild (codegen regenerated).

verify:boundary green (sandbox+cube+bootstrap 320/320).
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