|
| 1 | +# Immutable Session Source Attribution Implementation Plan |
| 2 | + |
| 3 | +> **For agentic workers:** REQUIRED SUB-SKILL: Use |
| 4 | +> superpowers:subagent-driven-development (recommended) or |
| 5 | +> superpowers:executing-plans to implement this plan task-by-task. Steps use |
| 6 | +> checkbox (- [ ]) syntax for tracking. |
| 7 | +
|
| 8 | +**Goal:** Finish PR #1170 as a filesystem-only source-label feature whose |
| 9 | +machine attribution is immutable after first ingestion. |
| 10 | + |
| 11 | +**Architecture:** Existing archive rows are authoritative for machine |
| 12 | +attribution, including during refreshes and full database rebuilds. Source |
| 13 | +configuration supplies a label only when a session is new. Comparison-only path |
| 14 | +cleanup is separate from stored path spelling, and the already-merged DuckDB |
| 15 | +implementation remains owned by main. |
| 16 | + |
| 17 | +**Tech Stack:** Go, SQLite/FTS5, TOML configuration, filesystem-backed parser |
| 18 | +providers, testify, Git, and Markdown. |
| 19 | + |
| 20 | +## Global Constraints |
| 21 | + |
| 22 | +- Preserve behavior and query-shape parity between SQLite and PostgreSQL where |
| 23 | + both backends participate; this change does not add PostgreSQL behavior. |
| 24 | +- Never delete, recreate, or destructively migrate the persistent SQLite |
| 25 | + archive. |
| 26 | +- Keep watcher and periodic sync work bounded by the changed batch. |
| 27 | +- Keep session_sources filesystem-only and additive to legacy source arrays. |
| 28 | +- Preserve native session-ID deduplication. |
| 29 | +- Use testify for all new or modified Go assertions. |
| 30 | +- Every behavior test must assert persisted or returned behavior and fail before |
| 31 | + its production fix is applied. |
| 32 | +- Run go fmt ./... and go vet ./... after Go changes. |
| 33 | +- Do not push or change branches as part of this local implementation. |
| 34 | + |
| 35 | +______________________________________________________________________ |
| 36 | + |
| 37 | +### Task 1: Synchronize main and remove duplicate DuckDB work |
| 38 | + |
| 39 | +**Files:** |
| 40 | + |
| 41 | +- Merge: current origin/main |
| 42 | +- Restore from origin/main: internal/duckdb/connect.go |
| 43 | +- Restore from origin/main: internal/duckdb/probe.go |
| 44 | +- Restore from origin/main: internal/duckdb/push.go |
| 45 | +- Restore from origin/main: internal/duckdb/rebuild.go |
| 46 | +- Restore from origin/main: internal/duckdb/schema.go |
| 47 | +- Restore from origin/main: internal/duckdb/sync.go |
| 48 | +- Restore from origin/main: internal/duckdb/sync_test.go |
| 49 | + |
| 50 | +**Interfaces:** |
| 51 | + |
| 52 | +- Consumes: merged PR #1302 on origin/main |
| 53 | + |
| 54 | +- Produces: a branch with no DuckDB diff relative to its updated base |
| 55 | + |
| 56 | +- [ ] **Step 1: Fetch and merge current main without changing branches** |
| 57 | + |
| 58 | + ``` |
| 59 | + git fetch origin main |
| 60 | + git merge --no-commit --no-ff origin/main |
| 61 | + ``` |
| 62 | + |
| 63 | + Inspect every conflict before resolving it. |
| 64 | + |
| 65 | +- [ ] **Step 2: Resolve DuckDB files to origin/main** |
| 66 | + |
| 67 | + Use the origin/main versions of all seven DuckDB files. Resolve other |
| 68 | + conflicts by preserving current-main behavior plus the filesystem-source |
| 69 | + feature. |
| 70 | + |
| 71 | +- [ ] **Step 3: Verify the DuckDB overlap is gone** |
| 72 | + |
| 73 | + ``` |
| 74 | + git diff --exit-code origin/main -- internal/duckdb |
| 75 | + ``` |
| 76 | + |
| 77 | + Expected: no output. |
| 78 | + |
| 79 | +- [ ] **Step 4: Commit the synchronization** |
| 80 | + |
| 81 | + ``` |
| 82 | + git commit -m "merge: sync machine source branch with main" |
| 83 | + ``` |
| 84 | + |
| 85 | +______________________________________________________________________ |
| 86 | + |
| 87 | +### Task 2: Protect immutable attribution with observable sync tests |
| 88 | + |
| 89 | +**Files:** |
| 90 | + |
| 91 | +- Modify: internal/sync/session_source_machine_test.go |
| 92 | +- Modify: internal/sync/provider_process_test.go |
| 93 | +- Modify: internal/db/source_path_hints_test.go |
| 94 | +- Modify: cmd/agentsview/sync_worker_test.go when full-resync coverage belongs |
| 95 | + at the command boundary after the merge |
| 96 | + |
| 97 | +**Interfaces:** |
| 98 | + |
| 99 | +- Consumes: Engine.SyncAllSince, Engine.SyncPathsContext, provider-backed |
| 100 | + writes, and the full-resync archive-copy path |
| 101 | + |
| 102 | +- Produces: regression coverage that reads persisted session rows and identity |
| 103 | + state after configured labels change |
| 104 | + |
| 105 | +- [ ] **Step 1: Write or revise the active-session regression** |
| 106 | + |
| 107 | + Create a session under a root labeled oldbox, sync it, change the configured |
| 108 | + root label to newbox, modify the file, sync again, and assert the persisted |
| 109 | + session still reports oldbox. The production mutation this catches is |
| 110 | + assigning the currently configured label on an existing-row upsert. |
| 111 | + |
| 112 | +- [ ] **Step 2: Run the active-session test and verify RED** |
| 113 | + |
| 114 | + ``` |
| 115 | + CGO_ENABLED=1 go test -tags fts5 ./internal/sync \ |
| 116 | + -run 'Test.*Machine.*Immutable' -count=1 |
| 117 | + ``` |
| 118 | + |
| 119 | + Expected: FAIL because an existing write path still adopts newbox. |
| 120 | + |
| 121 | +- [ ] **Step 3: Write or revise trash and full-resync regressions** |
| 122 | + |
| 123 | + Trash a labeled session, change its configured root label, and run both an |
| 124 | + ordinary source refresh and the command full-resync path. Assert the session, |
| 125 | + trash timestamp, project-identity snapshot, and observation retain oldbox. The |
| 126 | + production mutation this catches is any machine-only update during trash |
| 127 | + handling or orphan copying. |
| 128 | + |
| 129 | +- [ ] **Step 4: Run the new cases and verify RED** |
| 130 | + |
| 131 | + ``` |
| 132 | + CGO_ENABLED=1 go test -tags fts5 ./internal/sync ./cmd/agentsview \ |
| 133 | + -run 'Test.*(Trashed|FullResync).*Machine' -count=1 |
| 134 | + ``` |
| 135 | + |
| 136 | + Expected: at least one assertion sees newbox before production cleanup. |
| 137 | + |
| 138 | +- [ ] **Step 5: Confirm newly discovered sessions use the current label** |
| 139 | + |
| 140 | + After changing the configured label, add a second session and assert its |
| 141 | + stored machine is newbox while the first remains oldbox. |
| 142 | + |
| 143 | +______________________________________________________________________ |
| 144 | + |
| 145 | +### Task 3: Remove continuous reattribution machinery |
| 146 | + |
| 147 | +**Files:** |
| 148 | + |
| 149 | +- Modify: internal/sync/engine.go |
| 150 | +- Modify: internal/db/sessions.go |
| 151 | +- Modify: internal/db/orphaned.go |
| 152 | +- Modify: internal/db/schema.sql |
| 153 | +- Modify: internal/db/source_path_hints_test.go |
| 154 | +- Modify: internal/sync/session_source_machine_test.go |
| 155 | +- Modify: internal/sync/provider_process_test.go |
| 156 | + |
| 157 | +**Interfaces:** |
| 158 | + |
| 159 | +- Removes: DB.UpdateTrashedSessionMachine |
| 160 | + |
| 161 | +- Removes: DB.UpdateTrashedSessionMachineByPath |
| 162 | + |
| 163 | +- Removes: DB.ListActiveSessionSourceOwnershipScopesAllMachinesPage |
| 164 | + |
| 165 | +- Removes: idx_local_source_baselines_source |
| 166 | + |
| 167 | +- Preserves: existing row machine values through upsert and archive copy |
| 168 | + |
| 169 | +- [ ] **Step 1: Remove machine-only trash updates and their call sites** |
| 170 | + |
| 171 | + Delete both database methods and the engine calls that invoke them during |
| 172 | + cached-skip, provider, batch, and full-resync paths. Do not replace them with |
| 173 | + a different relabel mechanism. |
| 174 | + |
| 175 | +- [ ] **Step 2: Remove the all-machines baseline query and index** |
| 176 | + |
| 177 | + Return watcher reconciliation to the machine-scoped ownership query. Remove |
| 178 | + query-plan tests that exist only for the private implementation, retaining |
| 179 | + behavior tests for move and delete tombstoning. |
| 180 | + |
| 181 | +- [ ] **Step 3: Make full-resync archive copy preserve stored attribution** |
| 182 | + |
| 183 | + Remove snapshot and observation SQL whose only purpose is adopting a newly |
| 184 | + configured label. Ensure copied sessions remain authoritative before parser |
| 185 | + writes are considered. |
| 186 | + |
| 187 | +- [ ] **Step 4: Run focused tests and verify GREEN** |
| 188 | + |
| 189 | + ``` |
| 190 | + CGO_ENABLED=1 go test -tags fts5 \ |
| 191 | + ./internal/db ./internal/sync ./cmd/agentsview \ |
| 192 | + -run 'Machine|SourceOwnership|FullResync|SessionSource' -count=1 |
| 193 | + ``` |
| 194 | + |
| 195 | + Expected: PASS, including active, trash, full-resync, move, and delete cases. |
| 196 | + |
| 197 | +______________________________________________________________________ |
| 198 | + |
| 199 | +### Task 4: Preserve path spelling and clear lint |
| 200 | + |
| 201 | +**Files:** |
| 202 | + |
| 203 | +- Modify: internal/config/config.go |
| 204 | +- Modify: internal/config/config_test.go |
| 205 | +- Modify: internal/sync/session_source_machine_test.go |
| 206 | + |
| 207 | +**Interfaces:** |
| 208 | + |
| 209 | +- Consumes: normalizeSessionSourceDir and sessionSourceComparisonKey |
| 210 | + |
| 211 | +- Produces: stored trimmed path spelling plus cleaned comparison keys |
| 212 | + |
| 213 | +- [ ] **Step 1: Add a failing platform-neutral spelling test** |
| 214 | + |
| 215 | + Pass a structured directory containing dot segments and assert |
| 216 | + ResolveSessionSources retains the trimmed input string while deduplicating it |
| 217 | + against an equivalent legacy root through the comparison key. The production |
| 218 | + mutation this catches is returning filepath.Clean(value) from |
| 219 | + normalizeSessionSourceDir. |
| 220 | + |
| 221 | +- [ ] **Step 2: Run the config test and verify RED** |
| 222 | + |
| 223 | + ``` |
| 224 | + CGO_ENABLED=1 go test -tags fts5 ./internal/config \ |
| 225 | + -run 'Test.*SessionSource.*Spelling' -count=1 |
| 226 | + ``` |
| 227 | + |
| 228 | + Expected: FAIL because the stored structured source path is cleaned. |
| 229 | + |
| 230 | +- [ ] **Step 3: Return validated spelling from normalization** |
| 231 | + |
| 232 | + Keep whitespace trimming and NUL/empty validation, but return value. Continue |
| 233 | + calling filepath.Clean only from sessionSourceComparisonKey. |
| 234 | + |
| 235 | +- [ ] **Step 4: Replace repeated string concatenation in the sync test** |
| 236 | + |
| 237 | + Use one strings.Builder with Grow in |
| 238 | + TestMachineForPathUsesNormalizedRootSpecificity. This is test maintenance only |
| 239 | + and needs no new behavior test. |
| 240 | + |
| 241 | +- [ ] **Step 5: Verify GREEN and lint** |
| 242 | + |
| 243 | + ``` |
| 244 | + CGO_ENABLED=1 go test -tags fts5 ./internal/config ./internal/sync \ |
| 245 | + -run 'SessionSource|MachineForPath' -count=1 |
| 246 | + make lint |
| 247 | + ``` |
| 248 | + |
| 249 | +______________________________________________________________________ |
| 250 | + |
| 251 | +### Task 5: Align documentation and pull-request copy |
| 252 | + |
| 253 | +**Files:** |
| 254 | + |
| 255 | +- Modify: README.md |
| 256 | +- Modify: docs/configuration.md |
| 257 | +- Modify: docs/filesystem-sync.md |
| 258 | +- Prepare: replacement body for PR #1170 |
| 259 | + |
| 260 | +**Interfaces:** |
| 261 | + |
| 262 | +- Consumes: the immutable first-ingestion contract |
| 263 | + |
| 264 | +- Produces: user guidance with no retroactive-relabel or DuckDB claim |
| 265 | + |
| 266 | +- [ ] **Step 1: Rewrite user documentation** |
| 267 | + |
| 268 | + State that configuration changes affect only newly discovered sessions and |
| 269 | + that ordinary sync plus sync --full preserve stored labels. Describe |
| 270 | + retroactive relabeling as unsupported; do not promise a command. |
| 271 | + |
| 272 | +- [ ] **Step 2: Draft the synchronized PR description** |
| 273 | + |
| 274 | + Summarize structured filesystem sources, additive configuration, immutable |
| 275 | + attribution, native-ID deduplication, and the S3 exclusion. Remove the DuckDB |
| 276 | + section and retroactive-rebuild language. |
| 277 | + |
| 278 | +- [ ] **Step 3: Review prose for contradictions** |
| 279 | + |
| 280 | + Read the README, both guides, and draft together. Confirm every description of |
| 281 | + label changes and full sync states the same behavior. |
| 282 | + |
| 283 | +______________________________________________________________________ |
| 284 | + |
| 285 | +### Task 6: Final verification and commit |
| 286 | + |
| 287 | +**Files:** all files changed by Tasks 1-5 |
| 288 | + |
| 289 | +**Interfaces:** |
| 290 | + |
| 291 | +- Produces: a clean, committed worktree ready for the PR author's fork |
| 292 | + |
| 293 | +- [ ] **Step 1: Format and vet** |
| 294 | + |
| 295 | + ``` |
| 296 | + go fmt ./... |
| 297 | + go vet ./... |
| 298 | + ``` |
| 299 | + |
| 300 | +- [ ] **Step 2: Run the Go suites** |
| 301 | + |
| 302 | + ``` |
| 303 | + make test-short |
| 304 | + make test |
| 305 | + ``` |
| 306 | + |
| 307 | +- [ ] **Step 3: Run repository lint** |
| 308 | + |
| 309 | + ``` |
| 310 | + make lint |
| 311 | + ``` |
| 312 | + |
| 313 | +- [ ] **Step 4: Inspect the final diff and private-data scrub** |
| 314 | + |
| 315 | + ``` |
| 316 | + git diff --check |
| 317 | + git diff --stat origin/main...HEAD |
| 318 | + git diff origin/main...HEAD |
| 319 | + ``` |
| 320 | + |
| 321 | + Confirm the changed lines contain no private paths, identities, or |
| 322 | + infrastructure names. |
| 323 | + |
| 324 | +- [ ] **Step 5: Commit the completed cleanup** |
| 325 | + |
| 326 | + ``` |
| 327 | + git add README.md docs cmd internal |
| 328 | + git commit -m "fix(sync): keep source attribution immutable" |
| 329 | + ``` |
| 330 | + |
| 331 | +- [ ] **Step 6: Report the handoff** |
| 332 | + |
| 333 | + Report commit IDs, tests run, remaining environmental limitations, and the |
| 334 | + fact that no push or PR mutation was performed. |
0 commit comments