Files
archy/.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-PLAN.md
T

244 lines
19 KiB
Markdown
Raw Normal View History

2026-08-12 10:55:50 +00:00
---
phase: 13-aiui-functional-conversational-node-control-and-content-surf
plan: 07
type: execute
wave: 3
depends_on: ["13-04"]
files_modified:
- core/archipelago/src/music/index.rs
- core/archipelago/src/music/mod.rs
- core/archipelago/src/api/rpc/music.rs
- core/archipelago/src/api/rpc/dispatcher.rs
autonomous: true
requirements: [AIUI-03]
must_haves:
truths:
- "The node has a real music library — albums, artists and tracks derived from extracted tags, persisted under data_dir, not a MIME filter over a folder listing (D-13)"
- "The index stays fresh: a file added, changed or removed since the last scan is reflected without a full rebuild, and a full reindex is available on demand"
- "An index written by a newer MUSIC_SCHEMA_VERSION is refused and rebuilt rather than misread"
- "A concurrent read during a reindex returns a consistent snapshot, never a partially-written index (edge: AIUI-03 concurrency — authored, not probe-surfaced)"
- "Album and track ordering is deterministic and stable across repeated calls, with a defined tiebreak when sort keys are equal (edge: AIUI-03 ordering — authored, not probe-surfaced)"
- "An empty library returns empty arrays with a scanned-at timestamp, not an error and not a null (edge: AIUI-03 empty — authored, not probe-surfaced)"
- "The indexer never reads outside the configured media roots"
artifacts:
- path: "core/archipelago/src/music/index.rs"
provides: "Scan, extract, persist and incrementally refresh the library index under data_dir"
contains: "pub async fn reindex"
- path: "core/archipelago/src/api/rpc/music.rs"
provides: "music.* RPC surface backing SongGrid"
contains: "handle_music"
key_links:
- from: "core/archipelago/src/music/index.rs"
to: "core/archipelago/src/music/tags.rs"
via: "extract_tags per file, with media_roots confinement passed through"
pattern: "extract_tags"
- from: "core/archipelago/src/api/rpc/dispatcher.rs"
to: "core/archipelago/src/api/rpc/music.rs"
via: "single music. prefix arm, mirroring 13-01's assistant. arm"
pattern: "starts_with\\(\"music\\.\"\\)"
---
<objective>
Build the library D-13 asked for: albums, artists, tracks, tag extraction and an index that
stays fresh — over the entity model decided at 13-04's checkpoint, using the extraction built
there.
CONTEXT.md is blunt about why this exists: today "music" on a node is only a MIME branch and a
hardcoded `Music` folder, with no library domain at all. The user chose the real library over
the narrower MIME-filtered-files option after being told that.
**Track independence (D-13):** no plan on the control or content track lists any music plan in
its `depends_on`. This plan depends only on 13-04. Peer files, movies and conversational control
ship on their own track; the library lights up `SongGrid` in 13-11 when it is ready.
**Deliberately out of scope, stated rather than implied:** this phase does not add a music tool
to the assistant's curated registry. Music browsing is a grid surface here, not a chat surface;
the registry's `content_list` (Media) already covers media reads, and adding a music tool would
create a coupling between the two tracks that D-13 exists to avoid.
Output: `music/index.rs`, `music/mod.rs` completed, and the `music.*` RPC surface.
</objective>
<flagged_assumptions>
None in this plan.
**Edge-tag provenance.** The three truths above tagged `(edge: AIUI-03 … — authored, not
probe-surfaced)` are **not** deterministic-probe output. The AIUI-03 probe surfaced four edges
and all four are discharged in 13-06 as covered truths. These three re-apply the same edge kinds
— concurrency, ordering, empty — to a different subject (the on-disk music index rather than the
in-browser content adapter), because a persisted index has its own versions of them that
13-06's tests cannot reach. They are planner-authored coverage, and the tag says so, so the
phase's edge accounting is not double-counting one probe as seven findings. The full
reconciliation is in `13-VALIDATION.md` § Edge-Probe Reconciliation.
</flagged_assumptions>
<artifacts_this_phase_produces>
Symbols created by **this plan**:
- `core/archipelago/src/music/index.rs`: `pub struct MusicIndex`, `pub async fn reindex`,
`pub async fn refresh_incremental`, `pub fn load`, `pub fn save_atomic`, `struct IndexEntry`,
`struct ScanStats`, `fn group_albums`, `fn group_artists`, `const INDEX_FILENAME`
- `core/archipelago/src/music/mod.rs`: `pub fn media_roots`, `pub struct LibrarySnapshot`
- `core/archipelago/src/api/rpc/music.rs`: `handle_music` (prefix sub-dispatcher),
`handle_music_list_albums`, `handle_music_list_artists`, `handle_music_list_tracks`,
`handle_music_status`, `handle_music_reindex`
- New RPC method names: `music.list-albums`, `music.list-artists`, `music.list-tracks`,
`music.status`, `music.reindex`
- `core/archipelago/src/api/rpc/dispatcher.rs`: one `music.` prefix arm
</artifacts_this_phase_produces>
<execution_context>
@$HOME/.claude/gsd-core/workflows/execute-plan.md
@$HOME/.claude/gsd-core/templates/summary.md
</execution_context>
<context>
@.planning/PROJECT.md
@.planning/STATE.md
@CLAUDE.md
@.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-CONTEXT.md
@.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-MUSIC-MODEL.md
@.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-04-SUMMARY.md
</context>
<tasks>
<task type="auto" tdd="true">
<name>Task 1: The index — scan, group, persist, and stay fresh without a full rebuild</name>
<files>core/archipelago/src/music/index.rs, core/archipelago/src/music/mod.rs</files>
<behavior>
- A first `reindex` over a directory of tagged files produces tracks, and albums and artists grouped exactly as `13-MUSIC-MODEL.md` decided.
- `refresh_incremental` after adding one file adds one track and does not re-extract tags for unchanged files.
- `refresh_incremental` after deleting one file removes that track, and removes the album if it had no other tracks.
- `refresh_incremental` after a file's mtime changes re-extracts that file's tags and updates the track in place, keeping its identity per the decided scheme.
- Loading an index whose `schema_version` is greater than `MUSIC_SCHEMA_VERSION` returns a distinct error and triggers a full rebuild rather than a partial read.
- A read taken while a reindex is in progress returns either the complete previous snapshot or the complete new one — never a mix and never a truncated file.
- `reindex` on an empty directory produces an index with empty collections and a populated `scanned_at`.
- A symlink pointing outside the media roots is skipped, not followed.
</behavior>
<read_first>
- `.planning/phases/13-.../13-MUSIC-MODEL.md` — the decided identity scheme, whether albums/artists are stored or derived, the index path and format, and the reindex path. **This task implements that decision; it does not revisit it.**
- `core/archipelago/src/music/mod.rs` and `music/tags.rs` from 13-04 — `Track`/`Album`/`Artist`, `MUSIC_SCHEMA_VERSION`, `extract_tags(path, media_roots)`.
- `core/archipelago/src/content_server.rs``load_catalog`. `13-PATTERNS.md` assigns this as the role-match analog: read its scan-and-persist shape, its `data_dir` convention and its error handling, and follow them.
- `core/archipelago/src/streaming/session.rs` — the other `data_dir`-scoped persisted-state precedent, for file permissions.
- `core/archipelago/src/swarm/payment.rs` — the `#[tokio::test]` + `tempfile` convention.
</read_first>
<action>
Create `core/archipelago/src/music/index.rs` implementing the entity model recorded in `13-MUSIC-MODEL.md`.
`media_roots(&Config) -> Vec<PathBuf>` in `mod.rs` returns the roots the indexer is confined to, drawn from the sources 13-04 decided to index. Every filesystem operation in this module takes those roots and refuses paths outside them, canonicalizing first and skipping symlinks whose target escapes — an indexer that can be aimed at `data_dir/secrets` is a secret-exfiltration primitive, and this is the second of the two places (with `tags.rs`) that confinement is enforced.
`reindex` walks the roots, calls `extract_tags` per audio file, builds `Track` rows, and groups albums and artists per the decision. `refresh_incremental` compares each file's `(path, mtime, size)` against the stored `IndexEntry` and only re-extracts changed files, removing rows for files that disappeared and pruning albums that lost their last track. Track a `ScanStats { scanned, extracted, skipped, removed, elapsed_ms }` and return it — a library scan that gives no feedback is indistinguishable from a hang on a large collection.
Persistence: `save_atomic` writes to a sibling temp file in the same directory and `rename`s over the target, so a read never sees a partial file and a crash mid-write leaves the previous index intact. That single choice is what makes the concurrency behaviour above true; do not write in place. `load` refuses an index whose `schema_version` exceeds `MUSIC_SCHEMA_VERSION` with a distinct error variant and lets the caller rebuild — a forward-incompatible index misread as current is worse than no index.
Ordering: define one comparator used everywhere — albums by album artist then album title then year, tracks by disc then track number then title, with the decided identity as the final tiebreak so equal keys never reorder between calls.
Guard the reindex with a lock or an atomic in-progress flag so two concurrent `music.reindex` calls do not both walk the tree; the second returns "already running" with the current stats rather than queueing a duplicate scan.
Write the tests FIRST, one per `<behavior>` bullet, generating fixture audio into a `tempfile::tempdir()` with `lofty`'s writing API as 13-04 established (no committed binary fixtures). Name them under `music::index::tests::`.
</action>
<verify>
<automated>cd core &amp;&amp; CARGO_INCREMENTAL=0 cargo test --package archipelago music::index:: 2>&amp;1 | tail -25</automated>
</verify>
<acceptance_criteria>
- `cd core && cargo test --package archipelago music::index::` exits 0 with a test per `<behavior>` bullet
- `grep -q 'pub async fn reindex' core/archipelago/src/music/index.rs` and `grep -q 'refresh_incremental' core/archipelago/src/music/index.rs`
- `grep -cE 'rename|persist' core/archipelago/src/music/index.rs` ≥ 1 and `grep -c 'save_atomic' core/archipelago/src/music/index.rs` ≥ 1 — the write is atomic, not in place
- `grep -q 'MUSIC_SCHEMA_VERSION' core/archipelago/src/music/index.rs` and the load path compares against it
- `grep -q 'media_roots' core/archipelago/src/music/index.rs` — confinement is a parameter, and it is enforced here as well as in `tags.rs`
- `git ls-files core/archipelago | grep -ciE '\.(mp3|flac|m4a|ogg)$'` returns 0
- The grouping field names in `index.rs` match `13-MUSIC-MODEL.md` — spot-check each and record the result in the summary
</acceptance_criteria>
<reversibility rating="costly">The on-disk index is the persisted half of D-13's one-way decision — but that door was already gated: the entity model, the index location and the index format were decided at **13-04 Task 1's `checkpoint:decision`**, which this plan depends on. This task implements that recorded decision and adds the `MUSIC_SCHEMA_VERSION` guard plus a written reindex path, which is what turns a future entity-model change from silently lossy into merely costly. No new one-way door is opened here.</reversibility>
<done>A directory of real tagged files becomes a persisted album/artist/track index; adding, changing and deleting one file each update it incrementally; a crash mid-write cannot corrupt it; and a forward-version index is refused rather than misread.</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: The music.* RPC surface, behind one dispatcher arm</name>
<files>core/archipelago/src/api/rpc/music.rs, core/archipelago/src/api/rpc/dispatcher.rs</files>
<behavior>
- `music.list-albums` returns albums in the deterministic order, with a `scanned_at` and a total count.
- `music.list-tracks` accepts an optional `album_id` filter and paginates with `limit`/`offset`, capping `limit` so a huge library cannot be pulled in one response.
- `music.status` returns the last scan's `ScanStats`, whether a scan is running, and the schema version.
- `music.reindex` starts a scan and returns immediately; a second call while one is running reports already-running instead of starting a duplicate.
- Every `music.*` method is refused without an authenticated session.
- An empty library returns empty arrays with a populated `scanned_at`, never null and never an error.
</behavior>
<read_first>
- `core/archipelago/src/api/rpc/mesh/assistant.rs``13-PATTERNS.md`'s exact-match analog for handler shape: `impl RpcHandler` + `pub(in crate::api::rpc) async fn handle_* -> Result<serde_json::Value>`.
- `core/archipelago/src/api/rpc/dispatcher.rs` — the arm 13-01 added, `m if m.starts_with("assistant.")`. **Mirror it exactly for `music.`**; do not add five individual arms.
- `core/archipelago/src/api/rpc/mod.rs` lines 264-330 — the session + CSRF + `role.can_access()` gate that runs before dispatch, so no bespoke auth belongs in these handlers.
- `core/archipelago/src/api/rpc/middleware.rs``UNAUTHENTICATED_METHODS`. Read it to confirm you are not adding to it.
- `core/archipelago/src/api/rpc/content.rs` — the pagination and response-envelope conventions used by `content.list-mine` / `content.owned-list`; match them so the neode-ui side has one shape to learn.
</read_first>
<action>
Create `core/archipelago/src/api/rpc/music.rs` with `handle_music(&self, method: &str, params) -> Result<Value>` as a prefix sub-dispatcher plus `handle_music_list_albums`, `handle_music_list_artists`, `handle_music_list_tracks`, `handle_music_status` and `handle_music_reindex`.
Register in `dispatcher.rs` as a **single** guarded arm `m if m.starts_with("music.") => self.handle_music(m, params).await`, mirroring 13-01's `assistant.` arm. Place it adjacent to the `content.*` block so a reader finds the media surfaces together. This is the only `dispatcher.rs` edit in the music track.
Response envelopes match `content.*`'s conventions so `archyContentAdapter.ts` (13-11) has one shape to consume. `music.list-tracks` caps `limit` at 500 and defaults to 100; an out-of-range `limit` is clamped, not rejected, so a UI bug degrades to a smaller page rather than an error.
`music.reindex` spawns the scan with `tokio::spawn` and returns immediately with the in-progress flag — a synchronous reindex would hold an RPC connection for the length of a library walk. It must not hold any shared lock across the walk (the same discipline `mesh/listener/assist.rs` documents for its own spawned work).
Do **not** add any `music.*` method to `UNAUTHENTICATED_METHODS`. Add a test asserting no string starting with `music.` appears there, mirroring 13-01's `assistant_methods_require_session`.
Write the tests FIRST, one per `<behavior>` bullet, under `api::rpc::music::tests::`.
</action>
<verify>
<automated>cd core &amp;&amp; CARGO_INCREMENTAL=0 cargo test --package archipelago music:: 2>&amp;1 | tail -25</automated>
<automated>cd core &amp;&amp; CARGO_INCREMENTAL=0 cargo build --package archipelago 2>&amp;1 | tail -5</automated>
</verify>
<acceptance_criteria>
- `cd core && cargo test --package archipelago music::` exits 0 with a test per `<behavior>` bullet, including `music_methods_require_session`
- `grep -c 'starts_with("music.")' core/archipelago/src/api/rpc/dispatcher.rs` returns 1 — one arm for the whole surface
- `grep -n 'music\.' core/archipelago/src/api/rpc/middleware.rs` returns no match
- `grep -q 'handle_music' core/archipelago/src/api/rpc/music.rs`
- `grep -cE 'limit' core/archipelago/src/api/rpc/music.rs` ≥ 1 and the clamp is visible in the source
- `cd core && CARGO_INCREMENTAL=0 cargo build --package archipelago` exits 0
</acceptance_criteria>
<done>An authenticated caller can list albums, artists and paginated tracks, read scan status, and trigger a reindex that does not duplicate itself; an unauthenticated caller gets nothing.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| filesystem → indexer | Filenames and tag contents are peer-influenceable for any shared audio |
| index file → readers | A persisted, versioned artifact that survives restarts and upgrades |
| `music.*` RPC → callers | Session + CSRF + RBAC, inherited from the existing dispatch gate |
## STRIDE Threat Register
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|-----------|----------|-----------|----------|-------------|-----------------|
| T-13-39 | Information Disclosure | Indexer walking outside the media roots (symlink escape) | high | mitigate | Canonicalize-and-confine in both `tags.rs` and `index.rs`; symlinks whose target escapes are skipped, not followed. Asserted by the symlink test |
| T-13-40 | Elevation of Privilege | `music.*` reachable unauthenticated | high | mitigate | Registered in the normal dispatch table so the existing session/CSRF/RBAC gate applies; asserted by `music_methods_require_session` and by the `middleware.rs` grep |
| T-13-41 | Denial of Service | A huge library pulled in one response, or a reindex duplicated per click | medium | mitigate | `limit` clamped at 500; `music.reindex` is spawned, returns immediately, and refuses to start a second concurrent scan |
| T-13-42 | Tampering | Crash mid-write corrupts the index | medium | mitigate | `save_atomic` writes to a temp sibling and renames; a crash leaves the previous index intact. Asserted by the concurrent-read test |
| T-13-43 | Tampering | Forward-version index misread as current, producing silently wrong entities | medium | mitigate | `load` refuses `schema_version > MUSIC_SCHEMA_VERSION` with a distinct error and rebuilds |
| T-13-44 | Denial of Service | Hostile audio file hangs or panics the scan | medium | mitigate | Per-file `extract_tags` errors are collected into `ScanStats.skipped` and the walk continues; no `unwrap` on parser output (inherited from 13-04) |
| T-13-45 | Tampering | Peer-authored tag text treated as trusted once indexed | high | accept | Out of this plan's scope by sequencing: nothing here places tag text in a model context. 13-12's `wrap_untrusted` boundary owns it. Recorded so the assumption is explicit |
| T-13-SC | Tampering | npm/pip/cargo installs | high | mitigate | **Zero** packages added — `lofty` entered at 13-04 through its human legitimacy gate. No install task here |
</threat_model>
<verification>
- `cd core && CARGO_INCREMENTAL=0 cargo test --package archipelago music::` green
- `cd core && CARGO_INCREMENTAL=0 cargo build --package archipelago` exits 0
- `grep -c 'starts_with("music.")' core/archipelago/src/api/rpc/dispatcher.rs` == 1
- Index field names match `13-MUSIC-MODEL.md`
</verification>
<success_criteria>
The node has a real, persisted, incrementally-refreshed music library with a versioned schema
and an atomic write, exposed over an authenticated `music.*` surface — and it got there without
any plan on the control or content track depending on it.
</success_criteria>
<output>
Create `.planning/phases/13-aiui-functional-conversational-node-control-and-content-surf/13-07-SUMMARY.md` when done
</output>