From a12609ea5a70e6f15c909f98762dcbe71969b49f Mon Sep 17 00:00:00 2001 From: Ben Marshall Date: Fri, 26 Jun 2026 19:13:13 -0500 Subject: [PATCH] fix(onboarding): persist sharing for unanswered questions + save on click (43) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The screenshot bug: on a fresh intimacy section, clicking "share with Partner" on the whole section saved nothing. Reproduced with a decrypt E2E driving that exact path (fresh 18+ section, share before answering) — answerSharing came back empty. Root cause: submitSectionForm wrote answerSharing for ANSWERED questions only, so a share-before-answer persisted nothing. Now it persists a scope for every question the renderer explicitly scopes (answered union the sharing payload) — safe, since an unanswered question has no derived fact until it's answered, when the pre-set choice is honored. Also: a sharing change now saves IMMEDIATELY (saveScopesNow on the click), not on the ~600ms debounce; only answer typing stays debounced. 940 core + 855 desktop unit (+core: persists a scope for an unanswered question) + the screenshot-repro E2E (fresh intimacy section, share Partner, nothing answered -> every scope persists). Re-amended spec 43. Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 24 +++++ apps/desktop/e2e/launch.spec.ts | 88 +++++++++++++++++++ .../app/routes/onboarding/IntakeFormPanel.tsx | 83 +++++++++-------- ...-relationship-scoped-onboarding-sharing.md | 8 ++ .../core/src/intake/intakeService.test.ts | 20 +++++ packages/core/src/intake/intakeService.ts | 14 +-- 6 files changed, 197 insertions(+), 40 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 21a1939..e328090 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -389,6 +389,30 @@ placing anything. Specifically: A running log of durable decisions and feedback captured into the project config. Newest first. +- 2026-06-26 — **Fix #3 (onboarding sharing STILL didn't save — `answerSharing` was written for ANSWERED questions + only; SPEC 43 re-amended again; on `fix/intimacy-sharing-instant-save`).** Third report, with a screenshot: on a + fresh **intimacy** section the user clicks "share with Partner" on the whole section and it doesn't save. **My + prior two fixes never tested the intimacy section OR the share-before-answer case** — both my E2Es used `basics` + with an answer present. **Reproduced FIRST (decrypt E2E driving the exact screenshot):** fresh intimacy section, + ack 18+, open section sharing → Partner, NO answer → `answerSharing` came back **empty** on the current build → + proving the bug before any change. **Root cause:** `submitSectionForm`'s `nextSharing` loop iterated + `Object.keys(section.answers)` — ANSWERED questions only — so a section with nothing answered persisted no scope. + **Fix (core):** iterate the **union** of answered questions **and** the questions the renderer explicitly scoped + (`Object.keys(sharing)`), so a fresh-section bulk-share persists for every question (safe: an unanswered question + has no derived fact, so the scope shares nothing until it IS answered, at which point the pre-set choice is + honored instead of reverting to the category default). **Fix (panel, "right away"):** a sharing change now saves + **immediately** (`saveScopesNow` → `autoSaveForm` on the click, `complete:false`), not on the ~600ms debounce — + only answer TYPING stays debounced (the effect deps dropped `scopes`). Extracted a module-level `cleanAnswers` + shared by both. Gate green: typecheck (all), lint, **940 core + 855 desktop** unit (+core: persists a scope for + an UNANSWERED question without inventing an answer; the existing draft test), **E2E +1** (the screenshot repro: + fresh intimacy section → share Partner with nothing answered → every question's scope persists to the vault; the + prior first-time + edit-a-completed E2Es still green). **Lessons: (1) test the EXACT surface the user is on — all + three of my fixes used `basics`; the bug lived on the 18+-gated INTIMACY section + the share-BEFORE-answer path I + never drove. (2) `answerSharing` keyed off answers means sharing can't precede answering — key it off what the + user explicitly SCOPED (the renderer sends a scope per question), so "share this whole section" sticks on the + click. (3) "save right away" means on the CLICK — a debounce that's correct for typing is wrong for a discrete + sharing tap.** + - 2026-06-26 — **Fix #2 (onboarding STILL didn't save — auto-save was scoped to COMPLETED sections only; SPEC 43 re-amended; on `fix/onboarding-autosave-all-sections`).** The v0.11.1 fix shipped but the user came back (rightly angry): "YOURE OBVIOUSLY NOT PROPERLY TESTING — onboarding questions arent being saved when selected diff --git a/apps/desktop/e2e/launch.spec.ts b/apps/desktop/e2e/launch.spec.ts index 5a6eec8..0589cbc 100644 --- a/apps/desktop/e2e/launch.spec.ts +++ b/apps/desktop/e2e/launch.spec.ts @@ -7158,6 +7158,94 @@ test('onboarding: intimacy conditionals reveal under their trigger (partner / op } }); +test('onboarding: clicking "share with partner" on a section saves immediately, even before answering (the screenshot bug)', async () => { + const { userData, vault } = await seedReadyVault({ 'ai.enabled': true }); + await createNodeSecretStore(userData, passthrough).set('anthropic.apiKey', 'sk-ant-e2e'); + const fs = createNodeFileSystem(vault); + const key = await loadMasterKey(createNodeSecretStore(userData, passthrough)); + if (!key) throw new Error('share-instant e2e: master key missing'); + // A partner so the picker offers a type, + a FRESH intimacy section (nothing answered yet) — the exact + // state in the user's screenshot. + { + const now = new Date().toISOString(); + await savePerson(fs, key, { + id: 'partner-b', + schemaVersion: 1, + displayName: 'Bee', + isSubject: true, + tags: [], + createdAt: now, + updatedAt: now, + }); + await saveRelationship(fs, key, { + id: 'rel-b', + schemaVersion: 1, + fromPersonId: 'owner-1', + toPersonId: 'partner-b', + type: 'partner', + createdAt: now, + updatedAt: now, + }); + await writeEncryptedJson( + fs, + 'people/owner-1/intake/session.enc', + { + id: 'intake-share-instant', + schemaVersion: 1, + personId: 'owner-1', + status: 'inProgress', + sections: ['basics', 'life-now', 'values', 'want', 'intimacy'].map((id) => ({ + id, + status: id === 'intimacy' ? 'notStarted' : 'skipped', + restricted: id === 'intimacy', + messages: [], + answers: {}, + })), + startedAt: 'now', + updatedAt: 'now', + }, + key, + ); + } + const app = await launch(userData); + try { + const w = await app.firstWindow(); + await w.getByRole('link', { name: /Onboarding/ }).click(); + await w.getByRole('button', { name: /Intimacy & sexuality/ }).click(); + await w.getByRole('button', { name: /18 or older/ }).click(); + await expect(w.getByText('Who are you drawn to?')).toBeVisible(); + + // Open the section sharing and click Partner — WITHOUT answering any question first (the screenshot). + await w.getByRole('button', { name: /this whole section/i }).click(); + await w.getByRole('button', { name: 'Private (only me)' }).click(); + await w.getByRole('checkbox', { name: 'Partner', exact: true }).click(); // the picker checkbox, not an answer option + await w.keyboard.press('Escape'); + + // It must persist to the vault right away — even though NOTHING is answered yet. (Pre-fix: answerSharing + // is only written for ANSWERED questions, so a fresh-section bulk-share saved nothing.) + await expect + .poll( + async () => { + const intimacy = (await getIntakeSession(fs, key, 'owner-1'))?.sections.find( + (s) => s.id === 'intimacy', + ); + return Object.values(intimacy?.answerSharing ?? {}).length; + }, + { timeout: 4000 }, + ) + .toBeGreaterThan(0); + const intimacy = (await getIntakeSession(fs, key, 'owner-1'))?.sections.find( + (s) => s.id === 'intimacy', + ); + const scopes = Object.values(intimacy?.answerSharing ?? {}); + expect(scopes.every((v) => v.length === 1 && v[0] === 'partner')).toBe(true); // every question → Partner + } finally { + await app.close(); + await rm(userData, { recursive: true, force: true }); + await rm(vault, { recursive: true, force: true }); + } +}); + // 46 §7/§10: editing anatomy after rating the matrix re-LABELS the oral rows but must NOT orphan a rating — // each row is keyed by a STABLE key, so the stored ratings stay attached through the edit. This is the headline // orphan regression the spec calls out, driven through the real UI + a decrypt before/after. diff --git a/apps/desktop/src/renderer/src/app/routes/onboarding/IntakeFormPanel.tsx b/apps/desktop/src/renderer/src/app/routes/onboarding/IntakeFormPanel.tsx index a80a242..ac0d640 100644 --- a/apps/desktop/src/renderer/src/app/routes/onboarding/IntakeFormPanel.tsx +++ b/apps/desktop/src/renderer/src/app/routes/onboarding/IntakeFormPanel.tsx @@ -33,6 +33,25 @@ function sameScope(a: readonly RelationshipType[], b: readonly RelationshipType[ return a.length === b.length && a.every((t) => b.includes(t)); } +/** + * Normalize the host answer map to the bridge contract: a matrix answer is a row→point record + * (`Record`) — keep it; every other answer is a scalar/array; any OTHER non-array object isn't + * a valid intake answer, so drop it defensively (`IntakeAnswerValueSchema`). Pure — shared by the immediate + * sharing save + the debounced answer save. + */ +function cleanAnswers(src: AnswerMap): Record { + const out: Record = {}; + for (const [qid, value] of Object.entries(src)) { + if (value !== null && typeof value === 'object' && !Array.isArray(value)) { + if (Object.values(value).every((v) => typeof v === 'number')) + out[qid] = value as IntakeAnswerValue; + continue; + } + out[qid] = value as IntakeAnswerValue; + } + return out; +} + /** * A structured **form** intake section (18-personal-onboarding §14.3/§14.6) — renders the section's questions * through the shared `@selfos/answering` `QuestionnaireForm` (branch-aware, the host owns the answer state), @@ -131,11 +150,20 @@ export function IntakeFormPanel({ const promptOf = (qid: string): string => (meta.questions ?? []).find((q) => q.id === qid)?.prompt ?? qid; - // One tap applies a scope directly — no confirm (owner decision, 2026-06-26). A sensitive answer still - // STARTS Private (its category default), so sharing it stays a deliberate choice; it just takes effect (and - // auto-saves) on a single tap instead of a second confirm. - const applyScope = (qid: string, types: RelationshipType[]): void => - setScopes((s) => ({ ...s, [qid]: types })); + // A sharing change saves RIGHT AWAY (not debounced) — it's a discrete tap, and "share with partner" must + // persist the moment it's clicked. One tap applies directly, no confirm (owner decision, 2026-06-26); a + // sensitive answer still STARTS Private, so sharing it stays a deliberate choice. `complete: false` (a draft) + // so it never completes a section being filled out, and it persists for EVERY question — answered or not — + // because the renderer sends a scope for each (the core `submitSectionForm` keys off the `sharing` payload). + const saveScopesNow = (nextScopes: Record): void => { + if (meta.adult && !adultAcknowledged) return; // a locked (un-acked) section has no form to save + void autoSaveForm(meta.id, cleanAnswers(answers), nextScopes); + }; + const applyScope = (qid: string, types: RelationshipType[]): void => { + const next = { ...scopes, [qid]: types }; + setScopes(next); + saveScopesNow(next); + }; // The section bulk scope — a common value when every question agrees, else "mixed" (43 §3.2). const questionIds = (meta.questions ?? []).map((q) => q.id); @@ -145,12 +173,12 @@ export function IntakeFormPanel({ const first = scopes[firstId] ?? []; return questionIds.every((qid) => sameScope(scopes[qid] ?? [], first)) ? first : null; })(); - const applyBulk = (types: RelationshipType[]): void => - setScopes((s) => { - const next = { ...s }; - for (const qid of questionIds) next[qid] = [...types]; - return next; - }); + const applyBulk = (types: RelationshipType[]): void => { + const next = { ...scopes }; + for (const qid of questionIds) next[qid] = [...types]; + setScopes(next); + saveScopesNow(next); + }; const sharing: QuestionSharing = { renderControl: (questionId) => ( @@ -194,29 +222,13 @@ export function IntakeFormPanel({ const locked = meta.adult && !adultAcknowledged; const complete = section?.status === 'complete'; - // A matrix answer is a row→point record (Record) — keep it; every other intake answer is a - // scalar/array. Any OTHER non-array object isn't a valid intake answer, so drop it defensively to match the - // bridge contract (IntakeAnswerValueSchema). - const toSubmitFrom = (src: AnswerMap): Record => { - const out: Record = {}; - for (const [qid, value] of Object.entries(src)) { - if (value !== null && typeof value === 'object' && !Array.isArray(value)) { - if (Object.values(value).every((v) => typeof v === 'number')) { - out[qid] = value as IntakeAnswerValue; - } - continue; - } - out[qid] = value as IntakeAnswerValue; - } - return out; - }; - const toSubmit = (): Record => toSubmitFrom(answers); + const toSubmit = (): Record => cleanAnswers(answers); - // Auto-save (2026-06-26): persist every answer + sharing change the moment it's made (debounced), as a DRAFT - // (`autoSaveForm` → `complete: false`) — so picking a scope or answering a question saves right away with no - // Save click, whether the section is being filled for the FIRST time or edited later. A draft never completes - // the section (only the explicit Continue/Done does). The latest payload is mirrored to a ref so the unmount - // flush (Back / switching section within the debounce window) saves the last edit instead of dropping it. + // Auto-save (2026-06-26): a DRAFT save (`autoSaveForm` → `complete: false`) so nothing prematurely completes a + // section being filled out. SHARING changes save immediately (above, `saveScopesNow`); ANSWER typing is + // debounced here (~600ms) so we don't write on every keystroke — whether the section is being filled for the + // FIRST time or edited later. The latest payload is mirrored to a ref so the unmount flush (Back / switching + // section within the debounce window) saves the last edit instead of dropping it. const latestRef = useRef({ answers, scopes }); latestRef.current = { answers, scopes }; const timerRef = useRef | null>(null); @@ -224,7 +236,7 @@ export function IntakeFormPanel({ const runAutoSave = (): void => { timerRef.current = null; dirtyRef.current = false; - void autoSaveForm(meta.id, toSubmitFrom(latestRef.current.answers), latestRef.current.scopes); + void autoSaveForm(meta.id, cleanAnswers(latestRef.current.answers), latestRef.current.scopes); }; /** Drop any pending auto-save — the explicit Continue/Done/Skip owns the write from here (avoids a draft * save racing in AFTER the completing submit and reverting the section to in-progress). */ @@ -246,7 +258,8 @@ export function IntakeFormPanel({ return () => { if (timerRef.current) clearTimeout(timerRef.current); }; - }, [answers, scopes, locked]); + // Only ANSWER edits are debounced here; sharing changes save immediately via `saveScopesNow`. + }, [answers, locked]); // Flush a pending draft on unmount so a quick Back / section switch never loses the last edit. `flushRef` // always points at the latest closure (current `locked` + refs), so the unmount cleanup saves correctly. const flushRef = useRef<() => void>(() => {}); diff --git a/docs/specs/43-relationship-scoped-onboarding-sharing.md b/docs/specs/43-relationship-scoped-onboarding-sharing.md index 8be0969..b1fa605 100644 --- a/docs/specs/43-relationship-scoped-onboarding-sharing.md +++ b/docs/specs/43-relationship-scoped-onboarding-sharing.md @@ -18,6 +18,14 @@ > `markComplete` param (default `true`) for this. **Correction:** the first build scoped auto-save to > already-completed sections only, so first-time onboarding still didn't save on select — the real bug; auto-save > now covers all sections. +> +> **(3) Sharing saves on the CLICK, even before answering (2026-06-26).** Two more root causes the user hit on a +> fresh intimacy section: (a) `submitSectionForm` only wrote `answerSharing` for **answered** questions, so +> clicking "share with Partner" on a section you hadn't filled in yet persisted **nothing** — it now persists a +> scope for every question the renderer explicitly **scopes** (the union of answered + the `sharing` payload), so +> a fresh-section bulk-share sticks and is honored when that question is later answered (an unanswered question +> has no derived fact, so the scope shares nothing until then — safe). (b) A sharing change now saves +> **immediately** (`saveScopesNow`), not on the ~600ms debounce — only answer **typing** stays debounced. > Today, onboarding answers are **own-context-only** (all intake Insight facts hardcoded > `shareable: false`), and the only way to share anything is to finish the intake, run synthesis, then go to diff --git a/packages/core/src/intake/intakeService.test.ts b/packages/core/src/intake/intakeService.test.ts index d5457fb..c29bb1d 100644 --- a/packages/core/src/intake/intakeService.test.ts +++ b/packages/core/src/intake/intakeService.test.ts @@ -198,6 +198,26 @@ describe('intakeService', () => { expect(done.sections.find((s) => s.id === 'basics')?.status).toBe('complete'); }); + it('submitSectionForm persists a sharing scope for an UNANSWERED question (share-before-answer, the screenshot bug)', async () => { + const fs = await setup(); + // Click "share with Partner" on the whole section BEFORE answering anything — the scope must stick (pre-fix + // it keyed off answered questions only, so a fresh-section bulk-share saved nothing). + const session = await submitSectionForm( + fs, + key, + 'p1', + 'basics', + {}, // NO answers yet + NOW, + { occupation: ['partner'], pronouns: ['partner'] }, // explicit scopes for not-yet-answered questions + false, + ); + const basics = session.sections.find((s) => s.id === 'basics'); + expect(basics?.answerSharing?.occupation).toEqual(['partner']); + expect(basics?.answerSharing?.pronouns).toEqual(['partner']); + expect(basics?.answers.occupation).toBeUndefined(); // …and it did NOT invent an answer + }); + it('fills appearance, importantDates (dateList, incomplete rows dropped), and interests (from passions)', async () => { const fs = await setup(); await submitSectionForm( diff --git a/packages/core/src/intake/intakeService.ts b/packages/core/src/intake/intakeService.ts index 01fb86b..3f7f1a7 100644 --- a/packages/core/src/intake/intakeService.ts +++ b/packages/core/src/intake/intakeService.ts @@ -357,12 +357,16 @@ export async function submitSectionForm( } section.answers = { ...section.answers, ...clean }; - // Resolve + persist a sharing scope for EVERY currently-answered question in this section (43 §4): the - // renderer's choice wins, else a prior stored scope, else the category preset. Drop scopes for questions no - // longer answered. Storing the resolved scope explicitly keeps the read side (42 §5.2) honest and makes a - // no-interaction default share per the chip the person saw, never a hidden one. + // Resolve + persist a sharing scope for every question the person has ANSWERED *or* explicitly SCOPED (the + // renderer sends a scope for each question in `sharing`). Persisting an explicit scope for a not-yet-answered + // question is how "share this whole section with Partner" sticks the MOMENT it's clicked — before anything is + // filled in (the reported bug: a fresh-section bulk-share saved nothing because it keyed off answers only). It + // stays safe: an unanswered question has no derived fact, so a scope on it shares nothing until it's answered, + // at which point the person's pre-set choice (e.g. Partner) is honored instead of silently reverting to the + // category default. The renderer's choice wins, else a prior stored scope, else the category preset. + const scopedQids = new Set([...Object.keys(section.answers), ...Object.keys(sharing ?? {})]); const nextSharing: Record = {}; - for (const qid of Object.keys(section.answers)) { + for (const qid of scopedQids) { if (!byId.has(qid)) continue; // only this section's catalog questions const chosen = sharing?.[qid] ?? section.answerSharing?.[qid] ?? defaultScopeForQuestion(sectionId, qid);