docs(spec): apply round-1 review — orphan/wiring fixes

11 minimum edits + minor cleanups from first review pass:

- TTL field: add `ttl_seconds` alongside existing `expires_in_days` in
  PAT request, force-clamp to 1h for scope=bootstrap-analyst
- Drop layered per-workspace config (no defined producer; multi-instance
  is edge case) — moved to Open questions
- CLAUDE.md producer: explicit `GET /api/welcome` flow, address admin
  DB-override migration
- URL: `/install` → `/setup?role=...` (matches existing routing; legacy
  `/install` keeps 302 redirect)
- `da metrics import` → relocated under `da admin metrics import`
- Repo-root CLAUDE.md added to rewritten files list
- CLI surface: enumerate `da explore`, `da disk-info`, `da snapshot
  refresh/prune`, full `da snapshot create` flag list, `da push` flags
- Test fixtures section: contracts for `fastapi_test_server`, `test_pat`,
  `test_pat_no_grants`, `zero_grants_workspace`, `web_session`, `client`
- Cleanups: line ref for `_install_claude_hooks`, admin-namespace
  disclaimer, `da init`-doesn't-call-`da auth login` note, audit-log
  consumer note for PAT scope, `{workspace_path}` placeholder usage
This commit is contained in:
ZdenekSrotyr 2026-05-04 14:51:54 +02:00
parent 8cbf0aa818
commit 973e96e6af

View file

@ -1,6 +1,6 @@
# Clean analyst bootstrap — design
**Date:** 2026-05-04
**Date:** 2026-05-04 (revision 2 — incorporates first round of orphan/wiring review)
**Branch:** `zs/clean-analyst-bootstrap-spec`
**Status:** Draft (approved by user, pre-implementation)
**Successor to:** today's `da analyst setup` flow (interactive email/password) and the empty-folder bug under `da sync`.
@ -10,11 +10,11 @@
A new analyst should be able to:
1. Sign in to the Agnes web UI.
2. Click a button on `/install?role=analyst`, copy a single Claude-Code-paste prompt to the clipboard.
2. Click a button on `/setup?role=analyst`, copy a single Claude-Code-paste prompt to the clipboard.
3. In an empty terminal, in an empty folder, paste the prompt into Claude Code.
4. Have Claude Code do **all** of the local setup — install the `da` CLI, trust the server's TLS cert (when needed), authenticate, generate `CLAUDE.md`, install Claude Code hooks, pull the RBAC-allowed parquets, build the local DuckDB views, write a human-readable workspace docs file.
5. Immediately start asking questions about the data — without ever typing a follow-up command.
6. From the second session onwards, have data freshness handled automatically by hooks (no `da sync` ever typed by hand).
6. From the second session onwards, have data freshness handled automatically by hooks (no `da pull` ever typed by hand).
Today this flow does not exist. The closest piece (`da analyst setup` in `cli/commands/analyst.py`) is interactive (prompts for email + password), produces a workspace layout that does not match what `da sync` later writes (the `data/parquet/`, `data/duckdb/`, `data/metadata/`, `user/artifacts/` directories it creates are never read by anything; `da sync` writes parquets to a sibling `server/parquet/` and DuckDB to `user/duckdb/analytics.duckdb`), and never registers the SessionStart/End hooks unless the analyst already managed to authenticate.
@ -24,30 +24,31 @@ In addition, `da sync` itself creates empty directories (`.claude/rules/` is `mk
- Single web → paste → done UX. Zero interactive prompts in the CLI bootstrap path.
- Workspace contains only files that something writes intentionally. No empty directories, no unread caches.
- Reader commands (`query`, `catalog`, `schema`, `describe`, `snapshot list`, `disk-info`, `status`, `diagnose`, `auth whoami`, `fetch --estimate`) survive a freshly-bootstrapped workspace with zero table grants and zero corporate-memory rules without crashing.
- Reader commands (`query`, `catalog`, `schema`, `describe`, `snapshot list`, `disk-info`, `status`, `diagnose`, `auth whoami`, `explore`) survive a freshly-bootstrapped workspace with zero table grants and zero corporate-memory rules without crashing.
- CLI verbs are mnemonic and non-overlapping. No two commands mean almost the same thing.
- The clean-install behavior is testable: integration tests that boot a real FastAPI server fixture, run the bootstrap end-to-end against `tmp_path`, and assert the exact set of files created.
## Non-goals
- Migration of existing analyst workspaces. This is a greenfield rewrite — no `data/parquet/` deprecation aliases, no `da analyst setup` shim that calls the new code. Any analyst running today's setup gets a different workspace shape; their old files are dead but harmless. Documented in CHANGELOG `**BREAKING**`.
- Admin CLI rewrite. The `/install?role=admin` path keeps its current shape (TLS bootstrap, marketplace + plugins + skills + diagnose). Admin onboarding is unrelated.
- Admin CLI rewrite. The `/setup?role=admin` path keeps its current shape (TLS bootstrap, marketplace + plugins + skills + diagnose). Admin onboarding is unrelated.
- Offline initial bootstrap.
- PAT auto-refresh / refresh tokens.
- Multi-user / shared workspace.
- Web UI redesign. We add `?role=` query branching; no visual redesign.
- Layered per-workspace config (`<cwd>/.agnes/{config.yaml,token.json}`). Considered during brainstorming, dropped: multi-instance support is an edge case and the producer is undefined (no command currently creates `<cwd>/.agnes/`). Captured as a follow-up.
## Architecture overview
```
┌──────────────────────────────┐
│ Web /install?role=analyst
│ Web /setup?role=analyst
│ │ paste prompt
│ user logs in (web session) ├────────────────────┐
│ clicks "Generate prompt" │ │
│ → POST /auth/tokens │ │
│ {scope:"bootstrap-analyst"│ │
│ ttl:3600} │ │
│ ttl_seconds:3600} │ │
│ → renders setup_instructions│ │
│ .render(role="analyst") │ │
└──────────────────────────────┘ │
@ -67,7 +68,10 @@ In addition, `da sync` itself creates empty directories (`.claude/rules/` is `mk
┌────────────────────────────────────────────┐
<cwd> — workspace, fully usable post-init │
│ ├── CLAUDE.md (Claude rails) │
│ ├── CLAUDE.md (Claude rails; │
│ │ fetched from GET /api/welcome — │
│ │ server-side render incl. admin │
│ │ DB-stored override) │
│ ├── AGNES_WORKSPACE.md (human docs) │
│ ├── .claude/ │
│ │ ├── settings.json (model, perms, │
@ -92,47 +96,63 @@ Single source of truth for data path: `da pull`. `da init` is a thin orchestrato
Single source of truth for the install prompt: `app/web/setup_instructions.py`. New `role: Literal["analyst", "admin"]` parameter branches the step list. TLS trust block is the only piece shared between the two roles.
Layered config (greenfield decision: hybrid C from brainstorming): `<cwd>/.agnes/config.yaml` and `<cwd>/.agnes/token.json` override `~/.config/da/`. Default is global; per-workspace override unlocks multi-instance use cases without touching the simple case.
Single source of truth for `CLAUDE.md` content: server-side `/api/welcome`. `da init` fetches the rendered text rather than rendering from a client-side template. This means admin-published overrides (DB-stored at `claude_md_template` table, exposed via `/api/admin/workspace-prompt-template`) automatically flow to all analysts. Server-side default template (`config/claude_md_template.txt` or equivalent rendering source) and any DB override **both** need their path strings updated as part of this PR — see "Migration of admin override" in Components.
Config and PAT live globally per user at `~/.config/da/{config.yaml,token.json}`. There is no per-workspace config in this design.
## New CLI surface
The CLI is rewritten with mnemonic, non-overlapping verbs. There are no backward-compat aliases. Today's `da analyst *`, `da sync`, `da sync --upload-only`, `da fetch` are removed.
The CLI is rewritten with mnemonic, non-overlapping verbs. There are no backward-compat aliases. Today's `da analyst *`, `da sync`, `da sync --upload-only`, `da fetch`, `da metrics`, `da skills` are removed from the analyst CLI.
```
da init one-time workspace bootstrap (--server-url, --token)
da pull refresh registered data (server → workspace)
da push upload sessions + notes (workspace → server)
WORKSPACE LIFECYCLE
da init one-time workspace bootstrap (--server-url, --token, --force, --workspace)
da pull refresh registered data (server → workspace) [--quiet, --json, --dry-run]
da push upload sessions + notes (workspace → server) [--quiet, --json, --dry-run]
da status what's in this workspace, when last synced
da query "SELECT ..." local DuckDB SQL
da query --remote "SELECT ..." server-side BQ passthrough
DATA QUERY
da query "SELECT ..." local DuckDB SQL (over server/parquet/* + user/snapshots/*)
da query --remote "SELECT ..." server-side BQ passthrough
da explore <view> interactive REPL over a single view
da disk-info snapshot disk usage summary
da catalog tables I have access to (RBAC-filtered)
da schema <table> columns + types
da describe <table> sample rows
DISCOVERY
da catalog tables I have access to (RBAC-filtered)
da catalog --metrics list metric definitions
da schema <table> columns + types
da describe <table> sample rows
da snapshot create <table> --where "..." --as <name>
da snapshot list
da snapshot drop <name>
SNAPSHOTS (ad-hoc remote materialization)
da snapshot create <table> --as <name> [--select ... --where ... --limit ... --order-by ... --estimate / --no-estimate --force]
da snapshot list
da snapshot drop <name>
da snapshot refresh <name> re-run the snapshot's saved query
da snapshot prune drop snapshots older than --older-than
da auth login interactive login (browser flow)
da auth import-token <PAT> non-interactive
da auth whoami
da auth logout
AUTH + IDENTITY
da auth login interactive login (browser flow). NOT called by da init.
da auth import-token <PAT> non-interactive
da auth whoami
da auth logout
da token create / list / revoke
da token create
da token list
da token revoke <id>
HEALTH
da diagnose health check (server + local)
da status what's in this workspace, when last synced
da diagnose health check (server + local)
ADMIN-ADJACENT (kept; not part of analyst flow)
da admin metrics import starter-pack import of metric definitions
da admin <other> existing admin verbs continue unchanged
```
Removed:
- `da analyst setup`, `da analyst status` — the `analyst` namespace had only one user role; the namespace is dead weight. Replaced by top-level `da init` and `da status`.
- `da sync` (and `--upload-only`) — split into `da pull` + `da push` for git-flavored mnemonics. Hook commands rename accordingly.
- `da fetch` — folded into `da snapshot create` so snapshots are a self-contained group (`create/list/drop`).
- `da metrics` as a separate namespace — folded into `da catalog --metrics` (low-traffic, doesn't deserve its own group).
- `da skills` — admin/dev tool, removed from the analyst CLI surface entirely.
- `da analyst setup`, `da analyst status``analyst` namespace had only one user role; replaced by top-level `da init` + `da status`.
- `da sync` (and `--upload-only`) — split into `da pull` + `da push`. Hook commands rename accordingly.
- `da fetch` — folded into `da snapshot create` with all flags carried over (`--select`, `--where`, `--limit`, `--order-by`, `--as`, `--estimate`, `--no-estimate`, `--force`).
- `da metrics list/show` — folded into `da catalog --metrics` (low-traffic; doesn't deserve its own namespace). `da metrics import` moves to `da admin metrics import` (admin-only operation).
- `da skills` — admin/dev tool; removed from the analyst CLI surface entirely.
Reader commands explicitly listed in the surface above (`da explore`, `da disk-info`, `da snapshot refresh`, `da snapshot prune`) survive unchanged — flagged here because the first review round found them missing from earlier drafts.
## Components
@ -141,38 +161,42 @@ Removed:
| Component | File | Change |
|---|---|---|
| Analyst install-prompt branch | `app/web/setup_instructions.py` | Add `role: Literal["analyst","admin"]="admin"` to `resolve_lines()` and `render_setup_instructions()`. Analyst layout: TLS trust block (reused, when `ca_pem` supplied) → install `da` (reused) → `da init --server-url X --token Y --workspace .``da catalog` smoke verify → confirm. Drop for analyst: marketplace, plugins, skills, diagnose, login, whoami (all subsumed by `da init`). |
| `/install?role=...` query branching | `app/web/router.py:723` | Read `role` query param, default `"admin"`. Pass to `render_setup_instructions(role=...)`. |
| `install.html` UI | `app/web/templates/install.html` | Two role tiles: "Analyst workspace" / "Admin CLI". PAT mint button per tile, posts to `/auth/tokens` with `scope` matching the tile. Renders the prompt for the selected role. |
| PAT scope + TTL clamp | `app/api/tokens.py` | Add `scope` field to PAT-create request body (default `"general"`). For `scope="bootstrap-analyst"` the server force-clamps `ttl_seconds <= 3600` regardless of request. Audit-log entry includes the scope. |
| `/setup?role=...` query branching | `app/web/router.py` `setup_page` (line 717) | Read `role` query param, default `"admin"`. Pass to `render_setup_instructions(role=...)`. Existing `/install` 302 redirect to `/setup` is preserved (legacy bookmarks keep working). |
| `setup.html` UI | `app/web/templates/setup.html` (or wherever `setup_page` renders) | Two role tiles: "Analyst workspace" / "Admin CLI". PAT mint button per tile, posts to `/auth/tokens` with `scope` matching the tile. Renders the prompt for the selected role. |
| PAT scope + TTL clamp | `app/api/tokens.py` (`CreateTokenRequest` Pydantic model + `create_token` route) | Add two fields: `scope: str = "general"` and `ttl_seconds: int \| None = None` (alongside the existing `expires_in_days`). Resolution: when `ttl_seconds` is set, it wins; otherwise fall back to `expires_in_days`. For `scope == "bootstrap-analyst"`, server force-clamps the resolved TTL to ≤ 3600 s regardless of request. Audit-log entry includes the scope. The audit log is the only consumer of `scope` in this PR — per-endpoint enforcement is an explicit follow-up. |
| Server-side template rewrite | `config/claude_md_template.txt` (or wherever `render_claude_md` reads its default from) | Update path strings: `data/parquet/``server/parquet/`, `data/duckdb/...``user/duckdb/analytics.duckdb`. Replace `da sync``da pull`, `da fetch``da snapshot create`, `da metrics list``da catalog --metrics`. |
| Admin override migration | `claude_md_template` DB table | Any admin-saved override may contain legacy strings. Add a one-shot migration step (admin UI banner + manual review) to flag overrides that contain `data/parquet`, `da sync`, `da fetch`, etc. Migration is **not** automatic — admins must re-author overrides. Document in CHANGELOG. |
| `/api/welcome` content unchanged | `app/api/claude_md.py:91` (`get_welcome`) | No code change — endpoint already serves rendered CLAUDE.md. Spec calls it out so implementer knows `da init`'s producer is here, not in the client. |
### Client-side (CLI, Python)
| Component | File | Change |
|---|---|---|
| `da init` (new) | `cli/commands/init.py` (new) | Required args: `--server-url`, `--token`. Optional: `--force`, `--workspace` (default `cwd`). Steps: (1) verify server reachability + PAT validity via `/api/health` with `Authorization: Bearer <PAT>`; (2) save server URL + PAT to layered config (per-workspace if `<cwd>/.agnes/` exists, else global `~/.config/da/`); (3) write `CLAUDE.md` from server-side template; (4) write `.claude/settings.json` (model, permissions, hooks pointing at `da pull` and `da push`); (5) write `.claude/CLAUDE.local.md` (stub, only if absent); (6) call `da pull` programmatically (refactor below); (7) write `AGNES_WORKSPACE.md` from server-side template with timestamp + server URL substituted. |
| `da pull` (renamed from `da sync`) | `cli/commands/pull.py` (renamed from `cli/commands/sync.py`) | Behavior is today's `da sync` minus the `--upload-only` branch. Lazy-mkdir fixes (see below). Refactor `main()` so `da init` can call it programmatically without a Typer wrapper. |
| `da push` (extracted from `da sync --upload-only`) | `cli/commands/push.py` (new) | Uploads `user/sessions/*.jsonl` and `.claude/CLAUDE.local.md`. Lazy: skip when nothing to upload. Same auth as `da pull`. |
| `da snapshot create` (renamed from `da fetch`) | `cli/commands/snapshot.py` | Move logic from `cli/commands/fetch.py` into a `create` subcommand of the existing `snapshot` group. Remove `cli/commands/fetch.py`. Add existence check before opening DuckDB to avoid creating an empty DB file when no `da pull` has run yet (guard: `if not db_path.exists(): typer.echo("Local DuckDB not found. Run: da pull"); raise typer.Exit(1)`). |
| `da init` (new) | `cli/commands/init.py` (new) | Required args: `--server-url`, `--token`. Optional: `--force`, `--workspace` (default `cwd`). Steps: (1) verify server reachability + PAT validity via `GET /api/health` with `Authorization: Bearer <PAT>`; (2) save server URL + PAT to `~/.config/da/{config.yaml,token.json}`; (3) `GET /api/welcome` and write its body to `<workspace>/CLAUDE.md`; (4) write `.claude/settings.json` (model, permissions, hooks pointing at `da pull` and `da push`); (5) write `.claude/CLAUDE.local.md` (stub, only if absent); (6) call `da pull` programmatically (refactor below); (7) write `AGNES_WORKSPACE.md` from a static client-side template with `{created_at}`, `{server_url}`, `{workspace_path}` substituted. `da init` does NOT call `da auth login`; the PAT from the paste-prompt is the only auth path during bootstrap. |
| `da pull` (renamed from `da sync`) | `cli/commands/pull.py` (renamed from `cli/commands/sync.py`) | Behavior is today's `da sync` minus the `--upload-only` branch. Lazy-mkdir fixes (see below). Refactor: extract data-path logic into `cli/lib/pull.py:run_pull(server_url, token, workspace) -> PullResult` so `da init` can call it programmatically without a Typer wrapper. Flags: `--quiet` (suppress success stdout, used by hook), `--json` (machine output), `--dry-run` (compute deltas without writing). |
| `da push` (extracted from `da sync --upload-only`) | `cli/commands/push.py` (new) | Uploads `user/sessions/*.jsonl` and `.claude/CLAUDE.local.md`. Lazy: skip when nothing to upload (no `user/sessions/` mkdir if no sessions). Same auth as `da pull`. Flags: `--quiet`, `--json`, `--dry-run`. |
| `da snapshot create` (renamed from `da fetch`) | `cli/commands/snapshot.py` | Move logic from `cli/commands/fetch.py` into a `create` subcommand of the existing `snapshot` group. Remove `cli/commands/fetch.py`. Carry over all flags: `--select`, `--where`, `--limit`, `--order-by`, `--as`, `--estimate`, `--no-estimate`, `--force`. Add existence check before opening DuckDB to avoid creating an empty DB file when no `da pull` has run yet (guard: `if not db_path.exists(): typer.echo("Local DuckDB not found. Run: da pull"); raise typer.Exit(1)`). Existing `da snapshot {refresh, prune, list, drop}` are unchanged. |
| `da status` (renamed from `da analyst status`) | `cli/commands/status.py` (renamed from analyst.py status fn) | Path refs updated to new layout: `server/parquet/`, `user/duckdb/analytics.duckdb`. Drop `data/metadata/last_sync.json`; use mtime on `user/duckdb/analytics.duckdb` as freshness proxy. |
| Layered config loader | `cli/config.py` | `load_config()`/`load_token()` first check `<cwd>/.agnes/{config.yaml,token.json}`; fall back to `~/.config/da/`. `save_*()` writes per-workspace if a per-workspace file exists, else global. `DA_LOCAL_DIR` env var continues to override `cwd` for testing. |
| Lazy-mkdir contract | `cli/commands/pull.py`, all writers | No `mkdir(parents=True, exist_ok=True)` before a conditional write loop. Mkdir only immediately before the first file write. Concretely: `_fetch_and_write_rules` mkdirs `.claude/rules/` only when `mandatory approved` is non-empty; `parquet_dir` mkdir is inlined into the per-table download loop. |
| `da catalog --metrics` flag | `cli/commands/catalog.py` | Add `--metrics` flag that switches output to the metric definitions list (formerly `da metrics list`). Other `da metrics *` subcommands removed. |
| Removed: `da metrics`, `da skills`, `da fetch`, `da analyst *`, `da sync` | `cli/commands/{metrics.py,skills.py,fetch.py,analyst.py,sync.py}` | Deleted (greenfield). |
| `da catalog --metrics` flag | `cli/commands/catalog.py` | Add `--metrics` flag that switches output to the metric definitions list (formerly `da metrics list`). `--metrics --show <id>` covers `da metrics show`. |
| `da admin metrics import` (relocated) | `cli/commands/admin.py` | Move `da metrics import` here as `da admin metrics import`. Admin-only; not part of analyst flow. |
| Removed | `cli/commands/{metrics.py, skills.py, fetch.py, analyst.py, sync.py}` | Deleted (greenfield). |
### Templates
### Templates and docs
| Component | File | Change |
|---|---|---|
| `CLAUDE.md` template | `config/claude_md_template.txt` | Update path references: `data/parquet/``server/parquet/`, `data/duckdb/...``user/duckdb/analytics.duckdb`. Replace `da sync``da pull`, `da fetch``da snapshot create`. Add a one-line pointer: "For human docs, see `AGNES_WORKSPACE.md` in this folder." |
| `AGNES_WORKSPACE.md` template (new) | `config/agnes_workspace_template.txt` (new) | Static template with three placeholders: `{created_at}`, `{server_url}`, `{workspace_path}`. Content described in dedicated section below. |
| Server-side `CLAUDE.md` template | `config/claude_md_template.txt` (and any DB override flagged in admin migration) | Path strings + verb names updated as listed in Server-side table. |
| `AGNES_WORKSPACE.md` template (new) | `config/agnes_workspace_template.txt` (new, client-side static asset bundled with the wheel) | Three placeholders: `{created_at}`, `{server_url}`, `{workspace_path}`. Header line uses all three; remaining content is static. Content described in dedicated section below. |
| Repo-root `CLAUDE.md` rewrite | `CLAUDE.md` (project root) | Update all references: `da sync``da pull`, `da analyst setup``da init`, `da metrics list``da catalog --metrics`, `da fetch``da snapshot create`, `data/parquet/``server/parquet/`. The "Local sync & Claude Code hooks" subsection and the "Querying Agnes data — agent rails" subsection both need walk-throughs. |
## Web UI flow
```
GET /install?role=analyst
└─ render install.html with `role=analyst` context
GET /setup?role=analyst
└─ render setup.html with `role=analyst` context
├─ "Analyst workspace" tile is active (visual highlight)
├─ "Admin CLI" tile linked to /install?role=admin
├─ "Admin CLI" tile linked to /setup?role=admin
[user clicks "Generate prompt"]
└─ JS: POST /auth/tokens
@ -189,7 +213,9 @@ GET /install?role=analyst
└─ Claude executes the steps in order
```
PAT lifecycle: minted on click, single TTL window (1h), no auto-revoke after first use. The user has 1h to retry the bootstrap if a step fails. After 1h they re-click "Generate prompt" for a new PAT. The bootstrap PAT is functionally indistinguishable from a regular PAT once minted; the `scope` claim is informational (audit trail), not enforced per-endpoint. Per-endpoint scope enforcement is a follow-up issue.
PAT lifecycle: minted on click, single TTL window (1 h), no auto-revoke after first use. The user has 1 h to retry the bootstrap if a step fails. After 1 h they re-click "Generate prompt" for a new PAT. The `scope="bootstrap-analyst"` claim is informational at this stage — its only consumer in this PR is the server's audit log. Per-endpoint scope enforcement is a follow-up issue.
Legacy `/install` URL: kept as a 302 redirect to `/setup`. All new references in code and docs use `/setup?role=...`.
## Workspace layout
@ -209,15 +235,15 @@ Post-`da init`, workspace contains exactly:
Conditional additions:
- `./.claude/rules/km_*.md` — only when `/api/memory/bundle` returns ≥1 mandatory or approved item.
- `./server/parquet/<table>.parquet` — only when `/api/sync/manifest` returns ≥1 table the user has grants on.
- `./.claude/rules/km_*.md` — only when `/api/memory/bundle` returns ≥ 1 mandatory or approved item.
- `./server/parquet/<table>.parquet` — only when `/api/sync/manifest` returns ≥ 1 table the user has grants on.
- `./user/snapshots/<name>.parquet` — only after the user runs `da snapshot create <table> --as <name>`.
- `./user/sessions/<id>.jsonl` — only after the SessionEnd hook runs `da push` against captured Claude Code sessions.
- `./.agnes/config.yaml`, `./.agnes/token.json` — only if the user opts into per-workspace config (advanced; not created by default).
Forbidden under any circumstances (these are the dead paths today's setup creates):
- `./data/parquet/`, `./data/duckdb/`, `./data/metadata/`, `./user/artifacts/` — none of these were read by any code path; removed entirely.
- `./.agnes/` — per-workspace config is out of scope for this PR.
## Data flow / sequence
@ -232,11 +258,11 @@ Empty folder + Claude Code with paste prompt
├─ Step 2 — da init --server-url URL --token PAT --workspace .
│ ├─ verify: GET /api/health with Bearer PAT → 200
│ ├─ save: ~/.config/da/{config.yaml, token.json} (or per-workspace if <cwd>/.agnes/ exists)
│ ├─ write: ./CLAUDE.md
│ ├─ save: ~/.config/da/{config.yaml, token.json}
│ ├─ fetch: GET /api/welcome → write ./CLAUDE.md
│ ├─ write: ./.claude/settings.json (with hooks SessionStart→`da pull`, SessionEnd→`da push`)
│ ├─ write: ./.claude/CLAUDE.local.md (stub, if absent)
│ ├─ call: da pull (programmatic)
│ ├─ call: da pull (programmatic — calls cli/lib/pull.py:run_pull)
│ │ ├─ GET /api/sync/manifest → {tables, hashes}
│ │ ├─ for each table where local md5 ≠ remote md5:
│ │ │ GET /api/data/<id>/download (stream)
@ -247,7 +273,7 @@ Empty folder + Claude Code with paste prompt
│ │ └─ GET /api/memory/bundle → {mandatory, approved}
│ │ if non-empty: mkdir ./.claude/rules/ then write km_*.md
│ │ if empty: skip mkdir
│ └─ write: ./AGNES_WORKSPACE.md
│ └─ write: ./AGNES_WORKSPACE.md (from client-side static template)
├─ Step 3 — da catalog (smoke verify)
│ confirms end-to-end works; prints table count
@ -271,11 +297,11 @@ The clean-install contract has two halves: writers must be lazy (don't pre-alloc
Concretely:
| Writer | File:line | Today | After |
| Writer | File:line (today) | Today | After |
|---|---|---|---|
| `_fetch_and_write_rules` | `cli/commands/sync.py:222` | `rules_dir.mkdir(parents=True, exist_ok=True)` before iterating | Check `mandatory + approved` first; if empty, return without mkdir. |
| Per-table download loop | `cli/commands/sync.py:120, 529` | `parquet_dir.mkdir(parents=True, exist_ok=True)` before loop | Mkdir inlined into the per-file write block; first table triggers mkdir. |
| `_install_claude_hooks` | `cli/commands/analyst.py:290` | mkdir `.claude/` | unchanged — `.claude/` always has content (settings.json is load-bearing). |
| `_install_claude_hooks` | `cli/commands/analyst.py:254` | mkdir `.claude/` | unchanged — `.claude/` always has content (settings.json is load-bearing). |
| `_rebuild_duckdb_views` | `cli/commands/sync.py:321` | mkdir `user/duckdb/` | unchanged — DuckDB file is opened unconditionally as part of view rebuild; the file is the load-bearing artifact, not just the directory. |
| `da push` upload | (new) `cli/commands/push.py` | (n/a) | Mkdir `user/sessions/` only inside the per-session-write branch; `da push` with nothing to upload exits 0 without touching disk. |
| `da snapshot create` parquet write | `cli/commands/snapshot.py` | mkdir `user/snapshots/` before write | unchanged (snapshot create is the canonical writer; mkdir on first write is correct). |
@ -297,6 +323,7 @@ Audit of current readers (only commands that touch the filesystem are listed; ot
| `da snapshot create` (write side) | `user/snapshots/` | unchanged (writer, mkdir at first write) | unchanged. |
| `da disk-info` | `user/snapshots/` | `.exists()` guards around sum/count/free | unchanged. |
| `da snapshot list` | `user/snapshots/` | glob safe on missing | unchanged (glob returns empty iterator on missing dir). |
| `da snapshot refresh` / `prune` | `user/snapshots/` | glob/.exists() guards | unchanged. |
| `da push` | `user/sessions/` | `.exists()` check before iterating | unchanged. |
| `da status` | `server/parquet/`, `user/duckdb/...` | path strings reference legacy `data/parquet/` etc. | Update path strings; `.exists()` checks already in place. |
@ -306,7 +333,7 @@ Audit of current readers (only commands that touch the filesystem are listed; ot
def assert_no_dead_dirs(workspace: Path):
"""Workspace must not contain pre-allocated empty directories."""
forbidden_unconditional = ["data/parquet", "data/duckdb", "data/metadata",
"user/artifacts"]
"user/artifacts", ".agnes"]
for d in forbidden_unconditional:
assert not (workspace / d).exists(), f"forbidden dir created: {d}"
@ -322,13 +349,13 @@ This guard runs in every clean-install integration test.
## `AGNES_WORKSPACE.md` content
Generated by `da init` in the workspace root. Not state — pure documentation. Idempotent overwrite on every `da init` (preserves nothing, regenerates everything from the static template).
Generated by `da init` in the workspace root from a static client-side template (`config/agnes_workspace_template.txt`, bundled with the wheel). Not state — pure documentation. Idempotent overwrite on every `da init` (preserves nothing, regenerates everything).
Three placeholders only: `{created_at}`, `{server_url}`, `{workspace_path}`. No email, no user identity, no role. Email is not used anywhere in the analyst CLI flow; PAT identifies the user server-side, and decoded JWT email is informational at best — we drop it from this header for clarity.
Three placeholders only: `{created_at}`, `{server_url}`, `{workspace_path}`. Used in the header line "Created: {created_at} · Server: {server_url} · Workspace: {workspace_path}". No email, no user identity, no role. Email is not used anywhere in the analyst CLI flow; PAT identifies the user server-side, and decoded JWT email is informational at best — we drop it from this header for clarity.
Sections:
1. **Header**`Created: <ISO timestamp>`, `Server: <URL>`, `Workspace: <abs path>`.
1. **Header**`Created: <ISO timestamp> · Server: <URL> · Workspace: <abs path>`.
2. **What's installed (global, per-user)** — table of paths in `~/.local/bin/`, `~/.config/da/`, `~/.agnes/`, shell rc block. Each row: `path | what it is | how to remove`.
3. **What's in this folder** — table of paths in workspace. Each row: `path | what it is`. Notes which dirs are conditional ("only when grants/sessions/etc. exist").
4. **How it stays fresh** — explains SessionStart/End hooks: what they run, when, what failure looks like (silent, `|| true`).
@ -346,38 +373,52 @@ PAT value never appears in `AGNES_WORKSPACE.md` — only its location (`~/.confi
| Failure | Detection | Behavior |
|---|---|---|
| Server unreachable during `da init` | `httpx.ConnectError` on `/api/health` | exit 1, hint: "Cannot reach `<URL>` — check network or server status". |
| PAT expired | `/api/health` → 401 | exit 1, hint: "Token expired — get a fresh one at `<URL>/install?role=analyst`". |
| PAT invalid (mis-paste) | 401, JWT decode failure | exit 1, hint: "Token format invalid — re-copy from `/install`". |
| PAT expired | `/api/health` → 401 | exit 1, hint: "Token expired — get a fresh one at `<URL>/setup?role=analyst`". |
| PAT invalid (mis-paste) | 401, JWT decode failure | exit 1, hint: "Token format invalid — re-copy from `/setup`". |
| TLS trust failure | curl/wheel install fails with `unknown CA` | exit 1, hint refers user back to paste-prompt step 0. |
| Disk full during `da pull` | `OSError(ENOSPC)` on parquet write | atomic rename → partial file deleted; exit 1 with disk-info dump. |
| Concurrent `da init` in same folder | file lock on `<cwd>/.claude/settings.json` (or sentinel `<cwd>/.agnes/.init.lock`) | second invocation: "Setup already running" exit 1. |
| Concurrent `da init` in same folder | sentinel `<cwd>/.claude/.init.lock` | second invocation: "Setup already running" exit 1. |
| Partial state (previous `da init` crashed mid-way) | `CLAUDE.md` exists but `.claude/settings.json` missing | `da init` (without `--force`): friendly hint "Workspace partially set up — run `da init --force` to redo". |
| `da pull` 401 mid-session (PAT revoked server-side) | response 401 from `/api/sync/manifest` | hook command prints warning, exits 0 (`|| true`); session continues with last-known data. Manual `da pull` next time prints actionable hint. |
| `da pull` 401 mid-session (PAT revoked server-side) | response 401 from `/api/sync/manifest` | hook command prints warning, exits 0 (`\|\| true`); session continues with last-known data. Manual `da pull` next time prints actionable hint. |
| Empty manifest | `/api/sync/manifest``{"tables": []}` | success, no parquet dir created, no warning (valid state). |
| Empty memory bundle | `/api/memory/bundle``{"mandatory": [], "approved": []}` | success, no `.claude/rules/` dir (valid state). |
| Per-table 5xx mid-pull | per-table 500 from `/api/data/<id>/download` | per-table warn; pull continues; final exit 0 if at least one table succeeded, exit 1 if all failed. |
| Workspace path with spaces / unicode | path passed to subprocess as `cwd=`, no shell interpolation | works as-is; tested in clean-install integration test. |
| Hook fires in unrelated Claude Code session | settings.json is workspace-scoped (`<cwd>/.claude/settings.json`) | hook does not fire; Claude Code reads settings only for the directory it was opened in. |
Principle: hooks always end `|| true` so they never block a session. Manual commands are exit-1-with-hint. No silent failures in interactive flow.
Principle: hooks always end `\|\| true` so they never block a session. Manual commands are exit-1-with-hint. No silent failures in interactive flow.
## Verification / testing
Verification has three layers: (a) automated reader-smoke matrix that proves no command crashes on a freshly-bootstrapped workspace; (b) automated clean-install integration tests that prove the workspace contains exactly the expected files; (c) a manual end-to-end protocol that runs the actual paste prompt against a real local server.
### Test fixtures
| Fixture | Returns | What it pre-seeds |
|---|---|---|
| `fastapi_test_server` | object with `.url`, `.shutdown()` | Starts the FastAPI app in a background thread/subprocess against a `tmp_path`-rooted DATA_DIR. Clean schema, two seeded users (`admin@example.com`, `analyst@example.com`), two seeded user groups (`Admin`, `Everyone`), three seeded tables in `table_registry` with one `query_mode='local'`, one `query_mode='materialized'`, one `query_mode='remote'`. Manifest+memory endpoints serve real (test) data. |
| `test_pat` | string PAT for `analyst@example.com` | Group membership: `Everyone` only. `resource_grants` for the local + materialized tables (so manifest returns 2 rows for them). Two `mandatory` corporate-memory items granted via group. PAT TTL: 1 h. |
| `test_pat_no_grants` | string PAT for `analyst@example.com` | Same user, but `resource_grants` is empty and `corporate_memory` has zero items granted to `Everyone`. Manifest returns `{"tables": []}`; memory bundle returns `{"mandatory": [], "approved": []}`. |
| `zero_grants_workspace` | `tmp_path` after running `da init --token <test_pat_no_grants> --server-url <fastapi_test_server.url>` | A fully-bootstrapped workspace where every conditional dir is absent. Used by the reader smoke matrix. |
| `web_session` | authenticated `httpx.Client` with cookies | Logs in as `admin@example.com` via the test endpoint, returns the client. Used to mint PATs in PAT-scope tests. |
| `client` | `TestClient(app)` | Plain FastAPI test client with no auth. Used for endpoint-shape tests. |
Fixtures live in `tests/conftest.py` (existing) plus a new `tests/fixtures/analyst_bootstrap.py`.
### 5.1 Reader smoke matrix (automated)
```python
@pytest.mark.parametrize("cmd", [
["da", "catalog"],
["da", "catalog", "--metrics"],
["da", "schema", "any_table"],
["da", "describe", "any_table"],
["da", "query", "SELECT 1"],
["da", "explore", "any_view"],
["da", "disk-info"],
["da", "snapshot", "list"],
["da", "snapshot", "create", "any_table", "--as", "x", "--estimate"],
["da", "status"],
["da", "snapshot", "create", "any_table", "--estimate"],
["da", "diagnose"],
["da", "auth", "whoami"],
])
@ -397,7 +438,7 @@ This is the load-bearing test for "nothing crashes on missing dirs".
```python
def test_clean_install_minimal_grants(fastapi_test_server, tmp_path, test_pat):
"""User has 3 table grants, 2 mandatory rules → expected workspace shape."""
"""User has 2 table grants + 2 mandatory rules → expected workspace shape."""
subprocess.run(
["da", "init", "--server-url", fastapi_test_server.url,
"--token", test_pat, "--workspace", str(tmp_path)],
@ -409,7 +450,7 @@ def test_clean_install_minimal_grants(fastapi_test_server, tmp_path, test_pat):
"user/duckdb/analytics.duckdb"]:
assert (tmp_path / must).exists(), f"missing required: {must}"
# Conditional (present because grants/rules exist):
assert len(list((tmp_path / "server" / "parquet").glob("*.parquet"))) == 3
assert len(list((tmp_path / "server" / "parquet").glob("*.parquet"))) == 2
assert len(list((tmp_path / ".claude" / "rules").iterdir())) == 2
# Forbidden:
assert_no_dead_dirs(tmp_path)
@ -419,6 +460,9 @@ def test_clean_install_minimal_grants(fastapi_test_server, tmp_path, test_pat):
for h in settings["hooks"]["SessionStart"])
assert any("da push" in h["hooks"][0]["command"]
for h in settings["hooks"]["SessionEnd"])
# CLAUDE.md was fetched from /api/welcome (not local template):
claude_md = (tmp_path / "CLAUDE.md").read_text()
assert "da pull" in claude_md and "da sync" not in claude_md # post-rewrite content
def test_clean_install_zero_grants(fastapi_test_server, tmp_path, test_pat_no_grants):
@ -429,7 +473,7 @@ def test_clean_install_zero_grants(fastapi_test_server, tmp_path, test_pat_no_gr
"user/duckdb/analytics.duckdb"}
must_not_exist = {".claude/rules", "server/parquet", "data/parquet",
"data/duckdb", "data/metadata", "user/artifacts",
"user/sessions", "user/snapshots"}
"user/sessions", "user/snapshots", ".agnes"}
for p in must_exist:
assert (tmp_path / p).exists()
for p in must_not_exist:
@ -471,7 +515,7 @@ def test_render_setup_instructions_analyst_role():
```python
def test_bootstrap_pat_ttl_clamped_to_one_hour(client, web_session):
resp = client.post("/auth/tokens", json={
resp = web_session.post("/auth/tokens", json={
"scope": "bootstrap-analyst",
"ttl_seconds": 86400, # ignored — server force-clamps
"name": "init",
@ -481,13 +525,20 @@ def test_bootstrap_pat_ttl_clamped_to_one_hour(client, web_session):
payload = jwt.decode(pat, options={"verify_signature": False})
assert payload["scope"] == "bootstrap-analyst"
assert payload["exp"] - payload["iat"] <= 3600 + 5
def test_bootstrap_pat_falls_back_to_expires_in_days(web_session):
"""When ttl_seconds is omitted, expires_in_days still works (back-compat)."""
resp = web_session.post("/auth/tokens", json={"name": "test", "expires_in_days": 30})
assert resp.status_code == 200
payload = jwt.decode(resp.json()["pat"], options={"verify_signature": False})
assert payload["exp"] - payload["iat"] <= 30 * 86400 + 5
```
### 5.5 Manual clean-install protocol (pre-merge)
1. `git clean -fdx` in the repo (no build artifacts).
2. Boot FastAPI locally against a clean test instance state.
3. Empty terminal in `/tmp/test-analyst-1`. From the web `/install?role=analyst`, paste prompt.
3. Empty terminal in `/tmp/test-analyst-1`. From the web `/setup?role=analyst`, paste prompt.
4. `tree -a /tmp/test-analyst-1` and compare with the expected tree from §5.2.
5. `claude` in that folder. Three queries: "what tables can I see", "SELECT count(*) FROM <t>", "show me last 5 rows of <t>". All must work without further intervention.
6. `/exit`. Verify SessionEnd hook ran (server-side audit log shows `da push`; `du -sh /tmp/test-analyst-1/user/sessions/` non-empty).
@ -498,23 +549,26 @@ This protocol is documented in `docs/RELEASE_CHECKLIST.md` as a mandatory pre-me
## Out of scope
1. **Admin CLI tooling**`/install?role=admin` and `da admin *` continue unchanged.
1. **Admin CLI tooling**`/setup?role=admin` and `da admin *` continue unchanged. The new CLI surface listing in this spec is the *analyst* surface; admin verbs not listed (e.g., `da admin marketplace`, `da admin user`, etc.) are unaffected.
2. **Migration of existing analyst workspaces** — greenfield; old `data/parquet/` etc. are dead but harmless.
3. **Backward-compat aliases** — no `da analyst setup``da init` shim, no `da sync``da pull` shim. Hard cutover.
4. **Multi-user / shared workspace**`<cwd>` is single-user.
5. **Offline initial bootstrap**`da init` requires server reachability.
6. **PAT auto-refresh / refresh tokens** — bootstrap PAT expires after 1h; user re-clicks "Generate prompt".
6. **PAT auto-refresh / refresh tokens** — bootstrap PAT expires after 1 h; user re-clicks "Generate prompt".
7. **Per-endpoint PAT scope enforcement**`bootstrap-analyst` scope is informational at this stage (audit-trail). Per-endpoint enforcement is a follow-up issue.
8. **Web UI redesign**`/install?role=...` reuses the existing page shell + JS. No visual redesign.
8. **Web UI redesign**`/setup?role=...` reuses the existing page shell + JS. No visual redesign.
9. **CLI rename adjacent commands** beyond what's listed (e.g., `da auth login``da login`) — out of scope.
10. **Layered per-workspace config**`<cwd>/.agnes/{config.yaml,token.json}` overrides considered but dropped from this PR (no defined producer; multi-instance is edge case). Captured in Open questions.
## Open questions / follow-ups
- **Per-endpoint PAT scope enforcement** — should `scope="bootstrap-analyst"` PATs be restricted to `/api/health`, `/api/sync/manifest`, `/api/data/*/download`, `/api/memory/bundle` only, and refused on (e.g.) `/api/admin/*`? Today not enforced. New issue.
- **`da catalog --metrics`** — folding `da metrics` into a flag may be too aggressive; metrics has its own `show <id>`, `import` subcommands today. Open question whether to fully delete or just demote to `da catalog --metrics list` and keep `da catalog --metrics show <id>` etc. To be resolved in implementation plan.
- **`da snapshot create` UX** — does the `--where` clause keep BigQuery SQL flavor or switch to DuckDB flavor? Today `da fetch` uses BQ flavor. Keep BQ flavor for parity with `da query --remote`.
- **Layered per-workspace config** — supporting multi-instance use cases (one analyst, two Agnes servers) requires a defined producer for `<cwd>/.agnes/`. Options: `da init --per-workspace-config` flag, post-init manual `mkdir`, or `da config init`. Not chosen because no current user has asked for it. New issue if/when needed.
- **`da catalog --metrics` UX** — does `--metrics` show definitions list (replaces `da metrics list`), and `--metrics --show <id>` show one definition (replaces `da metrics show`), or do we just have `da catalog --metrics` for both with a separate flag? To be resolved in implementation plan.
- **`da snapshot create --where` SQL flavor** — keep BigQuery flavor (today's `da fetch`) for parity with `da query --remote`, since BQ is the only remote source. Confirmed in this PR; flagged in case a non-BQ remote source is added later.
- **Hook performance budget**`da pull` on a 1.1 GB workspace (real-world example: today's `tmp_oss/server/parquet/`) with all parquets unchanged should complete the manifest comparison in well under 1 s so SessionStart doesn't perceptibly delay the user. If incremental MD5 comparison is too slow at scale, consider a server-side ETag.
- **Anti-coupling test** — add a test that imports every `cli/commands/*.py` module and asserts no module imports any other `cli.commands.*` module except via dispatch (Typer subcommand registration). Prevents the `init` command from accidentally re-importing `pull` internals in a way that creates hidden coupling.
- **DB-stored CLAUDE.md override migration** — admins who saved an override via `PUT /api/admin/workspace-prompt-template` may have legacy strings (`data/parquet/`, `da sync`, etc.). Spec says "migration is not automatic — admins must re-author". Implementation plan needs to decide: surface a banner in the admin UI when an override contains flagged strings, or do nothing (rely on admin to notice).
## CHANGELOG entry (preview)
@ -522,14 +576,14 @@ This protocol is documented in `docs/RELEASE_CHECKLIST.md` as a mandatory pre-me
## [Unreleased]
### Changed
- **BREAKING** Analyst bootstrap rewritten end-to-end. `da analyst setup` is removed; replaced by `da init` (non-interactive, requires `--server-url` and `--token`). `da sync` is split into `da pull` (refresh) and `da push` (upload). `da fetch` is folded into `da snapshot create`. `da metrics` is folded into `da catalog --metrics`. `da skills` is removed from the analyst CLI. The `da analyst` namespace is removed; the workspace status command is now `da status`.
- **BREAKING** Analyst bootstrap rewritten end-to-end. `da analyst setup` is removed; replaced by `da init` (non-interactive, requires `--server-url` and `--token`). `da sync` is split into `da pull` (refresh) and `da push` (upload). `da fetch` is folded into `da snapshot create`. `da metrics list/show` is folded into `da catalog --metrics`; `da metrics import` moves to `da admin metrics import`. `da skills` is removed from the analyst CLI. The `da analyst` namespace is removed; the workspace status command is now `da status`.
- **BREAKING** Workspace layout simplified. Removed: `data/parquet/`, `data/duckdb/`, `data/metadata/`, `user/artifacts/`. Canonical paths: `server/parquet/` (synced parquets), `user/duckdb/analytics.duckdb` (DuckDB views), `user/snapshots/` (ad-hoc snapshots), `user/sessions/` (recorded sessions).
- The `/install` web page now branches on a `role` query parameter: `/install?role=analyst` renders the analyst workspace bootstrap prompt; `/install?role=admin` renders the admin CLI install prompt.
- The `/setup` web page now branches on a `role` query parameter: `/setup?role=analyst` renders the analyst workspace bootstrap prompt; `/setup?role=admin` renders the admin CLI install prompt. `/install` continues to 302 to `/setup`.
- `CLAUDE.md` server-side template (and any admin DB override) updated to reference the new CLI verbs and workspace paths. Admins who maintain a custom `workspace-prompt-template` override should re-author it; the admin UI surfaces a warning when an override contains legacy strings.
### Added
- `AGNES_WORKSPACE.md` — human-readable workspace docs file generated by `da init` in the workspace root. Documents global install, workspace layout, hooks, cheat sheet, uninstall recipe.
- Layered config: `<cwd>/.agnes/{config.yaml,token.json}` overrides `~/.config/da/`, enabling multi-instance use cases.
- PAT scope field. PATs minted with `scope="bootstrap-analyst"` are TTL-clamped to ≤ 1 h server-side.
- PAT request body now accepts `scope: str = "general"` and `ttl_seconds: int | None = None` fields. PATs minted with `scope="bootstrap-analyst"` are TTL-clamped to ≤ 1 h server-side. Existing `expires_in_days` field continues to work; `ttl_seconds` wins when both are set.
### Fixed
- `da pull` (formerly `da sync`) no longer creates `.claude/rules/` when the corporate-memory bundle is empty.
@ -538,6 +592,6 @@ This protocol is documented in `docs/RELEASE_CHECKLIST.md` as a mandatory pre-me
- Workspace `da status` reads from the canonical `server/parquet/` and `user/duckdb/analytics.duckdb` paths (was reading legacy `data/parquet/`, `data/metadata/last_sync.json`).
### Removed
- `da analyst setup`, `da analyst status`, `da sync`, `da fetch`, `da metrics`, `da skills`. See "Changed" above for replacements.
- `da analyst setup`, `da analyst status`, `da sync`, `da fetch`, `da metrics list/show`, `da skills`. See "Changed" above for replacements.
- Legacy workspace directories `data/parquet/`, `data/duckdb/`, `data/metadata/`, `user/artifacts/`. Existing analyst workspaces should be reinitialized with `da init --server-url ... --token ... --force` (a fresh empty folder is recommended).
```