| new file mode 100644 |
| index 0000000..069ee62 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/2026-09-05-notion-ingest-design.md |
| @@ -0,0 +1,728 @@ |
| +# 🤖 memoria — Notion ingest |
| + |
| +**Status:** draft 2. Draft 1 reviewed by `claude-fable-5-1`, REQUEST |
| +CHANGES, 15 findings — §1.1 records what it changed, including three |
| +blockers and one finding that removed the enumeration design entirely. |
| +**Parent:** `2026-09-03-memoria-design.md`. Where this disagrees with the |
| +parent, the parent wins and this document is wrong — fix it in the same |
| +commit. §3 does exactly that: it changes the parent, and says so. |
| + |
| +This is the first half of the parent's **slice 3** (§11, external |
| +connectors), under the parent's §4.2: content memoria owns outright, |
| +because there is no sibling to ask. |
| + |
| +--- |
| + |
| +## 0. The one rule, as it applies here |
| + |
| +The parent's rule is *memoria never speaks a passage without asking |
| +whoever owns it whether this person may see it, right now.* For Notion |
| +there is nobody to ask, and the parent already says so (§4.2): **memoria |
| +is the app of record for what it ingests from Notion.** The sharing act |
| +at the far end was the publish. §12 makes sure the product says this |
| +rather than implying otherwise. |
| + |
| +The second rule is unchanged, and this document adds to what it covers: a |
| +Notion page's **title, body, headings and ancestry path are all content** |
| +and all sealed. Its page id is a coordinate — sealed reversibly, and |
| +separately fingerprinted for lookup (§8, and §1.1 finding 1 for why draft |
| +1's version could not work at all). |
| + |
| +--- |
| + |
| +## 1. Settled decisions |
| + |
| +| # | Decision | Because | |
| +|---|---|---| |
| +| 1 | Notion ships **without** slices 1 and 2 | §4.2 needs no sibling contract, no person ref, no model. §2 | |
| +| 2 | Enumerate by **traversal from shared roots**, never by `/v1/search` | Notion documents that search "is not guaranteed to return everything". §6 — this is the finding that rebuilt the design | |
| +| 3 | A sync **never deletes on absence** | memoria is replacing Notion; a page vanishing upstream is the expected end state. **This changes the parent** — §3 | |
| +| 4 | One Notion page → one memoria document | The page is the unit people share, name and link to. §5 | |
| +| 5 | Ancestry is carried, in **its own index column at low weight** | Nesting is how a workspace is navigated; the existing heading column is 3×-weighted and would rank the whole workspace above the answer. §7 | |
| +| 6 | `Notion-Version: 2026-03-11`, pinned as a constant with a gate | The object model changed incompatibly at 2025-09-03 (data sources). An unpinned client is one release from silent extraction loss. §1.1 finding 5 | |
| +| 7 | The credential is sealed under its own HKDF purpose | Parent §3.5, verbatim: "so a chunk-store disclosure does not yield them" | |
| +| 8 | The run is admin-triggered and resumable, not scheduled | The app sleeps. §9 says what that costs and refuses to hide it | |
| +| 9 | **One package may reach the network**, host-pinned at runtime | The existing gate forbids all egress; the fix is to narrow it to something stronger, never to delete it. §11 | |
| +| 10 | Ship in **three stages**, ranking first | Stage 1 re-ranks every existing document and deserves its own review. §14 | |
| + |
| +### 1.1 What the Fable review changed |
| + |
| +Recorded rather than quietly folded in, in the parent's §1.2–§1.4 style. |
| +Findings 1, 2, 3 were blockers; finding 4 was the expensive one. |
| + |
| +- **The ledger could not fetch anything (blocker).** Draft 1 stored only |
| + a *keyed* page id, and `seal.Fingerprint` is HMAC-SHA256 |
| + (`seal/seal.go:106`) — one-way. Phase 2 was specified as |
| + `GET /v1/blocks/{page_id}/children` over ledger rows, so a resumed run |
| + held 1,930 fingerprints and no URL to call. "Resumption is free and |
| + needs no separate mechanism" was true only because the mechanism was |
| + absent. Now: a **sealed** page id (`PurposeCoordinate`, reversible) |
| + plus the print for lookup. |
| +- **Search cannot enumerate, and there is no signal when it fails to |
| + (finding 4, and it removed the design).** Draft 1 promised a total |
| + decoder: "every enumeration either yields every item or fails." Notion's |
| + own documentation says the search endpoint is not optimized for |
| + "exhaustively enumerating through all the documents that a bot has |
| + access to" and that **"search is not guaranteed to return everything"**, |
| + with no field in the response saying so. A clean run to |
| + `has_more: false` can be missing pages, and draft 1 would have reported |
| + a complete import. That is the codebase's worst failure shape produced |
| + by the choice of endpoint, and **gate N1 would have stayed green while |
| + the property it named was violated** — a guard written to pass, at the |
| + architecture level rather than the test level. §6 replaces it with |
| + traversal. The reviewer proposed a `request_status` field as the fix; |
| + that field is not in the documentation and the real situation is worse |
| + than the finding, which is why §6 is a redesign and not a decoder patch. |
| +- **Single-flight wedged the connector on the normal path (blocker).** |
| + The run is a detached goroutine; when CARLOS sleeps the app mid-fetch |
| + the goroutine dies without writing anything and the row stays |
| + `fetching`. Nothing in draft 1 ever wrote `interrupted`, so every later |
| + sync was refused as "already running", and §12's "interrupted at 412 of |
| + 1,930" copy had no writer. Draft 1 described the deadlock as a feature |
| + ("survives a restart that left a run marked active"). Now: boot-time |
| + reclaim plus a heartbeat, §9. |
| +- **Gate N3 could not go red (finding 7, the best one).** FTS5's `bm25()` |
| + sums per-column contributions, so a row matching in *both* path and |
| + body outranks a path-only match under any positive weights. Draft 1's |
| + fixture stayed green with the mutation applied — the gate written to |
| + catch §7's scoring trap could not catch it. §13 specifies the fixture |
| + that reproduces the trap and requires the flip to be **measured**, not |
| + assumed. |
| +- **Renames and moves never reached memoria.** `store.go:113` short- |
| + circuits on an unchanged body print without touching `sealed_title`, |
| + so a renamed page keeps its old title forever; an *ancestor* rename |
| + does not change a descendant's `last_edited_time` at all, so the fetch |
| + phase skipped it before `Ingest` was even called. And the ledger had |
| + one edit-time column, so the phase-2 comparison compared it against |
| + itself. §10. |
| +- **Empty pages could not be ingested.** `store.go:73` refuses an empty |
| + body; real workspaces are full of title-only pages and placeholders. |
| + Draft 1's gate N2 asserted an empty body "ingests" and would have |
| + failed before its mutation was applied. §5. |
| +- **A page that failed to fetch had nowhere to be recorded**, so three |
| + unreadable pages inside 1,930 rendered as a complete import. §8 gives |
| + every ledger row a state and an error code. |
| +- **"Owners only" named a distinction slice 0 does not have.** |
| + `MEMORIA_OWNERS` is the entire admission list (`config.go:65`, |
| + `auth.go:28`); today "owners" ≡ "signed in". Worse, `AppForTest` |
| + bypasses `RequireSession`, so no identity reaches the request context |
| + and the gate could not be written at all. §12. |
| +- **Decision 3 contradicts the parent, and draft 1 claimed it agreed.** |
| + Draft 1 cited parent §3.6 as proof. §3 now argues it as a change and |
| + amends the parent in this commit, which is what this document's own |
| + header requires. |
| +- Smaller: `Retry-After` applies to **529** as well as 429; a Notion |
| + 404 body quotes the page id *and* the connection name, which is a |
| + concrete reason for §8's code-not-message rule; the sync route must be |
| + mounted inside the CSRF and session group; `total` needs the codebase's |
| + `-1` sentinel; there is no disconnect affordance though parent §3.6 |
| + promises one; `context.WithoutCancel` in `store.go:189` is a bounded |
| + post-commit refresh, not a precedent for a multi-minute job. |
| + |
| +--- |
| + |
| +## 2. Why this is unblocked, when the parent sequenced it fourth |
| + |
| +The parent's §11 puts external connectors in slice 3, behind the sibling |
| +contract (slice 1) — which parent §13 records as "blocked on agreement, |
| +not on code", in *other repositories*. That ordering was never a |
| +dependency. Slice 3's stated requirements are least-privilege credentials |
| +(parent §3.5) and gate 7, both local. Notion content is memoria-owned |
| +under §4.2, so: |
| + |
| +- **No S2 call.** There is no sibling that could answer one. |
| +- **No person ref**, so no dependency on home §13 step 4. |
| +- **No content egress.** Bodies come *in*. The footer's claim — "no |
| + document ever leaves this instance" — stays true and |
| + `TestFooterDoesNotOverclaim` stays green. |
| + |
| +Draft 1 said "Notion sits on slice 0 and nothing else", which the review |
| +corrected: it also sits on slice 0's **test infrastructure**, because |
| +`TestNoOutboundNetworkInAppCode` forbids the client this design needs |
| +(§11), and on `AppForTest` growing an identity seam (§12). Those are |
| +dependencies. They are in this repository, which is the point. |
| + |
| +--- |
| + |
| +## 3. Deletion: this changes the parent |
| + |
| +**Custody** is the integration's share list. An internal integration sees |
| +only what is shared with it, and that act of sharing is the publish |
| +(parent §5). memoria adds no second selector — parent §3.5's |
| +"owner-selected subsets" is satisfied by the far end already having a |
| +per-page control, and a second list to keep in step with a list is the |
| +shape decision 11 rejected. |
| + |
| +**Authorization** is memoria's own admission gate and nothing else |
| +(§4.2). Every admitted person sees every Notion document memoria holds. |
| + |
| +**On absence, memoria does nothing.** |
| + |
| +| Upstream event | memoria does | |
| +|---|---| |
| +| Page edited, renamed, or moved | re-ingest (§10 — all three, which draft 1 got wrong) | |
| +| Page added under a shared root | ingest | |
| +| Page unshared, trashed, or the token revoked | **nothing.** The document stands until someone deletes it *in memoria* | |
| + |
| +**This is a change to the parent, not an application of it.** Parent §4.2 |
| +says "§5.1's enumeration diff is a convenience that eventually notices |
| +upstream deletions", and parent §5.1's Notion row records `Delete: diff |
| +only`. Draft 1 removed the diff and cited parent §3.6's README paragraph |
| +as evidence the parent already agreed. It is not evidence: §3.6 is about |
| +*disconnecting a space or revoking access*, a different event from a |
| +single page archived while the connection stays live. The review was |
| +right to call that smuggled. |
| + |
| +**The argument for the change, made properly.** memoria is being adopted |
| +as the replacement for Notion, not as a mirror of it. Under a |
| +replacement, archiving a page in Notion *after* migrating it is the |
| +intended workflow — and a diff-and-delete connector reads that as |
| +"forget it", deleting from the app of record the document the migration |
| +existed to preserve. A convenience that eventually notices deletions is |
| +the wrong convenience when deletion upstream is the goal. |
| + |
| +**What it costs, and the mitigation.** A page deleted upstream *because |
| +it should not have been shared* stays indexed and readable by every |
| +admitted person, with no notice. That is a real cost and §12 puts it in |
| +the interface. Every ledger row therefore carries **`last_seen_at`**, |
| +written on every enumeration — it costs nothing, since traversal already |
| +touches every row — so the "no longer in Notion" view is buildable later |
| +without a migration. Decision 3 keeps the documents; it does not license |
| +being unable to list them. |
| + |
| +**Parent amendments required in this commit:** §4.2's "eventually notices |
| +upstream deletions" and §5.1's Notion `Delete` cell. Not doing so leaves |
| +two specs that contradict, which is the drift CLAUDE.md exists to |
| +prevent. |
| + |
| +--- |
| + |
| +## 4. The Notion object model we are coding against |
| + |
| +Pinned at **`Notion-Version: 2026-03-11`**, a constant in the client with |
| +a gate asserting it. This matters more than a pin usually does: version |
| +`2025-09-03` changed the object model incompatibly. A **database** now |
| +contains one or more **data sources**; a row's parent is |
| +`{type: "data_source_id", data_source_id, database_id}`; |
| +`/v1/databases/{id}/query` is replaced by `/v1/data_sources/{id}/query`. |
| +`2026-03-11` further renames `archived` → `in_trash` and the |
| +`transcription` block type → `meeting_notes`. Draft 1 described the |
| +pre-2025 model and listed pinning as an *open item*; it is a decision |
| +with consequences for every parent link the ancestry walk reads. |
| + |
| +Trashed content is skipped: `in_trash: true` is not ingested, and an |
| +already-ingested page that goes to trash is an absence, which by decision |
| +3 means nothing happens. |
| + |
| +--- |
| + |
| +## 5. What a Notion page becomes |
| + |
| +One page → one `Document`, `Source = "notion"`. |
| + |
| +**Body** is Markdown extracted from the block tree: paragraph, |
| +`heading_1..3` → `#`/`##`/`###`, bulleted and numbered list items, |
| +`to_do` → `- [ ]`/`- [x]`, `quote`, `callout`, `toggle` (heading line |
| +then children), `code` (fenced, language preserved), `divider`, `table` |
| +(pipe table), `image`/`file`/`video`/`pdf`/`meeting_notes` (caption and |
| +filename only — memoria indexes text), and `child_page`/`child_database` |
| +as a one-line reference, **never inlined**: they are their own documents |
| +and inlining would duplicate every passage into all of its ancestors. |
| +Rich text uses Notion's `plain_text`, so an inline equation or mention |
| +degrades to the text Notion itself renders. |
| + |
| +**Unknown block types produce nothing and increment a counter.** Not an |
| +error — Notion adds block types, and a connector that fails a run on an |
| +unrecognised one breaks on Notion's release schedule. But not silence |
| +either: the run reports "N blocks of a type memoria does not read", so a |
| +page that came in thin has a reason attached rather than looking like a |
| +thin page. |
| + |
| +**Empty pages are ingested as title-only documents.** Draft 1 said an |
| +empty page "ingests as an empty body"; `store.go:73` and `:101` refuse |
| +exactly that, so the gate asserted something the store forbids. Real |
| +workspaces are full of placeholders and title-only rows, and both |
| +available alternatives were wrong: failing the run lets one placeholder |
| +block the whole import, and skipping makes the page an absence with no |
| +row to explain it. So `Store.Ingest` grows an explicit |
| +`AllowTitleOnly` option — off for uploads and paste, where an empty |
| +submission is a user error worth refusing, on for connectors, where it is |
| +a fact about the source. The document's body is its title; it chunks to |
| +one passage; the ledger row records `empty` and the run counts it. |
| + |
| +**Databases.** A database is not a document; each **row is a page** and |
| +becomes one, with properties rendered as a leading key/value block: |
| + |
| +``` |
| +Status: Shipped |
| +Owner: Ana |
| +Due: 2026-03-01 |
| + |
| +<the row page's blocks> |
| +``` |
| + |
| +`relation` renders the related pages' titles, `rollup` and `formula` |
| +their computed value. **This is the part most likely to be wrong** — a |
| +40-row database becomes 40 one-paragraph documents, which may be noise |
| +rather than memory. §15 records it as unanswerable without a real |
| +workspace. |
| + |
| +**Title** is the page's title property. A database row with an empty |
| +title gets its data source's name plus its first non-empty property, |
| +because `firstLine(body)` over a property block titles every document |
| +`Status: Shipped`. |
| + |
| +**External id** is the Notion page id — stable across edits, renames and |
| +moves, which is what `Document.ExternalID`'s contract requires. It is |
| +keyed through `seal.Fingerprint` like every other external id (parent |
| +§3.2: a coordinate must not be an oracle). |
| + |
| +--- |
| + |
| +## 6. Enumeration: traversal, not search |
| + |
| +**Draft 1 enumerated with `/v1/search` and promised a total decoder. It |
| +cannot be done.** Notion's documentation is explicit that search "is not |
| +optimized for … exhaustively enumerating through all the documents that a |
| +bot has access to", that **"search is not guaranteed to return |
| +everything"**, and that its index is eventually consistent. Crucially |
| +there is *no field in the response that says the result was partial*. So |
| +a run that pages cleanly to `has_more: false`, with no 429 and no error, |
| +may simply be missing pages — and would report a complete import. No |
| +decoder, however total, can detect it. |
| + |
| +The same documentation supplies the way out: |
| + |
| +> any pages or databases that are **directly shared** with a connection |
| +> are **guaranteed to be returned** |
| + |
| +So search is reliable for exactly one thing, and that is what we use it |
| +for: |
| + |
| +1. **Roots.** `POST /v1/search` finds the directly-shared pages and data |
| + sources — guaranteed complete for *that* set, which is the set an |
| + admin chose. |
| +2. **Traversal.** From each root, walk down: `GET /v1/blocks/{id}/children` |
| + for `child_page` and `child_database`, and |
| + `POST /v1/data_sources/{id}/query` for rows. Both are cursor-paginated |
| + with a proper `has_more`/`next_cursor` contract, and both are |
| + *complete* — they enumerate a parent's actual children, not an index's |
| + opinion of them. |
| + |
| +**Ancestry falls out for free.** You arrive at every page *from* its |
| +parent, so the path is known at the moment of fetch. Draft 1 walked |
| +`parent` links upward, which needed raw ids of pages possibly outside the |
| +run, had to hop `block_id` parents for inline child pages, and truncated |
| +silently when an ancestor was unshared. None of that exists now. |
| + |
| +**The decoder is total** — the parent's §4.1 rule, applied where it can |
| +actually hold, because these endpoints support it: |
| + |
| +- `has_more: true` with no `next_cursor` → error. |
| +- A repeated cursor → error. (A looping API would otherwise produce a run |
| + that never finishes and never says why.) |
| +- A short page with `has_more: true` → accepted; the *cursor* is the |
| + completeness signal, not the count. |
| +- `429`/`529` past the retry budget → error. Both carry `Retry-After` in |
| + integer seconds and both are honoured exactly; `500/502/503/504` are |
| + retried only because every request here is a read. |
| +- An id seen twice → deduplicated **and counted**; silent dedup hides a |
| + traversal bug. A cycle (a page reachable from itself) is bounded by the |
| + visited set and counted the same way. |
| + |
| +There is no partial return anywhere in the package's surface: a caller |
| +cannot mistake "we got some" for "that is all there is", because "some" |
| +is not a value the package produces. |
| + |
| +**What this still does not guarantee**, said plainly: memoria enumerates |
| +everything reachable from the roots the integration was given. A page |
| +shared with nobody, or in a space the integration cannot see, is not |
| +missing — it was never in scope. The status page says how many roots were |
| +found and names them, so "I expected more" is answerable. |
| + |
| +--- |
| + |
| +## 7. Ancestry, and the scoring trap in it |
| + |
| +A Notion page's meaning is largely positional: *"Week 1"* under |
| +*"Handbook › Onboarding"* is onboarding week 1; the same page floating |
| +free is nothing. memoria's chunk trail today comes only from ATX headings |
| +inside one document, so a naive import loses all of it. |
| + |
| +Each document therefore carries a **path**: the ancestor titles from the |
| +root down, known from traversal (§6). |
| + |
| +**The trap.** The index has two FTS5 columns scored |
| +`bm25(chunks, 3.0, 1.0)` — heading 3×, body 1× (`index.go:165`). |
| +Prepending the path onto the heading trail puts the workspace's top-level |
| +names into **every chunk of every page beneath them at triple weight**. A |
| +search for *handbook* returns the whole handbook above the page that |
| +answers the question, and it gets worse with depth. The symptom is bad |
| +ranking, not an error, which is the kind that survives. |
| + |
| +**Therefore:** a third FTS column, `path`, scored |
| +`bm25(chunks, 3.0, 1.0, 0.5)`. `chunk.Split` is unchanged; a sibling |
| +`chunk.SplitUnder(path, text)` carries the path without merging it into |
| +the document's own trail. `Store.Ingest` moves to an options struct — |
| +one production call site (`handlers.go:183`) and one dev tool |
| +(`tools/seed/main.go:115`). |
| + |
| +**The 0.5 is a guess**, chosen as half the body's weight because nothing |
| +has measured it, and the code says so where the constant lives. §15 keeps |
| +it open. What is *not* a guess is that the gate under it must actually |
| +fail when the weighting is wrong — see §13 N3, which is where draft 1's |
| +version fell down. |
| + |
| +--- |
| + |
| +## 8. Data |
| + |
| +Additive-only migrations, per AGENTS.md. |
| + |
| +**`Connector`** — `kind` (`"notion"`), `sealed_token` |
| +(`PurposeCredential`), `workspace_print`, `last_verified_at`, |
| +timestamps. |
| + |
| +**`SyncRun`** — `connector_id`, `state` |
| +(`enumerating|fetching|done|failed|interrupted`), `roots`, `total`, |
| +`fetched`, `skipped`, `empty`, `failed_pages`, `unread_blocks`, |
| +`error_code`, `heartbeat_at`, timestamps. `total` is `-1` until |
| +enumeration completes, following `handlers.go:62`'s sentinel — a total |
| +that could not be determined is not zero pages. |
| + |
| +**`error_code` is a code, never a message**, and Notion supplies the |
| +concrete reason: a 404 body reads *"Could not find database with ID: |
| +be907abe-… Make sure the relevant pages and databases are shared with |
| +your connection \"My Connection\""* — a page id and a connection name, |
| +one `%w` away from a log line. The closed set is `unauthorized`, |
| +`rate_limited`, `traversal_failed`, `extract_failed`, `network`, |
| +`interrupted`; the UI maps it to copy as `flashFor` already does. |
| + |
| +**`NotionPage`** — the ledger, keyed per **connector**, not per run, |
| +which is what makes a second run resume rather than restart: |
| + |
| +| Column | Why | |
| +|---|---| |
| +| `page_print` | keyed page id — lookup, and joins to `Document.ExternalID` | |
| +| `sealed_page_id` | the id itself, sealed under `PurposeCoordinate`. **Reversible**, because the fetch phase needs the id back (§1.1 finding 1) | |
| +| `sealed_path` | the ancestor trail at enumeration time | |
| +| `path_print` | keyed path — lets a move or an ancestor rename be detected without opening the row | |
| +| `seen_edited_time` | what the last enumeration saw | |
| +| `ingested_edited_time` | what memoria actually ingested. **Two columns, because one compares against itself** (§1.1) | |
| +| `state` | `pending\|fetched\|empty\|failed` | |
| +| `error_code` | per row, so one bad page is not a whole bad run | |
| +| `last_seen_at` | §3's mitigation | |
| +| `document_id` | the row it produced | |
| + |
| +**`Document.SealedPath`** under a new `seal.PurposePath`. A path is |
| +content: *"Acquisitions › Project Bluebird"* names a thing whether or not |
| +anyone opens the page. Sealing it under the title's purpose would be |
| +cheaper and is refused — one HKDF purpose per kind of thing sealed. |
| + |
| +New purposes: `PurposePath`, `PurposeCredential`, `PurposeCoordinate`. |
| + |
| +No request body is bound onto a GORM model; the connector form goes |
| +through an explicit `map[string]any` and a `.Select` allowlist. Neither |
| +`scope.Owned` nor `scope.OwnedBy` appears — these rows are team-shared, |
| +which is AGENTS.md's first mechanical rule. |
| + |
| +--- |
| + |
| +## 9. The run loop, and the sleep it cannot hide |
| + |
| +An owner posts to `/connectors/notion/sync`, **mounted inside the |
| +`csrf.Protect` + `requireSession` group** (`app.go:51,60`) — a route on |
| +the outer mux bypasses both, as `/static/` does. The handler starts the |
| +run and redirects to a status page that refreshes with |
| +`<meta http-equiv="refresh">`. Scriptless, per AGENTS.md. |
| + |
| +**Two phases, both checkpointed.** *Enumerate*: traverse from the roots |
| +(§6), upserting ledger rows and writing `seen_edited_time`, |
| +`sealed_path`, `path_print` and `last_seen_at`. *Fetch*: walk rows needing |
| +work (§10), extract, `Ingest`, set `ingested_edited_time` and `state`. |
| +The checkpoint is the row itself, so an interruption costs the page in |
| +flight. |
| + |
| +**Single-flight, and the deadlock draft 1 shipped.** A run holds the |
| +connector via a conditional update. Draft 1 called the lock's survival |
| +across a restart a feature; it is the bug. The goroutine dies when CARLOS |
| +sleeps the process, having written nothing, and the row stays `fetching` |
| +— so every later sync is refused as "already running", permanently, on |
| +the *normal* path. Two mechanisms fix it: |
| + |
| +- **At boot, any run in `enumerating` or `fetching` becomes |
| + `interrupted`.** There can be no live run at boot, so this is sound by |
| + construction rather than by timeout. |
| +- **A `heartbeat_at` written every N pages**, so a run wedged inside a |
| + *live* process is reclaimable after a stated interval rather than |
| + never. |
| + |
| +**What the sleep costs, said out loud.** CARLOS sleeps an idle app; a |
| +sleeping app syncs nothing. At ~3 requests/second a large workspace is |
| +tens of minutes, so runs will be interrupted. The status page says the |
| +sync progresses while the page is open and resumes where it stopped. |
| + |
| +And the honest naming, which draft 1 fudged: this is **a request-spawned |
| +goroutine that runs until the platform sleeps the process, kept alive by |
| +an open status tab** — not a background worker, and `context.WithoutCancel` |
| +here is not the same thing as `store.go:189`'s bounded post-commit index |
| +refresh. It also sits against parent §3.5's "memoria hibernates": an open |
| +status tab deliberately prevents that, for as long as it is open. §15 |
| +records the scheduler that would replace it. |
| + |
| +--- |
| + |
| +## 10. Re-ingest: edits, renames, and moves |
| + |
| +Draft 1 handled only edits, and silently. Three conditions mark a ledger |
| +row as needing work: |
| + |
| +1. `seen_edited_time != ingested_edited_time` — the page changed. |
| +2. `path_print` changed — the page moved, **or an ancestor was renamed**. |
| + An ancestor rename does not touch a descendant's `last_edited_time` at |
| + all, so without this the descendants keep a stale path forever. |
| +3. `state` is `pending` or `failed`. |
| + |
| +And in the store: `Ingest`'s unchanged-body short-circuit |
| +(`store.go:113`) currently returns without touching `sealed_title`, so a |
| +renamed page keeps its old title indefinitely. It must **update title and |
| +path when only those changed, without bumping the revision** — a revision |
| +that moves without the content moving would make every cached |
| +authorization stale for no reason, which is the comment already at that |
| +line, and it stays true. |
| + |
| +--- |
| + |
| +## 11. Egress, and the gate that forbids it |
| + |
| +`internal/memoriatest/gates_test.go:92`'s |
| +`TestNoOutboundNetworkInAppCode` walks `internal/memoria/**` and `cmd/**` |
| +and fails on `*http.Client`, `http.NewRequest`, `net.Dial` and more. It is |
| +slice 0's enforcement of the parent's AI-off floor. **A Notion client |
| +trips it on its first line, and draft 1 did not mention it** — leaving |
| +the implementer's cheapest move to be widening the gate, with no rule for |
| +how far. |
| + |
| +The rule: **the gate is narrowed to something stronger, never deleted.** |
| + |
| +- The source grep keeps its blanket ban over `internal/memoria/**` and |
| + `cmd/**`, with exactly one exemption: `internal/memoria/notion`. |
| +- That exemption is paid for by a **runtime** guard, which is stronger |
| + than the grep it replaces there. The package's client is built on a |
| + transport that **refuses any host but `api.notion.com`** and **refuses |
| + to follow redirects at all**. Both matter: the token travels in an |
| + `Authorization` header, and while Go drops that header across hosts, a |
| + *same-host* redirect keeps it, and a base URL built from stored input |
| + would send it anywhere. The base URL is a constant; nothing |
| + configurable feeds it. |
| +- Gate N8 asserts both, and gate N9 asserts no *other* package acquired a |
| + client. |
| + |
| +**Leak list additions** to parent §3.2: |
| + |
| +- memoria now holds a Notion token, sealed under its own purpose. The |
| + parent's live-host entry already names "§5.1's connector credentials"; |
| + this makes it real rather than prospective. |
| +- memoria now dials `api.notion.com`, so Notion learns the instance's |
| + egress address and sync cadence. No content of the team's goes *out* — |
| + the traffic is Notion's own content coming in — but the parent lists |
| + the sign-in dial for carrying no content either, and the same standard |
| + applies. |
| +- The ledger leaks a page count and an edit-time distribution from the |
| + bucket. |
| + |
| +The AI-off floor is unchanged: with the assistant off, no document of the |
| +team's leaves the instance, at ingest or at query. |
| + |
| +--- |
| + |
| +## 12. Credentials, privilege, and surfaces |
| + |
| +**The token** is a Notion internal integration token, pasted by an owner, |
| +sealed under `PurposeCredential`, never rendered back — not even masked, |
| +because a masked secret is still a secret in the DOM. Verified on save |
| +with one `/v1/users/me` call, so a bad token fails at the form rather |
| +than mid-run. Rotation is replacing the row; there is no key history, |
| +because a replaced token has no use. **Disconnect removes the row** — |
| +parent §3.6 promises "disconnecting" and draft 1 gave no affordance for |
| +it. Disconnecting deletes the credential and stops future syncs; by |
| +decision 3 it deletes no documents, and the confirmation says so in those |
| +words. |
| + |
| +**Least privilege** (parent §3.5, "a requirement, not a nicety"): a |
| +read-only internal integration with no user information, shared with the |
| +specific pages to import rather than the workspace root. The setup copy |
| +says this because the connector cannot enforce it — Notion decides what a |
| +token can do, and memoria can only ask for the right one and refuse to |
| +pretend otherwise. |
| + |
| +**"Owner" is `Config.Owners`, and in slice 0 that is the whole admitted |
| +set.** Draft 1 wrote "owners only" as though it were a restriction; it |
| +is not one today, and stating it as a gate while it gates nothing is the |
| +pattern AGENTS.md opens with. So: the routes are guarded by an explicit |
| +`requireOwner` that reads the session identity and checks |
| +`Config.Owners`; it is *currently equivalent* to `requireSession` and the |
| +code says so at the definition. To make it testable at all, |
| +**`AppForTest` grows an injectable identity** — today it bypasses |
| +`RequireSession` entirely, so no identity reaches the context and no such |
| +gate could be written. §15 records that slice 1, which replaces the |
| +owners list with S7, must supply an owner predicate or these routes lose |
| +their guard. |
| + |
| +**Surfaces.** `/connectors` lists connectors; `/connectors/notion` is |
| +setup, status and history. Scriptless, attribute grammar, colour tokens, |
| +`theme.css` linked. Three things the interface must say, each a place |
| +this design could mislead: |
| + |
| +1. **"memoria keeps what it imports."** On the setup page, before the |
| + token is entered — parent §3.6 has the wording. |
| +2. **Interrupted is not progress.** A run stopped by a sleep says |
| + `interrupted at 412 of 1,930` and offers resume; it never renders as a |
| + smaller number that looks like it is still going. |
| +3. **A count that could not be determined says so** — `total` is `-1` |
| + until enumeration finishes, and renders as "counting", never 0. |
| + |
| +Plus the run's honest totals: roots found, pages fetched, skipped, empty, |
| +**failed**, and blocks not understood. Copy goes through the copy-review |
| +gate before it is written to a file. |
| + |
| +--- |
| + |
| +## 13. Gates |
| + |
| +Every gate is written, then **broken so it goes red, then reverted** — |
| +per path, with the mutation and date recorded in the test comment. |
| + |
| +**N1 — `TestTraversalIncompleteFailsTheRun`.** A fake whose |
| +`blocks/{id}/children` returns `has_more: true` with no `next_cursor`, |
| +and one that 429s past budget. Run ends `failed` with `traversal_failed`; |
| +the status page does not say complete. *Mutation: return accumulated |
| +results with a nil error on retry exhaustion.* |
| + |
| +**N2 — `TestSearchIsNotUsedAsAnEnumerator`.** The traversal must not |
| +depend on search returning descendants: a fake whose `/v1/search` returns |
| +**only the roots** while children exist below must still import every |
| +child. *Mutation: enumerate from search results instead of traversing.* |
| +This is the gate that pins §6's whole reason for existing, and draft 1 |
| +had nothing like it. |
| + |
| +**N3 — `TestAncestryCannotOutrankContent`.** Draft 1's version could not |
| +fail: BM25 sums per-column contributions, so a row matching path *and* |
| +body beats a path-only match under any positive weights. The fixture that |
| +reproduces the real trap: **20 pages under a root titled "Handbook", none |
| +of whose bodies contain "handbook"**, and **one page under "Engineering" |
| +whose body contains it once**. Query "handbook"; the Engineering page must |
| +rank first. *Mutation: move the path into the heading column at 3.0.* |
| +**The flip must be measured and both orderings recorded in the test |
| +comment** — if the mutation does not actually flip the order, the fixture |
| +is wrong and must be strengthened until it does. A gate for a ranking |
| +property is worthless unless the ranking has been observed to change. |
| + |
| +**N4 — `TestTitleOnlyPageIsIngested`.** A page with no blocks becomes a |
| +title-only document, the ledger row is `empty`, and the run counts it. |
| +*Mutation: drop `AllowTitleOnly`, so `Ingest` refuses and the run fails.* |
| + |
| +**N5 — `TestUnreadBlocksAreCountedNotSwallowed`.** *Mutation: drop the |
| +counter increment.* |
| + |
| +**N6 — `TestAFailedPageIsNamedNotAbsent`.** One page of 5 404s. The run |
| +completes, `failed_pages` is 1, the row is `failed` with a code, and the |
| +status page does not read as a complete import. *Mutation: `continue` on |
| +fetch error.* |
| + |
| +**N7 — `TestSyncNeverDeletesOnAbsence`.** Two pages ingested, then a |
| +traversal returning one. Both documents survive and the run is `done` — |
| +absence is not an error either. *Mutation: add the diff-and-delete draft |
| +1's predecessor proposed.* |
| + |
| +**N8 — `TestNotionClientDialsOnlyNotionAndRefusesRedirects`.** The |
| +transport rejects any other host, and refuses a 302 even to the same |
| +host. *Mutations: change the base URL constant; enable redirects.* |
| + |
| +**N9 — `TestOnlyTheNotionPackageReachesTheNetwork`.** The narrowed source |
| +grep, with the exemption asserted to be exactly one directory. |
| +*Mutation: add an `http.Client` to `internal/memoria/handlers.go`.* |
| + |
| +**N10 — `TestRenameAndMoveReachMemoria`.** Rename a page; sync; the new |
| +title renders. Rename an *ancestor*; sync; the descendant's path updates |
| +though its own `last_edited_time` never changed. *Mutation: compare only |
| +`seen_edited_time`.* |
| + |
| +**N11 — `TestResumeContinuesRatherThanRestarts`** and |
| +**N12 — `TestInterruptedRunDoesNotWedgeTheConnector`.** Mark a run |
| +`fetching`, rebuild the app (a boot), POST sync: a new run starts and the |
| +old is `interrupted`. *Mutations: scope the ledger by run id; remove the |
| +boot-time reclaim.* |
| + |
| +**N13 — `TestNoNotionContentInLogs`** extends gate 4 to the new |
| +packages: no title, body, path, page id or token in any log line, error |
| +string or metric label, observed over a run driven end to end against the |
| +fake — whose error bodies carry a canary title and a canary page id, so |
| +the error path is actually exercised. Parent §13 notes the existing gate |
| +4's credential half cannot fail; this one is built not to repeat that. |
| +*Mutation: log the page title at Info in the fetch loop.* |
| + |
| +**N14 — `TestTokenIsNeverRendered`.** With a token stored, the connector |
| +page's HTML contains neither the whole token nor any 8-character run of |
| +it. (Draft 1 said "no substring", which every single character satisfies.) |
| +*Mutation: render it into the input's `value`.* |
| + |
| +**N15 — `TestNotionVersionIsPinned`.** The client sends |
| +`Notion-Version: 2026-03-11`. *Mutation: drop the header.* |
| + |
| +**And the run is looked at in a browser** via `tools/drive`, per |
| +AGENTS.md — which found three bugs in its first use that the whole Go |
| +suite passed over. A template test proves markup exists, not that a |
| +sync's progress reads sensibly at 40%. |
| + |
| +--- |
| + |
| +## 14. Sequencing: three stages, not one |
| + |
| +The review's last finding, and it is right — draft 1 was three pieces of |
| +work wearing one spec. |
| + |
| +**Stage 1 — ranking.** The third FTS column, `SplitUnder`, the `Ingest` |
| +options struct, `SealedPath`, `AllowTitleOnly`. No Notion code at all. |
| +This re-ranks **every existing document**, so it lands and is reviewed on |
| +its own, with N3's flip measured before anything depends on it. |
| + |
| +**Stage 2 — the client.** `internal/memoria/notion`: transport, host |
| +pin, total decoder, traversal, Markdown extraction, the fake server. Pure, |
| +no DB, no app types. Gates N1, N2, N8, N9, N15. |
| + |
| +**Stage 3 — the connector.** Credential, ledger, run loop, boot reclaim, |
| +surfaces, copy. Gates N4–N7, N10–N14. |
| + |
| +--- |
| + |
| +## 15. Open |
| + |
| +- **memoria cannot replace Notion yet, and this connector does not change |
| + that.** It has no editing surface. This moves the corpus, which has to |
| + happen first under any plan, but until someone can write in memoria, |
| + Notion remains where new pages are made. Recorded so the migration is |
| + not mistaken for the replacement. |
| +- **No real workspace has been touched.** Every test runs against a fake |
| + built from the documentation. AGENTS.md's first broken rule is about |
| + guards that cannot fail for the thing they name, and a connector |
| + verified only against fixtures its author also wrote is that risk in a |
| + different costume — the review agreed and asked for it to be promoted. |
| + **A run against the real workspace is a ship requirement**, owned by |
| + Paul, blocking stage 3's merge and nothing earlier. |
| +- **The 0.5 path weight is unmeasured** (§7), and N3 measures only that |
| + the ordering flips, not that 0.5 is the right number. Tune against a |
| + real workspace and a real query set. |
| +- **Database rows as documents may be noise** (§5). One look at a real |
| + import settles it. |
| +- **The scheduler.** §9's open status tab is a workaround for having no |
| + background worker, and it defeats hibernation while open. Whether |
| + CARLOS offers a scheduler is parent §13's open question about scratch |
| + storage in a second costume; if it does, decision 8 changes. |
| +- **Slice 1 must supply an owner predicate** (§12), or `/connectors` |
| + loses its guard when the owners list is replaced by S7. |
| +- **Listing what is no longer in Notion.** Decision 3 keeps such |
| + documents and §3 stores `last_seen_at` so the view is buildable; the |
| + view itself is not built. |
| +- **Review pass on draft 2.** Draft 1: `claude-fable-5-1`, REQUEST |
| + CHANGES, 15 findings, §1.1. Draft 2 is unreviewed. |
amadan