Workspace integration: phases 2-8

[marklubin]

Summary

Completes the Workspace abstraction by integrating it into the server, viewer, CLI, and templates. This is phases 2-8 from the workspace RFC (#124).

• Phase 2: serve.py — serve() takes Workspace + ServerBindings instead of ServerConfig. Deletes 40+ lines of manual wiring. serve_from_config() adapter for backward compat.
• Phase 3: _state → _workspace — Global _state: dict replaced with _workspace: Workspace. All MCP tools and REST routes access project/config/queue/prompts through workspace. _resolve_bucket_dir() becomes workspace.bucket_dir().
• Phase 4+5: Viewer — ViewerState.from_workspace() classmethod. Workspace name shown in header. /api/status returns workspace name and state.
• Phase 6: Templates — synix.toml added to all 8 templates. deploy/synix-server.toml → deploy/synix.toml.
• Phase 7: E2e tests — 7 tests covering runtime activation, real queue/prompt store through workspace, build+release lifecycle.
• Phase 8: Deploy — Containerfile updated for renamed config.

Test results

• 2123 tests pass, 0 failures
• uv run release passes (lint + tests + 6 demos)

Test plan

• All unit tests pass (uv run pytest tests/unit/ -q)
• Workspace e2e tests pass (uv run pytest tests/e2e/test_workspace_*.py -v)
• Server e2e tests pass (uv run pytest tests/e2e/server/ -v)
• All 6 demo-smoke pass
• uv run release gate passes

[github-actions]

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this PR is refactoring server initialization around a new workspace abstraction and quietly changes config conventions, but the compatibility and concurrency story is not convincingly locked down.

One-way doors

1. Standardizing on synix.toml as the workspace/server config file

   • Hard to reverse because templates, container images, docs, and user automation will start depending on this filename and mixed workspace/server semantics.
   • Safe to merge only if the project is committed to synix.toml as the long-term public config surface and has explicit backward-compat loading for existing synix-server.toml users.

2. Promoting Workspace to the server/runtime boundary

   • Hard to reverse because internal APIs (serve, viewer, MCP tools, tests) now assume a workspace object carries project + config + runtime services.
   • Safe to merge only if Workspace is intended as the canonical top-level abstraction across CLI/server/viewer, with clear invariants and thread-safety rules documented.

3. Embedding runtime state in a module-global _workspace

   • Hard to reverse because MCP tools and API handlers now reach into global mutable runtime state rather than explicit dependency injection.
   • Safe only if single-workspace-per-process is a deliberate product constraint and the team accepts testability/multi-tenant limitations.

Findings

1. src/synix/server/mcp_tools.py, module-global _workspace

   • You replaced a bad global dict with a slightly cleaner bad global object. This still couples request handlers, MCP tools, viewer, and build worker to shared mutable process state. It blocks running multiple workspaces in one process, makes tests order-dependent, and creates race potential if _workspace.project is swapped while requests are in flight.
   • Severity: [warning]

2. tests/e2e/server/test_server_e2e.py, mcp_tools._workspace._project = project

   • The tests are mutating a private field directly to “refresh” the project. That is a design smell, not just a test smell. If the only way to update release visibility is to monkeypatch internals, the abstraction is not real.
   • Severity: [warning]

3. src/synix/server/serve.py, serve_from_config() partial overlay

   • The adapter overlays pipeline_path, buckets, auto_build, and vllm, but not workspace.name or server bindings from workspace config. You now have two config models (ServerConfig and WorkspaceConfig) with partial reconciliation logic. That drift will break in non-obvious ways.
   • Severity: [warning]

4. deploy/Containerfile + deploy/synix.toml, filename/config shape change

   • Container entrypoint now points at /data/synix.toml, but README/website/docs in the provided materials do not mention synix.toml as the server config contract. This is a user-facing behavior change with no documentation update. Also unclear whether load_config still accepts the new mixed file shape everywhere.
   • Severity: [warning]

5. src/synix/server/serve.py, viewer startup does not pass workspace

   • viewer.__init__ and ViewerState.from_workspace() were added specifically for workspace binding, but run_viewer() still calls viewer_serve(..., project=project) and never passes workspace=workspace. So the new workspace-aware viewer path is mostly dead code, and /api/status won’t expose workspace identity as intended.
   • Severity: [warning]

6. src/synix/viewer/server.py, try_discover_release() reopens project instead of using workspace

   • Even in workspace mode, it throws away the bound workspace and reopens a project from disk. That leaks implementation details and risks stale/mismatched state versus the active runtime. If workspace is the abstraction, use it.
   • Severity: [minor]

7. src/synix/server/api.py and mcp_tools.py, repeated if _workspace and _workspace.runtime else None

   • This is scattered defensive access to optional runtime state instead of a stable runtime contract. It increases the chance of partial initialization bugs and inconsistent 503 behavior across endpoints.
   • Severity: [minor]

Missing

• Backward-compat tests proving old synix-server.toml still works, especially for existing deployments.
• Documentation updates in README/website/server docs explaining synix.toml, workspace config layout, and migration path.
• Tests for concurrent request/build behavior with global _workspace.
• Validation/error handling for malformed synix.toml with mixed [workspace] and server sections.
• A clear public API decision: is Workspace now the primary entrypoint, or is this still internal?

Verdict — Ship with fixes: the refactor direction may be right, but the config migration is under-documented, the viewer integration is incomplete, and the global runtime state remains an architectural liability.

Review parameters

• Model: gpt-5.4
• Context: README.md, DESIGN.md, synix.dev, PR diff
• Diff size: 1,536 lines
• Prompt: .github/prompts/openai_review.md
• Timestamp: 2026-04-06T03:54:45Z

[github-actions]

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR replaces the ad-hoc _state: dict global in the server module with a typed Workspace object that owns the project, config, and runtime services (queue, prompt store, build lock, vLLM). It also separates network bindings (ServerBindings) from domain config, renames the deploy config to synix.toml with a [workspace] section, and adds synix.toml to all templates.

Alignment

Strong fit. DESIGN.md establishes that a Pipeline is the core container and .synix/ is the single source of truth. The Workspace abstraction makes the "open a project directory, discover its config, load its pipeline, build, release, serve" lifecycle into a first-class object rather than scattered dictionary mutations. This is consistent with the build-system mental model — the workspace is the working tree. Separating ServerBindings (ports, hosts) from WorkspaceConfig (pipeline, buckets, build queue) correctly distinguishes deployment topology from build semantics, which aligns with the build/release separation principle.

Observations

1. [positive] Eliminating _state: dict removes a class of bugs where keys are misspelled or missing. Every access through _workspace.runtime.queue is now statically checkable and IDE-navigable.

2. [positive] serve_from_config() as a backward-compat adapter is clean — the CLI doesn't change, but the internal contract is now typed.

3. [concern] Multiple places use _workspace.runtime.queue if _workspace and _workspace.runtime else None — this 12-character guard clause is repeated ~10 times across mcp_tools.py and api.py. A helper method like _workspace.queue_or_none() or a property on Workspace would reduce duplication and risk of inconsistency.

4. [concern] In test_server_e2e.py:296, mcp_tools._workspace._project = project directly mutates a private attribute to swap the project mid-test. This bypasses any invariants Workspace might enforce. If Workspace ever caches state derived from the project, this will silently break.

5. [question] The Workspace constructor in tests (Workspace(project, config)) doesn't appear in the diff's workspace.py changes — is this an existing constructor? The test helper _make_workspace in conftest.py constructs one directly, but also sets ws._project.project_root = tmp_path on the mock, suggesting the abstraction boundary is leaky.

6. [positive] New test_workspace_serve.py covers runtime activation, queue/prompt store integration, build-and-release through workspace, and config roundtrip. Good coverage of the new surface area.

7. [nit] The deploy synix.toml adds [workspace] with name and pipeline_path, but ServerConfig presumably still has pipeline_path too. The serve_from_config adapter overwrites ws_config.pipeline_path = config.pipeline_path, which means the TOML value is silently ignored when launched via CLI. This could confuse someone editing the TOML expecting it to take effect.

8. [positive] All 8 templates now ship with synix.toml, making workspace identity consistent from project init. Names match template slugs — clear and predictable.

9. [nit] Formatting-only changes (textwrap.dedent call style, black reformatting of tuple literals) are mixed into the functional diff, making review harder. Consider separating format-only commits.

10. [question] run_viewer now takes two positional args (workspace, bindings) but is called via loop.run_in_executor(None, run_viewer, workspace, bindings). This works with run_in_executor for positional args, but is worth verifying — some executor implementations don't forward multiple args cleanly.

Verdict

This is a well-motivated structural improvement that replaces stringly-typed shared state with a typed domain object, consistent with the project's build-system architecture — approve with a request to extract the repeated _workspace.runtime.X if _workspace and _workspace.runtime else None guard into a helper.

Review parameters

• Model: claude-opus-4-6
• Context: README.md, DESIGN.md, synix.dev, PR diff
• Diff size: 1,536 lines
• Prompt: .github/prompts/claude_review.md
• Timestamp: 2026-04-06T03:54:50Z

[github-actions]

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this is a cross-cutting config/runtime refactor that changes server boot semantics and introduces a new user-facing config file without enough migration proof.

One-way doors

1. Standardizing on synix.toml as the workspace/server config name
   Hard to reverse because templates, container images, docs, and user habits will lock onto this filename and schema. Safe to merge only if the loader supports both old and new names with clear precedence, deprecation messaging, and migration docs/tests.

2. Making Workspace the runtime authority for serving
   This is architectural, not cosmetic. Once other modules start depending on workspace.runtime and workspace.config, backing it out gets ugly. Safe only if Workspace is explicitly the intended public/domain abstraction in docs/design, with tests covering all server/runtime lifecycles.

Findings

1. src/synix/server/serve.py + deploy/Containerfile/deploy/synix.toml: config rename without compatibility path
   The container now only copies synix.toml, and serve_from_config() opens a workspace from config.project_dir, but I see no evidence here that server startup still discovers legacy synix-server.toml. This risks breaking existing deployments and contradicts the README/current docs, which never mention this config-file migration. Severity: [critical]

2. src/synix/server/mcp_tools.py: module-global _workspace still exists
   This claims to replace _state, but it’s still mutable process-global state. That means test pollution, inability to safely host multiple workspaces in one process, and race-prone behavior if the server evolves beyond single-instance assumptions. This is not real decoupling; it just moved the bag of globals. Severity: [warning]

3. src/synix/server/serve.py::serve_from_config: partial overlay from ServerConfig to WorkspaceConfig
   You copy pipeline_path, buckets, auto_build, and vllm, but not workspace name or server bindings because those live elsewhere. Now there are two config models with overlapping responsibility and ad hoc reconciliation logic. That is hidden complexity and a drift vector. Severity: [warning]

4. src/synix/viewer/server.py::try_discover_release: ignores bound workspace for release access
   Even when a workspace is bound, it re-opens a project from disk via synix.open_project(...) instead of using workspace.release(...) consistently. That bypasses the abstraction this PR is introducing and risks stale/duplicated state behavior. Severity: [warning]

5. tests/e2e/server/test_server_e2e.py: test mutates mcp_tools._workspace._project directly
   That is a code smell, not just a test detail. If the only way to refresh state in tests is to mutate a private field, the runtime abstraction is leaky or missing an API. Severity: [warning]

6. src/synix/server/api.py and mcp_tools.py: runtime service lookup has no boundary validation
   _get_runtime_service(attr) uses getattr(..., None) on arbitrary strings. Today callers are internal, but this is a weak pattern for a central runtime registry. Typos silently degrade into “not initialized” instead of failing fast. Severity: [minor]

7. templates/*/synix.toml: user-facing behavior changed without matching docs updates
   Templates now include synix.toml, which is a new project convention, but README and website snippets still present projects as pipeline.py + .env + sources/ with no mention of workspace config. That’s a mental-model mismatch. Severity: [warning]

8. src/synix/server/serve.py: shared mutable pipeline config mutation in build worker
   project._pipeline.llm_config = llm_override mutates pipeline state at runtime before builds. If multiple builds/threads/processes ever touch the same project, this becomes order-dependent state mutation. Pre-1.0 doesn’t excuse making concurrency harder later. Severity: [warning]

Missing

• Tests proving backward compatibility with synix-server.toml for existing server deployments.
• Tests for precedence when both synix.toml and old server config exist.
• Documentation updates in README/website/integration docs explaining synix.toml, workspace semantics, and migration from old server config.
• Validation tests for malformed or missing [workspace] sections in template/container scenarios.
• Evidence that Workspace is now the intended design-level abstraction; DESIGN.md still centers Pipeline/Project, not Workspace.

Verdict — Block: the runtime refactor may be directionally fine, but the config migration and global-state rewrite are under-specified, under-documented, and not convincingly backward-compatible.

Review parameters

• Model: gpt-5.4
• Context: README.md, DESIGN.md, synix.dev, PR diff
• Diff size: 1,545 lines
• Prompt: .github/prompts/openai_review.md
• Timestamp: 2026-04-06T03:59:33Z

[github-actions]

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

Replaces the ad-hoc _state dictionary in the server module with a first-class Workspace object that owns the project, config, and runtime services (queue, prompt store, build lock, vLLM). Splits network binding concerns into a separate ServerBindings dataclass. Adds synix.toml to all templates and renames the deploy config to match.

Alignment

Strong fit. DESIGN.md describes the pipeline as a "named container for memory processing" and the build/release separation as absolute. The Workspace abstraction formalizes the container that was previously an implicit bag of globals — it now owns the project, config, state machine, and runtime services in a single coherent object. The ServerBindings split correctly separates deployment concerns (ports, hosts) from domain concerns (pipeline, buckets, build queue), which aligns with the build-system mental model where configuration of what to build is separate from how to serve it. The synix.toml addition to templates makes the "workspace as first-class concept" concrete for new users from synix init.

Observations

1. [positive] _state → _workspace eliminates a stringly-typed global dict with no schema. Every access is now typed and discoverable. _get_runtime_service(attr) uses getattr with a default of None, preserving the graceful-degradation behavior while being explicit.

2. [positive] serve_from_config() as a backward-compat adapter is clean. The CLI entry point calls it, while tests and internal code can use the real serve(workspace, bindings) directly. Good incremental migration.

3. [concern] In test_server_e2e.py:296, the test does mcp_tools._workspace._project = project — directly mutating a private attribute on the workspace. This couples the test to internal structure and suggests Workspace may need a method to rebind its project (e.g., after re-opening to pick up new refs). If this pattern is needed in production later, a public API would be better.

4. [concern] _get_runtime_service uses getattr(workspace.runtime, attr, None). If someone passes a typo like "queu", it silently returns None instead of failing. A whitelist or enum would catch this at development time. The old _state.get() had the same problem, but the refactor was a chance to fix it.

5. [question] serve_from_config manually overlays config.pipeline_path, config.buckets, etc. onto the workspace config. If ServerConfig gains new fields, this overlay must be updated in lockstep. Is there a reason not to have WorkspaceConfig accept a ServerConfig directly, or share a common base?

6. [nit] The deploy rename from synix-server.toml to synix.toml adds a [workspace] section with name and pipeline_path. This is good — it means the container deployment and local development use the same config format. But the deploy config still has server-specific sections ([server], [buckets], [vllm]) that are irrelevant to synix build. No issue today, but worth a doc comment.

7. [positive] All 8 templates now ship with synix.toml. This means open_workspace() will always find a config after synix init, which is a better UX than falling back to directory-name heuristics.

8. [positive] New test_workspace_serve.py covers runtime activation, queue/prompt integration, build-through-workspace, and config round-tripping. Good coverage of the new surface area.

9. [nit] Formatting-only changes (trailing whitespace, textwrap.dedent call style) are mixed in with behavioral changes. These are harmless but add noise to the diff.

10. [question] run_viewer no longer imports synix at the top — it was removed. But the workspace path still calls _synix.open_project() inside try_discover_release. Is re-opening the project still necessary when the workspace already holds a reference?

Verdict

This is a well-structured refactor that replaces implicit global state with an explicit domain object, aligns with the project's build-system identity, and comes with solid test coverage — a good incremental step.

Review parameters

• Model: claude-opus-4-6
• Context: README.md, DESIGN.md, synix.dev, PR diff
• Diff size: 1,545 lines
• Prompt: .github/prompts/claude_review.md
• Timestamp: 2026-04-06T03:59:46Z