fix(cli-lib): I1+I2+I3 review fixes — token-precedence note, sync-state TODO, dry-run hermeticity test

This commit is contained in:
ZdenekSrotyr 2026-05-04 18:04:56 +02:00
parent 37da602060
commit 15004126de
2 changed files with 17 additions and 0 deletions

View file

@ -72,6 +72,15 @@ def _override_server_env(server_url: str, token: str) -> Iterator[None]:
`cli.config.get_server_url` / `get_token` already honor these env vars, `cli.config.get_server_url` / `get_token` already honor these env vars,
which is the same mechanism used in production. Restores prior values which is the same mechanism used in production. Restores prior values
on exit so the caller's environment isn't mutated permanently. on exit so the caller's environment isn't mutated permanently.
Caveats:
- **Token override is honored only when no `~/.config/agnes/token.json`
exists.** `get_token()` reads the file first and only falls back to
`AGNES_TOKEN`. `agnes init` writes `token.json` before calling
`run_pull` so the values agree in production; isolated tests/callers
that pass a different token must clear the on-disk token first.
- **Not safe for concurrent invocation in the same process** env-var
swap is global. Single-threaded use only.
""" """
prev_server = os.environ.get("AGNES_SERVER") prev_server = os.environ.get("AGNES_SERVER")
prev_token = os.environ.get("AGNES_TOKEN") prev_token = os.environ.get("AGNES_TOKEN")
@ -175,6 +184,11 @@ def run_pull(
result.errors.append({"table": tid, "error": str(exc)}) result.errors.append({"table": tid, "error": str(exc)})
# 5. Persist sync state (only on real runs). # 5. Persist sync state (only on real runs).
# TODO(workspace-scoped-sync-state): currently saved to
# ~/.config/agnes/sync_state.json (per legacy sync.py behavior).
# Two workspaces sharing one user account share this state.
# Future: scope to <workspace>/.agnes/sync_state.json so workspace
# bootstrap leaves no residue outside <workspace>/.
local_state["tables"] = local_tables local_state["tables"] = local_tables
local_state["last_sync"] = datetime.now(timezone.utc).isoformat() local_state["last_sync"] = datetime.now(timezone.utc).isoformat()
save_sync_state(local_state) save_sync_state(local_state)

View file

@ -100,3 +100,6 @@ def test_run_pull_dry_run_writes_nothing(tmp_path, fake_server):
run_pull(server_url="http://x", token="t", workspace=tmp_path, dry_run=True) run_pull(server_url="http://x", token="t", workspace=tmp_path, dry_run=True)
assert not (tmp_path / "server").exists() assert not (tmp_path / "server").exists()
assert not (tmp_path / "user" / "duckdb").exists() assert not (tmp_path / "user" / "duckdb").exists()
# No user-home state file either — dry_run must be hermetic.
# The autouse fixture sandboxes AGNES_CONFIG_DIR to tmp_path/_agnes_cfg.
assert not (tmp_path / "_agnes_cfg" / "sync_state.json").exists()