Review (2026-08-21) found that stage 1 promoted ExecutionResult from a
plain class to @dataclass(frozen=True) along the way. No caller mutates
or compares these objects by identity, so the only externally visible
change is structured log output. User opted to keep the upgrade.
- domain/execution.py module docstring: explicit note that the frozen +
value-equality form is a deliberate enhancement, not a behavioral
accident
- CLAUDE.md "Schedule service layering" lesson: add a "don't silently
upgrade dataclass-ness during a structural-only refactor" note so
future refactors copy class definitions verbatim unless they intend
to tighten semantics explicitly
No code change; tests still 29 green.
Co-Authored-By: Claude <noreply@anthropic.com>
Follow-up to the layered refactor (review-driven):
- Move scheduling/orchestrator.py -> application/orchestrator.py
(orchestrator is application-level coordination, not a cron-trigger
primitive; matches the intended target tree)
- Migrate orchestrator re-exports from scheduling/__init__.py to
application/__init__.py; scheduling/ now exposes only CronScheduler
- Rewrite imports + 5 mock.patch string targets in test_janitor.py and
the orchestrator import in test_layering.py
- Update docstring refs in application/service.py + execution/worker.py
- Add 4 runner smoke tests (test_layering.py): _limited_log under-limit /
empty-sentinel / above-MAX_LOG_BYTES truncation; execute_artifact
rejects unsupported script_type with ValueError
- infrastructure/__init__.py re-exports SchedulerStorageClient so
``from schedule.infrastructure import SchedulerStorageClient`` is a
stable top-level surface
- CLAUDE.md engineering note: extend the commit trail to 7476c27 and
note the stage-8 orchestrator placement
Zero behavior change; schedule/pyproject.toml untouched. 29 tests green.
Co-Authored-By: Claude <noreply@anthropic.com>
Pin the new five-layer contract so a later refactor can't silently break
an import surface or lifecycle:
- domain: ExecutionResult defaults, terminal/failed state-set invariants,
naive_utc normalization (import-side-effect-free)
- infrastructure.storage: SchedulerStorageClient base64 upload via
httpx.MockTransport (no live server; base_url required for relative URL)
- scheduling: CronScheduler start/close lifecycle, global trigger cleared
- application: SchedulerService wires cron + orchestrator + worker, handler
dispatch table points at the wired NodeExecutor
- execution: schedule.notebook_runner shim re-exports the real main
25 schedule tests green; schedule/pyproject.toml untouched.
Co-Authored-By: Claude <noreply@anthropic.com>
- Move service.py -> application/service.py (SchedulerService class name
unchanged; build_object_store / build_storage_http_client move along)
- main.py imports schedule.application.service
- Rewrite worker.py's lazy `from schedule.service import build_object_store`
and test_worker.py's mock patch string targets — same class of bug as the
test_janitor patch strings (silent no-op until the old file is deleted)
- Delete flat service.py (orphaned; only docstring refs remain in
orchestrator, cleaned up in stage 6)
- Zero behavior change; schedule/pyproject.toml untouched
Co-Authored-By: Claude <noreply@anthropic.com>
- Move worker.py -> execution/worker.py, executor.py -> execution/executor.py
(byte-identical copies; import sites updated)
- Merge old execution.py + notebook_runner.py into
execution/runners/notebook.py: subprocess CLI (main/emit_outputs) plus the
in-process helpers (_execute_notebook/_execute_python/execute_artifact)
- schedule/notebook_runner.py becomes a compatibility shim so
`python -m schedule.notebook_runner` (the worker's stable -m string) still works
- Delete flat execution.py (shadowed by the new execution/ package)
- Zero behavior change; schedule/pyproject.toml untouched
Co-Authored-By: Claude <noreply@anthropic.com>
Stage 3 of the layered refactor. Relocate the two scheduling components
into their own package so that domain / application / scheduling /
execution / infrastructure boundaries actually exist on disk.
- Add schedule/src/schedule/scheduling/__init__.py
- Move scheduler.py (232 lines) -> scheduling/scheduler.py
(byte-identical via diff; CronScheduler class name unchanged)
- Move orchestrator.py (946 lines) -> scheduling/orchestrator.py
(byte-identical via diff; DispatchOrchestrator + event constants
unchanged; NOT further split this round, per plan)
- service.py lines 39-40: import paths rewritten to the new module
- tests/test_janitor.py: rewrite the import + 5 patch() string targets
The 5 patch() targets ("schedule.orchestrator.session_scope" x3,
"schedule.orchestrator.asyncio.sleep" x2) were NOT caught by the
import-line grep — they patch module attributes at runtime and would
have become dead no-ops after the move (and would hard-raise once
the old module is deleted in stage 6). Rewriting them to
"schedule.scheduling.orchestrator.*" keeps the janitor tests meaningfully
exercising the new module.
- old flat scheduler.py / orchestrator.py left on disk; stage 6 deletes
them once all layers are extracted.
Validation:
- uv run --package schedule pytest schedule/tests -q: 18 passed
- uv run python -m compileall schedule/src: zero errors
- grep 'from schedule.(scheduler|orchestrator)\\b' (old paths): 0 matches
- grep '"schedule.orchestrator.' (old patch targets): 0 matches
- main.py / worker.py / domain/ / infrastructure/ / pyproject.toml
byte-identical to HEAD
Co-Authored-By: Claude <noreply@anthropic.com>
Stage 2 of the layered refactor. Move the storage HTTP client one
package deeper so that infrastructure code lives under a dedicated
namespace.
- Add schedule/src/schedule/infrastructure/__init__.py
- Add schedule/src/schedule/infrastructure/storage/__init__.py
- Add schedule/src/schedule/infrastructure/storage/client.py
(verbatim copy of old schedule/src/schedule/storage_client.py,
byte-identical via diff — 2682 bytes)
- main.py line 17: import path rewrite to the new module
(only consumer — service.py and worker.py take storage_client as
an `Any` constructor param and never imported the class)
- old schedule/src/schedule/storage_client.py left on disk; stage 6
deletes it once all layers are extracted.
Validation:
- uv run --package schedule pytest schedule/tests -q: 18 passed
- uv run python -m compileall schedule/src: zero errors
- grep 'from schedule.storage_client' (old path): 0 matches
- service.py and worker.py byte-identical to HEAD
- main.py / pyproject.toml / tests/ unchanged apart from the 1 import line
Co-Authored-By: Claude <noreply@anthropic.com>
Add the three files that complete stage 1 of the layered refactor:
- schedule/src/schedule/domain/__init__.py (empty package marker)
- schedule/src/schedule/domain/context.py
(TERMINAL_NODE_STATES / FAILED_NODE_STATES / TERMINAL_RUN_STATES / naive_utc —
pure types, no I/O)
- schedule/src/schedule/domain/execution.py
(ExecutionResult dataclass, frozen=True)
The corresponding import-path rewrites in worker.py / orchestrator.py /
scheduler.py / execution.py were already landed in cdcfcb2 (the prior
commit on this branch). This commit only adds the missing domain/
package files those imports point at.
Validation:
- uv run --package schedule pytest schedule/tests -q: 18 passed
- uv run python -m compileall schedule/src: zero errors
- from schedule.domain.context / schedule.domain.execution importable
- main.py / pyproject.toml / tests/ unchanged
Co-Authored-By: Claude <noreply@anthropic.com>
B1: `_mark_upload_failed_and_raise` now commits on a separate session
- Helper takes `request + upload_id`, opens a fresh session from
`request.app.state.session_factory` and commits there before raising.
- Closes the named-lock connection-pool leak Codex flagged: the old
"commit-on-the-same-session" implementation could return the
GET_LOCK connection to the pool before the enclosing
`finally: release_named_lock` ran, leaking `mp:<hash>` for up to
`pool_recycle` and re-opening the same-key upload race.
- Same helper now used by `create_server_object_payload`'s put-failure
branch — two failure paths have identical semantics.
B2: streaming copy for soft-delete + restore (`get_stream() + put()`)
- LOCAL backend: zero-copy (aiofiles stream write). OOM fixed.
- S3 backend: still OOMs on multi-GB objects — `put()` materializes
the async iter via `b"".join(chunks)`. Multipart `put` is a
follow-up; do NOT claim "OOM fixed on production" since production
defaults to S3.
C1: worker re-verifies `Users.status='active' AND is_deleted=0`
- `_assert_user_active` called from `_execution_context` after
resolving `triggered_by`; skips `SYSTEM_CRON_USER_ID`.
- `USER_DISABLED` error_code goes into the `NODE_FINISHED_EVENT`
outbox payload — `schedule_node_runs` has no `error_code` column,
the row only carries the `message` text. Docstrings corrected to
say so explicitly (previous docstring falsely promised row-level
observability).
F1: honest browser-local file lock
- `api.ts` `acquireFileLock/heartbeatFileLock/releaseFileLock/
releaseFileLockOnUnload` are now no-ops with comments stating they
never call the network.
- `scriptWorkspaceStore` dropped `tickHeartbeats`; `tickCleanup`
simplified to just clear cache.
- `useEditSessionLifecycle` dropped its 15s heartbeat `setInterval`.
- `ScriptWorkspace.tsx` renders `.local-lock-banner` info bar when
`isEditing`. Two tabs may still silently last-write — banner is the
only guard (acceptable disclosure-only tradeoff).
Dead code: deleted the duplicate `upload_bytes_to_session` in
`backend/src/backend/storage_api.py`. The `services.storage` import
is now the only source of the function; `create_upload_record`'s
docstring updated to point at `backend.resources`.
S2: schedule worker add janitor task that force-terminals node_runs
whose deadline (timeout_seconds + retry_count*retry_interval + 120s
slack from started_at) has passed. Closes the gap where outbox retry
exhaustion (5 tries, capped 30s backoff) marked the *event* failed
but left the *node_run* stuck in queued/running forever. Re-reads the
row under FOR UPDATE before writing so a worker that races us to a
real terminal state is not overwritten; idempotency key uses
:timed_out variant so the :finished path cannot collide.
R1: extract _reap_once() from _reap_loop for testability; in the
dead-process branch, re-verify (process.pid, started_at) against the
live JUPYTER_PROCESSES entry before del. A start_workspace that
replaced the dead record mid-cycle used to have its new entry
silently erased by the reaper's stale snapshot — leaked the port.
R2: delete _drop_workspace_lock and its two call sites
(stop_workspace tail, get_workspace 404 path). Popping the lock
object after release breaks mutual exclusion for any coroutine still
holding the old reference while a fresh caller gets a new lock
object — same ws_id can race two starts. The dict is bounded by the
number of workspaces so the leak is negligible; invariant lives on
WORKSPACE_LOCKS in a comment.
Tests:
- schedule/tests/test_janitor.py — 8 tests covering normal kill /
healthy-skip / worker-race / never-started / multi-row batch /
cancellation propagation / per-iteration self-heal
- runtime/tests/test_process.py — 7 tests covering reaper identity
match / replacement-skip / alive-preserved + lock
same-object / concurrent-serialize / survives-stop /
helper-removed guard
uv run --package schedule pytest schedule/tests → 14 passed
uv run --package runtime pytest runtime/tests → 7 passed
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fix lands in three concentric layers, all backed by a single
INTERNAL_SERVICE_TOKEN shared secret so we have one mechanism
instead of three:
1. docker-compose: drop the backend.ports: 8891:8000 and
runtime.ports: 8892:8000 mappings. Nginx is the only host
ingress again (architecture §2.2).
2. /internal/v1/*: the storage control plane had six endpoints, five
of which were dead code (frontend already migrated to
/api/v1/data-resources/* with JWT; schedule only ever called
POST /internal/v1/objects). Delete the dead routes, mount the
one survivor with Depends(require_internal_service) that
compares the X-Internal-Service-Token header against
settings.internal_service_token with secrets.compare_digest.
3. POST /api/v1/jupyter on the runtime container: previously open
inside the Docker network. Same token mechanism — backend's
runtime_http_client now carries the header, runtime's
handle_jupyter_action requires the same header. /api/v1/health
stays open for the Nginx and compose healthchecks.
The schedule worker was already configured to call
POST /internal/v1/objects; build_storage_http_client now
sets the token header so its existing call site keeps working
without changes.
Files touched:
backend/src/backend/storage_api.py # 5 dead routes deleted + token guard
backend/src/backend/main.py # runtime_http_client header
runtime/src/runtime/main.py # require_internal_service Depends
common/src/common/config.py # internal_service_token setting
schedule/src/schedule/service.py # httpx client header
docker-compose.yml # ports dropped, INTERNAL_SERVICE_TOKEN env
.env.example # INTERNAL_SERVICE_TOKEN placeholder
API.md / README.md / DEVELOP.md # §9 trimmed to 1 endpoint
Verified:
compileall -> 0 errors
pytest backend/tests -> 37 passed
in-process ASGI smoke:
POST /internal/v1/objects no/wrong/correct token -> 401/401/200
POST /api/v1/jupyter no/wrong/correct token -> 401/401/200
5 deleted internal routes -> 404
docker compose config (with env) -> OK
P0-1 still has one open sub-item (rclone RC --rc-no-auth) that
the user has explicitly deferred; not touched here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the old RustFS-specific storage layer (common.storage.client /
RustFSObjectStore) with a minimal sync/async abstraction:
AsyncStorageBackend: put / get / get_stream / delete / exists / stat /
list / get_url / copy
StorageBackend: same surface, sync implementations
create_storage({"type": "s3" | "local", "mode": "async", ...})
backends/s3.py: S3-compatible (boto3 / aioboto3)
backends/local.py: on-disk filesystem (aiofiles)
Concretely:
- Drop RustFSObjectStore + common.storage.client (deleted).
- Drop the RustFS-specific ensure_bucket / presign_put / move_to_trash /
rewrite_to_public_path / sha256 / put_bytes methods.
- Migrate backend/storage_api.py + backend/main.py + backend/scripts.py
+ schedule/service.py + schedule/worker.py to the new abstraction.
- Migrate backend/storage_client.py + schedule/storage_client.py to
stub status (HTTP wrapper is dead code post-migration; rewrite pending).
- Rename all RUSTFS_* env vars to S3_* across .env.example,
docker-compose.yml, default.conf, scripts/nginx-entrypoint.sh,
common/config.py.
- Replace hardcoded rclone remote name "rustfs" with "s3" in
docker-compose.yml + config.py default.
- Rename "rustfs" SQLAlchemy column comments + table comments to
provider-neutral wording; StorageObjects.storage_backend enum
value moves from "rustfs" to "s3" (DB rows with the old value will
fail the != "s3" check until a one-shot migration is applied).
- Drop unused common/src/common/migrations/{README,env.py,script.py.mako}
(the alembic setup lives in /migrations/, not here).
Migration of the old abstractions has been done in one pass; per-route
method calls (delete / stat / put / get_url) are now direct one-liners
against AsyncStorageBackend.
After this commit:
- All Python imports resolve; routes compile (compileall green).
- s3 mode is fully wired.
- Routes that depended on removed methods (presign_put, move_to_trash,
rewrite_to_public_path, head() metadata) raise NotImplementedError
with a one-line TODO; rewriting these route handlers is the next step.
schedule:
- New _execution_loop runs alongside _database_event_loop. It claims
job.node.execute rows, sets a 30-min lease on available_at, then
dispatches each as asyncio.create_task under a Semaphore(N).
Polling loop is back to sub-millisecond turnaround for
schedule.run.requested and job.node.finished. Long notebook
execution no longer blocks DAG advance events.
- _process_pending_events filters by event_type IN
('schedule.run.requested', 'job.node.finished'); the executor
loop owns job.node.execute exclusively.
- _process_outbox_event builds a plain dict envelope before
handler dispatch; the previous ORM-row handoff risked
DetachedInstanceError once the outer session closed.
- _sync_once uses get_job + reschedule_job for existing job ids
instead of add_job(replace_existing=True). Each cron schedule
no longer removed-and-readded every 5s.
- service.py threads settings.schedule_execution_concurrency into
the orchestrator (default 4).
common:
- create_async_engine gets explicit pool_size=10, max_overflow=20,
pool_recycle=1800. No more relying on SQLAlchemy defaults.
- New schedule_execution_concurrency setting.
runtime:
- scan_workspaces: add missing 'import os' (NameError on startup)
and switch to asyncio.gather bounded by Semaphore(4) so N
workspaces start in parallel instead of sequentially.
Co-Authored-By: Claude <noreply@anthropic.com>
Replace uvicorn with gunicorn + UvicornWorker in backend, runtime,
and schedule Dockerfiles. New gunicorn.conf.py per service exposes
bind/workers/timeout/graceful_timeout as GUNICORN_* env knobs.
Notable details:
- schedule: timeout=0 (disables gunicorn worker heartbeat) so long
notebook execution isn't killed by gunicorn's silent-worker kill.
- All three configs: drop dead threads=4 setting; UvicornWorker is
async and ignores threads.
- Pin gunicorn>=26.0.0 in each pyproject; uv.lock regenerated.
- Drop stale 'COPY contracts ./contracts' from runtime and schedule
Dockerfiles (contracts dir was deleted in an earlier refactor;
builds would have failed).
- backend Dockerfile: switch to uv sync layout matching runtime/
schedule; add build deps for native wheels.
Co-Authored-By: Claude <noreply@anthropic.com>
tao.chenandtao.chen