Skip to content

Commit e2ca8e2

Browse files
david-siqi-liuIsaac
andcommitted
Split managed state into draft and published slots
`~/.ucode/managed-state.json` held one config per workspace, written both by `refresh_managed_config` (the copy fetched from the workspace on every launch) and by the authoring wizard (the admin's local, unpublished draft). The two clobbered each other: a launch wiped an in-progress draft, and a launch could apply an unpublished draft as if it were published policy. Store both under a versioned per-workspace map with separate `draft` and `published` slots. A v1 file migrates on read, with its original bytes kept at `managed-state.json.pre-v2.bak` on the next write. The map is written through a sibling temp file and renamed into place, because it now holds the admin's draft: nothing can refetch that, so a torn write would lose it outright. `load_managed_state` and `save_managed_state` stay as thin wrappers over the published slot so existing callers keep working; a follow-up moves them over. Co-authored-by: Isaac <no-reply@databricks.com>
1 parent ca28ce3 commit e2ca8e2

3 files changed

Lines changed: 211 additions & 37 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ control the installation.
387387
| `~/.pi/agent/models.json` | Pi |
388388
| `~/.cursor/mcp.json` | Cursor Agent (MCP servers only) |
389389
| `~/.ucode/managed-state.json` | The managed config — authored by `ucode setup` (admins) and refreshed from the workspace on launch |
390+
| `~/.ucode/managed-state.json.pre-v2.bak` | One-time copy of a pre-slots `managed-state.json`, kept when it is first migrated |
390391
| `~/.ucode/managed-backups/` | Baseline backups for OS-managed files changed by ucode |
391392

392393
Existing files are backed up before being overwritten. `ucode revert` restores backups.

src/ucode/managed_config.py

Lines changed: 126 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@
3636
fetch_model_recommendation,
3737
get_databricks_token,
3838
)
39-
from ucode.ui import console, print_warning
39+
from ucode.ui import print_warning
4040

4141
MANAGED_STATE_PATH = config_io.APP_DIR / "managed-state.json"
4242

43+
MANAGED_STATE_VERSION = 2
44+
4345
# Opt-in switch while the feature is in bug bash: unset means launches ignore managed configs
4446
# entirely and behave exactly as they did before.
4547
MANAGED_CONFIG_ENV_VAR = "ENABLE_MANAGED_AGENT_CONFIG"
@@ -342,29 +344,96 @@ def _is_permission_denied(reason: str) -> bool:
342344
return "http 403" in lowered or "permission_denied" in lowered
343345

344346

345-
def save_managed_state(workspace: str, config: dict) -> None:
346-
"""Persist the normalized managed config to ``~/.ucode/managed-state.json`` at mode 0600.
347+
def _migrated_workspaces(data: dict) -> dict[str, dict]:
348+
"""Return the ``workspaces`` map from a raw ``managed-state.json`` dict, migrating v1 in-memory.
349+
350+
v1 stored a single ``{workspace, config}`` slot shared by both the admin's authored draft and the
351+
launch-fetched published copy, so a legacy value's provenance can't be recovered. It is migrated
352+
into the ``published`` slot — the common case is a developer's fetched snapshot, and treating it
353+
as published keeps ``ucode export`` / ``ucode publish`` (now strictly draft-only) from mistaking a
354+
cached fetch for authored work. A one-time backup (see :func:`_backup_legacy_file_once`) makes a
355+
rare unpublished admin draft recoverable. Read-only: never writes.
356+
"""
357+
if data.get("version") == MANAGED_STATE_VERSION and isinstance(data.get("workspaces"), dict):
358+
return {k: v for k, v in cast("dict", data["workspaces"]).items() if isinstance(v, dict)}
359+
workspaces: dict[str, dict] = {}
360+
legacy_ws = data.get("workspace")
361+
if isinstance(legacy_ws, str) and legacy_ws:
362+
legacy_cfg = data.get("config")
363+
workspaces[legacy_ws] = {"published": legacy_cfg if isinstance(legacy_cfg, dict) else {}}
364+
return workspaces
365+
366+
367+
def _is_legacy_file(data: dict) -> bool:
368+
"""True when ``data`` is a pre-v2 ``{workspace, config}`` file (not the versioned map)."""
369+
return data.get("version") != MANAGED_STATE_VERSION and "workspace" in data
370+
371+
372+
def _backup_legacy_file_once() -> None:
373+
"""Copy a pre-v2 ``managed-state.json`` to ``<path>.pre-v2.bak`` before it is overwritten.
374+
375+
Best-effort and idempotent: migration maps the single legacy slot into ``published``, which can't
376+
preserve a rare unpublished admin draft, so the original bytes are kept once for recovery. Written
377+
through a temp file so an interrupted copy cannot leave a truncated backup that the ``exists()``
378+
guard would then treat as the original."""
379+
backup = MANAGED_STATE_PATH.with_suffix(MANAGED_STATE_PATH.suffix + ".pre-v2.bak")
380+
if backup.exists() or not MANAGED_STATE_PATH.exists():
381+
return
382+
if not _is_legacy_file(config_io.read_json_safe(MANAGED_STATE_PATH)):
383+
return
384+
tmp = backup.with_name(backup.name + ".tmp")
385+
try:
386+
tmp.write_bytes(MANAGED_STATE_PATH.read_bytes())
387+
_restrict_permissions(tmp)
388+
os.replace(tmp, backup)
389+
except OSError:
390+
pass
391+
392+
393+
def _save_slot(workspace: str, slot: str, config: dict) -> None:
394+
"""Write ``config`` to ``workspace``'s ``published`` or ``draft`` slot, preserving everything else.
347395
348-
The file is org-authored, not developer-editable — 0600 keeps it readable/writable only by the
349-
user (a light guard; hard enforcement / sudo ownership is a separate concern). No-op in dry-run.
396+
Reads the current file, migrates it to the v2 map, sets one slot for one workspace, and writes it
397+
back — so a refresh of the published slot never disturbs an admin's draft (or other workspaces).
398+
0600 keeps the org-authored file readable/writable only by the user. No-op write in dry-run.
350399
351-
An empty ``config`` records "this workspace has no managed config", which matters because the
352-
file doubles as the fallback when a later read fails: without it, removing a config server-side
353-
would leave the old one on disk to be reapplied after a transient outage.
400+
An empty ``config`` in the ``published`` slot records "this workspace has no managed config",
401+
which matters because that slot doubles as the fallback when a later read fails: without it,
402+
removing a config server-side would leave the old one on disk to be reapplied after an outage.
354403
"""
355-
payload = {"workspace": workspace, "config": config}
404+
workspaces = _migrated_workspaces(config_io.read_json_safe(MANAGED_STATE_PATH))
405+
workspaces[workspace] = {**workspaces.get(workspace, {}), slot: config}
406+
payload = {"version": MANAGED_STATE_VERSION, "workspaces": workspaces}
356407
if config_io.is_dry_run():
357-
# Print rather than write, matching how the agent config writers behave under --dry-run.
358-
console.print(
359-
f"\n[bold]\\[dry run] {MANAGED_STATE_PATH}[/bold]\n{json.dumps(payload, indent=2)}\n"
360-
)
408+
config_io.write_json_file(MANAGED_STATE_PATH, payload)
361409
return
362-
config_io.ensure_parent_dir(MANAGED_STATE_PATH)
410+
_backup_legacy_file_once()
411+
_write_state_atomically(payload)
412+
413+
414+
def _write_state_atomically(payload: dict) -> None:
415+
"""Write the state map through a sibling temp file and rename it into place.
416+
417+
The map holds the admin's draft, which nothing can rebuild: a torn write would take the draft
418+
with it, where before v2 an interrupted write only cost a snapshot the next launch refetches.
419+
"""
420+
tmp = MANAGED_STATE_PATH.with_name(MANAGED_STATE_PATH.name + ".tmp")
421+
config_io.write_json_file(tmp, payload)
422+
_restrict_permissions(tmp)
363423
try:
364-
MANAGED_STATE_PATH.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8")
424+
os.replace(tmp, MANAGED_STATE_PATH)
365425
except OSError as exc:
366426
raise RuntimeError(f"Failed to write managed state file: {MANAGED_STATE_PATH}") from exc
367-
_restrict_permissions(MANAGED_STATE_PATH)
427+
428+
429+
def save_published_config(workspace: str, config: dict) -> None:
430+
"""Persist the workspace's launch-fetched ``published`` snapshot, leaving any ``draft`` intact."""
431+
_save_slot(workspace, "published", config)
432+
433+
434+
def save_draft_config(workspace: str, config: dict) -> None:
435+
"""Persist the admin's locally authored ``draft``, leaving the ``published`` snapshot intact."""
436+
_save_slot(workspace, "draft", config)
368437

369438

370439
def _restrict_permissions(path: Path) -> None:
@@ -376,34 +445,55 @@ def _restrict_permissions(path: Path) -> None:
376445
pass
377446

378447

379-
def load_managed_state(workspace: str | None) -> dict | None:
380-
"""Load the persisted managed config for ``workspace``, or None if absent/mismatched.
381-
382-
Returns the normalized config dict (the ``config`` field), only when the stored file is for the
383-
same workspace — so a stale file from another workspace is ignored rather than misapplied.
384-
385-
This is the single local managed config: ``ucode setup`` authors it here, ``ucode publish``
386-
publishes it, and a launch refreshes it from the workspace. The admin-authored draft and the
387-
pulled copy share one file because the workspace is the source of truth — to keep a draft,
388-
publish it with ``ucode publish``.
389-
"""
448+
def _load_slot(workspace: str | None, slot: str) -> dict | None:
449+
"""Return ``workspace``'s ``published`` or ``draft`` config, or None if absent."""
390450
if not workspace:
391451
return None
392-
data = config_io.read_json_safe(MANAGED_STATE_PATH)
393-
if data.get("workspace") != workspace:
394-
return None
395-
config = data.get("config")
452+
entry = _migrated_workspaces(config_io.read_json_safe(MANAGED_STATE_PATH)).get(workspace) or {}
453+
config = entry.get(slot)
396454
return config if isinstance(config, dict) else None
397455

398456

457+
def load_published_config(workspace: str | None) -> dict | None:
458+
"""Load the launch-fetched ``published`` snapshot for ``workspace``, or None if absent.
459+
460+
This is what the launch path overlays (:func:`ucode.managed_resolve.resolve_state`) and what
461+
``ucode status`` reports — the admin-defined config a developer actually runs under. A stale
462+
snapshot from a different workspace is ignored rather than misapplied.
463+
"""
464+
return _load_slot(workspace, "published")
465+
466+
467+
def load_draft_config(workspace: str | None) -> dict | None:
468+
"""Load the admin's locally authored, unpublished ``draft`` for ``workspace``, or None.
469+
470+
Only ``ucode configure`` (admin authoring) writes this, and only ``ucode export`` / ``ucode
471+
publish`` read it — never the launch path. A developer who has only ever fetched a published
472+
snapshot has no draft, which is why export/publish are draft-only rather than falling back to the
473+
fetched copy: a cached publication is not authored work.
474+
"""
475+
return _load_slot(workspace, "draft")
476+
477+
399478
def managed_state_workspace() -> str | None:
400-
"""The workspace the on-disk managed config was authored/pulled for, or None when there is none.
479+
"""The sole workspace recorded in ``managed-state.json``, or None when absent/ambiguous.
401480
402-
Lets a caller that has no workspace in local ucode state (e.g. ``ucode setup --show`` before
403-
``ucode configure``) still find the manifest on disk and report which workspace it belongs to.
481+
Lets a caller with no workspace in local ucode state still find the config on disk. With several
482+
workspaces recorded the answer is ambiguous, so it returns None and the caller reports that a
483+
workspace must be selected first.
404484
"""
405-
workspace = config_io.read_json_safe(MANAGED_STATE_PATH).get("workspace")
406-
return workspace if isinstance(workspace, str) and workspace else None
485+
workspaces = _migrated_workspaces(config_io.read_json_safe(MANAGED_STATE_PATH))
486+
return next(iter(workspaces)) if len(workspaces) == 1 else None
487+
488+
489+
def save_managed_state(workspace: str, config: dict) -> None:
490+
"""Deprecated alias for :func:`save_published_config`, kept while callers migrate to the slots."""
491+
save_published_config(workspace, config)
492+
493+
494+
def load_managed_state(workspace: str | None) -> dict | None:
495+
"""Deprecated alias for :func:`load_published_config`, kept while callers migrate to the slots."""
496+
return load_published_config(workspace)
407497

408498

409499
def refresh_managed_config(state: dict) -> tuple[dict | None, bool]:

tests/test_managed_config.py

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,15 @@
1313
import ucode.managed_config as mc_mod
1414
from ucode.managed_config import (
1515
get_managed_config,
16+
load_draft_config,
1617
load_managed_state,
18+
load_published_config,
1719
managed_state_workspace,
1820
normalize_managed_config,
1921
refresh_managed_config,
22+
save_draft_config,
2023
save_managed_state,
24+
save_published_config,
2125
)
2226
from ucode.managed_setup import serialize_managed_config
2327

@@ -246,7 +250,9 @@ def test_workspace_is_none_when_absent(self, _managed_path):
246250
def test_dry_run_writes_nothing(self, _managed_path, monkeypatch):
247251
# Under --dry-run the config writers print instead of touching disk, so a launch that
248252
# dry-runs an admin's authored draft never overwrites it.
249-
monkeypatch.setattr(config_io_mod, "is_dry_run", lambda: True)
253+
# Patch the flag itself rather than `is_dry_run`: the shared JSON writer reads the module
254+
# global directly.
255+
monkeypatch.setattr(config_io_mod, "_dry_run", True)
250256
save_managed_state("https://ws.example.com", {"default_agent": "claude"})
251257
assert not _managed_path.exists()
252258

@@ -268,6 +274,83 @@ def test_loaded_config_serializes_to_a_json_encodable_payload(self, _managed_pat
268274
assert json.loads(json.dumps(serialize_managed_config(loaded)))
269275

270276

277+
class TestDraftPublishedSeparation:
278+
"""The draft (admin-authored, unpublished) and published (fetched) slots are kept apart."""
279+
280+
@pytest.fixture(autouse=True)
281+
def _managed_path(self, tmp_path, monkeypatch):
282+
path = tmp_path / ".ucode" / "managed-state.json"
283+
monkeypatch.setattr(mc_mod, "MANAGED_STATE_PATH", path)
284+
return path
285+
286+
def test_slots_are_independent(self, _managed_path):
287+
ws = "https://ws.example.com"
288+
save_draft_config(ws, {"default_agent": "claude"})
289+
save_published_config(ws, {"default_agent": "codex"})
290+
assert load_draft_config(ws) == {"default_agent": "claude"}
291+
assert load_published_config(ws) == {"default_agent": "codex"}
292+
293+
def test_saving_published_preserves_the_draft(self, _managed_path):
294+
ws = "https://ws.example.com"
295+
save_draft_config(ws, {"default_agent": "claude"})
296+
save_published_config(ws, {"default_agent": "codex"})
297+
save_published_config(ws, {"default_agent": "gemini"})
298+
assert load_draft_config(ws) == {"default_agent": "claude"}
299+
300+
def test_refresh_never_clobbers_a_draft(self, _managed_path, monkeypatch):
301+
ws = "https://ws.example.com"
302+
save_draft_config(ws, {"default_agent": "claude", "enabled_agents": {"claude": {}}})
303+
monkeypatch.setattr(mc_mod, "get_databricks_token", lambda w, p: "tok")
304+
monkeypatch.setattr(
305+
mc_mod, "get_managed_config", lambda w, tok: ({"default_agent": "codex"}, None)
306+
)
307+
result, _ = refresh_managed_config({"workspace": ws})
308+
assert result == {"default_agent": "codex"}
309+
assert load_published_config(ws) == {"default_agent": "codex"}
310+
assert load_draft_config(ws) == {
311+
"default_agent": "claude",
312+
"enabled_agents": {"claude": {}},
313+
}
314+
315+
def test_refresh_of_one_workspace_keeps_another_workspaces_draft(
316+
self, _managed_path, monkeypatch
317+
):
318+
ws_a, ws_b = "https://a.example.com", "https://b.example.com"
319+
save_draft_config(ws_a, {"default_agent": "claude"})
320+
monkeypatch.setattr(mc_mod, "get_databricks_token", lambda w, p: "tok")
321+
monkeypatch.setattr(
322+
mc_mod, "get_managed_config", lambda w, tok: ({"default_agent": "codex"}, None)
323+
)
324+
refresh_managed_config({"workspace": ws_b})
325+
assert load_draft_config(ws_a) == {"default_agent": "claude"}
326+
327+
def test_a_failed_write_leaves_the_previous_state_intact(self, _managed_path, monkeypatch):
328+
ws = "https://ws.example.com"
329+
save_draft_config(ws, {"default_agent": "claude"})
330+
331+
def partial_write(path, payload):
332+
path.write_text('{"version": 2, "workspa', encoding="utf-8")
333+
raise RuntimeError("disk full")
334+
335+
monkeypatch.setattr(mc_mod.config_io, "write_json_file", partial_write)
336+
with pytest.raises(RuntimeError):
337+
save_published_config(ws, {"default_agent": "codex"})
338+
assert load_draft_config(ws) == {"default_agent": "claude"}
339+
340+
def test_legacy_file_migrates_into_the_published_slot_with_a_backup(self, _managed_path):
341+
ws = "https://ws.example.com"
342+
_managed_path.parent.mkdir(parents=True, exist_ok=True)
343+
legacy = {"workspace": ws, "config": {"default_agent": "claude"}}
344+
_managed_path.write_text(json.dumps(legacy), encoding="utf-8")
345+
assert load_published_config(ws) == {"default_agent": "claude"}
346+
assert load_draft_config(ws) is None
347+
backup = _managed_path.with_suffix(_managed_path.suffix + ".pre-v2.bak")
348+
assert not backup.exists()
349+
save_published_config(ws, {"default_agent": "codex"})
350+
assert backup.exists()
351+
assert json.loads(backup.read_text()) == legacy
352+
353+
271354
class TestFetchClient:
272355
"""fetch_managed_coding_agent_configs lives in databricks.py; test its response parsing."""
273356

0 commit comments

Comments
 (0)