chore: move learning-subsystem machine-state to config-store JSON (#1438)

[josephfung]

Summary

Moves the email-observation learning subsystem's queue/status/guard machine-state out of OKF markdown doc bodies into config-store JSON under the existing ceo_inbox namespace, deleting the regex-parse / block-rewrite machinery that bit us three times during the #1429 review (the t1/t10 block-boundary bleed, trimming on any ## header, the first-block-only proposal scan). Prose evidence stays in OKF where it belongs.

Closes #1438. Part of #1419 (Tier 4 follow-up from the #1429 simplification review).

What moved to config-store JSON (ceo_inbox, one whole-rewritten key per store)

Key	Shape	Writer(s)	Reader(s)
sent_observe.completion_candidates	map keyed by taskId	sent-observe adds; task-completion removes on consume	task-completion
sent_observe.completion_digest	map keyed by taskId	task-completion adds; resolve removes	list / resolve
voice_learn.proposal	single object or null	voice-learn (supersede)	list / resolve
sent_observe.matched_draft_ids	string[]	sent-observe	sent-observe
sent_observe.asked_task_ids	string[]	sent-observe	sent-observe

Removal = writing the map without the entry (no per-item tombstones, no status parsing). Guard sets are pruned on write to live entities already loaded each run (asked_task_ids → currently-open tasks; matched_draft_ids → present draft snapshots), so they stay bounded without a new query.

What stayed OKF (unchanged format)

pending-diffs.md (the (draft, sent) prose diffs), per-draft snapshots, shadow docs — the workspace is the scratchpad; only the machine-state moved.

Deleted

parseCompletionCandidates, parseCompletionDigest, parseVoiceGuideProposal, pruneGuideProposals, removeCompletionBlock, guideProposalBlocks, guideFromBlock, extractMatchedDraftIds, extractAskedTaskIds, markCandidateProcessed, candidateBlock, formatUndoNote, formatConfirmNote, formatCompletionCandidateBlock, and the version-conflict retry loops protecting those doc rewrites. The digest render helpers stay and read JSON; the risk-tiering logic stays.

Key design decisions

• Why asked_task_ids is a separate guard: once consumed candidates are removed from the queue map, the map can't double as the "already-asked" guard, so it becomes its own key.
• Watermark-hold atomicity (acceptance criterion "no candidate lost"): end-of-run writes are ordered diffs → completion_candidates → matched_draft_ids (gated on diffs persisting, excludes drafts whose body-fetch failed) → asked_task_ids (gated on the candidate write persisting). Maps are keyed by taskId so a held-watermark retry re-adds idempotently.
• Config-store soft-reject hardening: ConfigStore.set returns { stored } (additive — existing void-callers unaffected) so a soft-reject (storeFact stored:false, which does not throw) is surfaced instead of silently treated as durable. sent-observe holds the watermark on a candidate soft-reject; task-completion consumes only when the digest persisted; voice-learn advances its checkpoint only when the proposal persisted; parseJson now logs a corrupt stored value instead of silently resetting the whole map. (Found by the pre-PR silent-failure review — it was a real regression vs the old OKF path's reliable ok:false signal.)
• Concurrency: whole-object last-write-wins replaces per-doc optimistic-version checks — acceptable because each key has effectively a single writer per cron tick and the queue key is keyed by taskId. Documented in ADR-029.

Docs

ADR-029 store-mapping table split (cadence vs queue/status/guard-state, last-write-wins trade-off noted); specs 04/13/19 updated to describe the config keys instead of the old doc markers.

Testing

• Full skills suite green: 1795 tests, 143 files; typecheck clean.
• New behavioral coverage for the load-bearing invariants: candidate survives a completeTask failure (retry); asked-guard not written when the candidate write is held; failed-fetch drafts excluded from matched_draft_ids; guard-set pruning to live entities; supersede of a pending voice proposal; digest UX byte-identical (verbatim note composers + unchanged render helpers); and the four soft-reject regression tests (held watermark / retained candidate / un-advanced checkpoint / logged corruption).

Sequencing

The learning subsystem ships disabled in registry-defaults.yaml; deploy this before enabling it in prod, so there is no live state to migrate (any dev-instance scratch docs TTL out). No migration code.

Follow-ups (out of scope; noted for later)

• No age/TTL backstop on the config maps (the old pending-completions.md trimmed at 90 days) — a permanently-failing completeTask retries indefinitely and retains PII-adjacent metadata. Consider age-stamping + pruning or a retry cap.
• ConfigStore.set's soft-reject is now surfaced here, but the underlying primitive still soft-rejects across all its consumers (watermark, idle-backoff) — a broader hardening of ConfigStore itself could throw or return stored everywhere.
• workingDocs is now a vestigial (declared-but-unused) capability in list/task-completion/resolve.
• trimEvidenceDoc's ## Candidate — branch is now dead (its only producer was deleted).
• The matched-guard and asked-guard writes are structurally identical — a prunedIdSetWrite helper could DRY them.
• completion+digest is not fully atomic (a digest soft-fail after auto-complete loses the undo note — now logged, not silent; a full fix needs completing the task only after the digest is durable).

[josephfung]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

The learning subsystem now stores completion candidates, completion digests, voice proposals, and idempotency guards as JSON in ConfigStore under ceo_inbox. Sent-mail observation retains prose evidence in OKF while persisting machine state separately and holding watermarks when writes fail or are soft-rejected. Voice and completion handlers, renderers, tests, ADRs, specifications, changelog entries, and skill versions were updated accordingly. ConfigStore.set now returns { stored }.

Possibly related PRs

• josephfung/curia#890 — Introduces the ConfigStore implementation and return behaviour used by this migration.

Suggested labels: enhancement, orchestration, size:XXL

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarises the main change: moving learning-subsystem machine state into config-store JSON.
Description check	✅ Passed	The description is directly about the config-store JSON migration and matches the changeset.
Linked Issues check	✅ Passed	The changes satisfy #1438 by moving queue, status, and guard state to config-store while preserving OKF prose evidence and updating docs/tests.
Out of Scope Changes check	✅ Passed	I see no clearly unrelated code changes; the doc updates, version bumps, and config-store API tweak all support the migration.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (1)

skills/list-learning-digest/handler.ts (1)

8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale workingDocs guard — handler no longer uses workingDocs.

The !ctx.workingDocs check at line 8 is now dead logic. After the migration to ConfigStore, the handler body exclusively reads via entityMemory and never touches workingDocs. If workingDocs were absent but entityMemory present, the handler would incorrectly return a failure despite having everything it needs. Consider replacing this guard with !ctx.entityMemory (or removing it, since the execution layer guarantees declared capabilities) and dropping workingDocs from the skill manifest capabilities.

♻️ Suggested guard update

   async execute(ctx: SkillContext): Promise<SkillResult> {
-    if (!ctx.workingDocs) {
-      return { success: false, error: 'list-learning-digest requires workingDocs' };
+    if (!ctx.entityMemory) {
+      return { success: false, error: 'list-learning-digest requires entityMemory' };
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/list-learning-digest/handler.ts` at line 8, Replace the obsolete
!ctx.workingDocs guard in the handler with a check for !ctx.entityMemory, or
remove the guard if capability guarantees make it unnecessary; ensure the
handler no longer depends on workingDocs and remove that capability from the
skill manifest.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@skills/_shared/learning-state.ts`:
- Around line 56-59: Update parseJson to validate parsed values against the
expected persisted shape before casting and returning them as T. Add per-store
validators for each learning-state store, reject arrays and malformed candidate
entries, invoke onCorrupt for validation failures, and return null so callers
fall back safely.
- Around line 70-75: Update corruptionLogger so it never logs raw learning-state
content or rawSnippet. Replace the payload snippet with non-sensitive metadata:
retain key, include the raw value’s byte length, and optionally add a hash using
an existing project utility; preserve the warning message and empty-state
behavior.

In `@skills/ceo-inbox-sent-observe/handler.ts`:
- Around line 566-582: Update the watermark advancement logic around the
matched-draft and asked-task guard writes so both writeIdSet results are
tracked, treating thrown errors and stored:false as failures. Include both guard
outcomes in advanceOk, and hold the watermark whenever either matched_draft_ids
or asked_task_ids persistence is rejected, while preserving retry behavior.

In `@skills/list-learning-digest/skill.json`:
- Line 4: Update the version fields to reflect the new entityMemory capability:
change skills/list-learning-digest/skill.json lines 4-4 from 0.1.1 to 0.2.0, and
make the same change in skills/task-completion-from-sent/skill.json lines 3-4.
Keep the capability additions unchanged.

In `@skills/resolve-learning-digest/handler.ts`:
- Around line 69-71: Check the result of every ConfigStore write in the handler,
including writeVoiceProposal and the other changed write paths, and handle
stored: false instead of reporting successful resolution while state remains
actionable. Add idempotent recovery for each declined write, especially
restoring or otherwise reconciling an undone task after its task mutation
already succeeded. Extend handler tests to cover { stored: false } for
proposals, digest items, completions, and undo flows.

In `@skills/task-completion-from-sent/handler.ts`:
- Around line 136-202: The auto-completion flow in
skills/task-completion-from-sent/handler.ts (lines 136-202) must preserve a
durable digest-pending state when completeTask succeeds but
writeCompletionDigest soft-rejects; retain the candidate or otherwise retry the
undo digest for the already-completed task instead of removing it as ineligible
on the next run, or reopen the task as compensation. Update the handler test in
skills/task-completion-from-sent/handler.test.ts (lines 302-342) to run the
handler twice and verify the rejected undo remains recoverable on retry.

---

Nitpick comments:
In `@skills/list-learning-digest/handler.ts`:
- Line 8: Replace the obsolete !ctx.workingDocs guard in the handler with a
check for !ctx.entityMemory, or remove the guard if capability guarantees make
it unnecessary; ensure the handler no longer depends on workingDocs and remove
that capability from the skill manifest.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f0158916-040f-490a-afbf-7e6d6906c0ff

📥 Commits

Reviewing files that changed from the base of the PR and between 113660d and b2c3bbe.

📒 Files selected for processing (31)

• CHANGELOG.md
• docs/adr/029-passive-email-observation-and-counterfactual-competence.md
• docs/specs/04-channels.md
• docs/specs/13-office-identity.md
• docs/specs/19-tasks-and-backlog.md
• docs/wip/2026-07-17-learning-state-config-store-design.md
• docs/wip/2026-07-17-learning-state-config-store.md
• skills/_shared/learning-digest.test.ts
• skills/_shared/learning-digest.ts
• skills/_shared/learning-state.test.ts
• skills/_shared/learning-state.ts
• skills/_shared/sent-observe-match.test.ts
• skills/_shared/sent-observe-match.ts
• skills/_shared/task-completion-risk.test.ts
• skills/_shared/task-completion-risk.ts
• skills/ceo-inbox-sent-observe/handler.test.ts
• skills/ceo-inbox-sent-observe/handler.ts
• skills/ceo-inbox-sent-observe/skill.json
• skills/list-learning-digest/handler.test.ts
• skills/list-learning-digest/handler.ts
• skills/list-learning-digest/skill.json
• skills/resolve-learning-digest/handler.test.ts
• skills/resolve-learning-digest/handler.ts
• skills/resolve-learning-digest/skill.json
• skills/task-completion-from-sent/handler.test.ts
• skills/task-completion-from-sent/handler.ts
• skills/task-completion-from-sent/skill.json
• skills/voice-learn/handler.test.ts
• skills/voice-learn/handler.ts
• skills/voice-learn/skill.json
• src/memory/config-store.ts

💤 Files with no reviewable changes (3)

• skills/_shared/sent-observe-match.test.ts
• skills/_shared/task-completion-risk.ts
• skills/_shared/task-completion-risk.test.ts