mcpx: owner-only elicitation broker store (0700/0600) with migration on open (#1676) by dezren39 · Pull Request #1724 · developing-today/code

dezren39 · 2026-10-28T16:18:45Z

Implements #1676 — Phase 1 of #1675, per triage task 1884 ruling. Landing this also clears the Phase 1 gate for #1677 (task 1885) and half the #1678 gate (task 1886).

Change

New elicit.StorePath(stateDir) — the single helper resolving the broker store: <state>/elicit/elicit.db. Both open sites use it via the new shared elicit.OpenAt(stateDir, legacyPath):
- daemon OpenBroker (legacy source: <state>/logs/elicit.db)
- CLI broker() (legacy source: <effective logging.dir>/elicit.db — logging.dir honoured for the migration source only, destination is always <state>/elicit/)
- the conformance officialclient harness is swapped too, so it keeps answering questions from the store the daemon actually uses.
Migration on open: when the new store is absent and a legacy store exists, its files (db + WAL/SHM) are renamed into place, with a copy-then-delete fallback; pending questions are preserved. If migration fails outright, the legacy store is opened where it lies, tightened in place — never stranded.
Tightening on open: store dir held at defaults.DirMode (0700), DB/WAL/SHM at 0600, chmod-if-looser, idempotent under concurrent daemon+CLI open; a tightening failure is a returned error. The shared logs directory mode is unchanged.
Conformance: gap elicitation-state-storage-protected retired — registered as covered by the new tests; docs/spec/conformance-matrix.md regenerated (diff is exactly that requirement).

Tests (laptop)

go test ./internal/elicit/ — pass, including 4 new tests: fresh-open modes by stat (0700 dir / 0600 file), legacy migration with a pending question preserved and answerable, pre-loosened modes re-tightened on open, concurrent opens.
go test ./internal/daemon/ ./internal/cli/ — pass. go test ./internal/conformance/ -run TestMatrix — pass after regen. go build ./... — pass.

Queue task 1893 · branch queue/1893-1676-elicit-store

^{Need help on this PR? Tag @codesmith-bot with what you need.}

coderabbitai · 2026-10-28T16:18:54Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 9f8cd98d-78a7-49e3-8018-848e00d9f65f

Commits

Reviewing files that changed from the base of the PR and between 6a284ef and 41fc1a4.

Files selected for processing: 8

pkgs/mcpx/docs/spec/conformance-matrix.md
pkgs/mcpx/internal/cli/elicit.go
pkgs/mcpx/internal/conformance/coverage_clientfeatures.go
pkgs/mcpx/internal/conformance/officialclient/main.go
pkgs/mcpx/internal/daemon/hooks.go
pkgs/mcpx/internal/e2e/e2e_test.go
pkgs/mcpx/internal/elicit/store.go
pkgs/mcpx/internal/elicit/store_test.go

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

chatgpt-codex-connector · 2026-10-28T16:18:56Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-28T22:48:19.933256Z	`41fc1a4`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ace892baa0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-28T16:23:50Z

+	for _, suffix := range []string{"", "-wal", "-shm"} {
+		src, dst := legacyPath+suffix, dest+suffix


Serialize migration before exposing the destination database

When a legacy store has uncheckpointed WAL data (for example, after a daemon crash), concurrent daemon and CLI opens can permanently lose pending questions. This loop publishes elicit.db before moving its -wal and -shm files; another OpenAt then sees the destination, skips migration, and calls Open(dest), which initializes the database without the legacy WAL. Moving the sidecars afterward does not repair that connection and can result in their contents being discarded on close. I reproduced this interleaving with SQLite: a committed pending row disappeared and remained missing after reopening. Protect migration and destination opening with a shared cross-process lock, and only expose a complete, consistent migrated store.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-28T16:23:50Z

-		return nil, err
-	}
-	return elicit.Open(filepath.Join(dir, "elicit.db"))
+	return elicit.OpenAt(a.Paths.State, filepath.Join(dir, "elicit.db"))


Update the e2e seeder to use the relocated store

TestElicitationsCanBeListedAndAnsweredFromTheCommandLine and TestACredentialQuestionIsRoutedToAHuman initialize the store with elicit list, then call seedElicitation in internal/e2e/e2e_test.go:2443–2457. That helper still opens state/logs/elicit.db, whereas this call now initializes only state/elicit/elicit.db. Consequently, both tests fail during seeding because the legacy directory or its elicitations table does not exist, before reaching their assertions. Update the helper to resolve the database through elicit.StorePath; the full test suite run by .github/workflows/mcpx.yml includes these tests.

Useful? React with 👍 / 👎.

…t-store # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md

…ed e2e via StorePath (#1676) Codex review on #1724: (P1) concurrent daemon+CLI opens could see the destination DB before its WAL sidecar moved and lose questions that live only in the legacy WAL - OpenAt now holds an flock on the store directory across migration and first open, with a regression test racing concurrent opens against an uncheckpointed-WAL legacy store. (P2) the e2e elicitation seeder still opened state/logs/elicit.db, failing both elicitation e2e tests in CI - it now resolves the store through elicit.StorePath.

dezren39 · 2026-10-28T18:06:09Z

🤖 Agent note · fixes pushed (queue task 1893, lane B resume)

Addressing both Codex findings + the merge conflict, in badb3628d (on top of merge a5c2c1b74 with current main):

P1 — migration race: elicit.OpenAt now takes a cross-process flock on the store directory (elicit.db.lock, 0600 inside the 0700 dir) and holds it across migration and the first open of the destination, so no opener can see elicit.db before its WAL/SHM sidecars arrive. New regression test TestOpenAtConcurrentMigrationPreservesUncheckpointedWAL snapshots a live legacy store's WAL, races 8 concurrent first opens against it, and verifies the WAL-only question survives — passes (also ×5 repeat).
P2 — e2e seeder: seedElicitation now resolves the store through elicit.StorePath instead of the legacy state/logs/elicit.db. Both named e2e tests pass locally on the laptop.
Conflict: only docs/spec/conformance-matrix.md (generated-inputs hash); resolved by regenerating the matrix from the merged tree — the requirement still shows as tested by the new elicit tests.

Local (laptop) verification: go test ./internal/elicit/ all pass, go vet clean, both elicitation e2e tests pass. CI is the arbiter for the full suite.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: badb3628d2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-28T18:11:48Z

+			if err := migrateStore(legacyPath, dest); err != nil {
+				if _, derr := os.Stat(dest); derr != nil {


Preserve the complete store when a migration step fails

If migrating the main database succeeds but copying its WAL fails—for example, because a cross-filesystem destination runs out of space—dest already exists, so this branch suppresses the migration error and proceeds to open an incomplete store. Committed questions present only in the WAL can then disappear, and subsequent opens skip migration because the destination exists. Reproducing an interrupted WAL copy with SQLite produced a destination that opened successfully but contained no pending rows, including after reopening. Preserve a recoverable complete source and only open the destination after every required migration step succeeds; destination existence alone does not establish success.

Useful? React with 👍 / 👎.

…t-store # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md

mcpx: owner-only elicitation broker store with migration on open (#1676)

ace892b

dezren39 requested a review from a team as a code owner October 28, 2026 16:18

dezren39 mentioned this pull request Oct 28, 2026

mcpx: Phase 1 of #1675 — owner-only elicitation broker store (0700 dir, 0600 files, migration on open) #1676

Closed

chatgpt-codex-connector Bot reviewed Oct 28, 2026

View reviewed changes

dezren39 mentioned this pull request Oct 28, 2026

mcpx: Phase 2 of #1675 — broker retention bound + summary-only elicitation events #1677

Closed

github-actions Bot assigned dezren39 Oct 28, 2026

dezren39 mentioned this pull request Oct 28, 2026

mcpx: Phase 3 of #1675 — review-first mcpx elicit surface + nix#255 truth-up #1678

Closed

dezren39 added 2 commits October 28, 2026 13:05

Merge remote-tracking branch 'origin/main' into queue/1893-1676-elici…

a5c2c1b

…t-store # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md

chatgpt-codex-connector Bot reviewed Oct 28, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into queue/1893-1676-elici…

41fc1a4

…t-store # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md

dezren39 merged commit 2953f27 into main Oct 29, 2026
17 checks passed

dezren39 deleted the queue/1893-1676-elicit-store branch October 29, 2026 00:37

dezren39 mentioned this pull request Oct 31, 2026

mcpx: elicitation storage is readable by any local user and never pruned; elicitation UI and response validation rows #1954

Closed

4 tasks

dezren39 restored the queue/1893-1676-elicit-store branch November 1, 2026 07:59

dezren39 deleted the queue/1893-1676-elicit-store branch November 1, 2026 16:45

This was referenced Nov 3, 2026

Preserved branch: queue/1893-1676-elicit-store — decision needed (merge / cherry-pick / drop) #3636

Closed

design(mcpx): elicitation broker hygiene — owner-only store, bounded retention, redacted events, review-first answering #1675

Closed

Navigation Menu

Repository navigation

mcpx: owner-only elicitation broker store (0700/0600) with migration on open (#1676) - #1724

mcpx: owner-only elicitation broker store (0700/0600) with migration on open (#1676)#1724
dezren39 merged 4 commits into
mainfrom
queue/1893-1676-elicit-store

dezren39 commented Oct 28, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Uh oh!

dezren39 commented Oct 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		for _, suffix := range []string{"", "-wal", "-shm"} {
		src, dst := legacyPath+suffix, dest+suffix

		if err := migrateStore(legacyPath, dest); err != nil {
		if _, derr := os.Stat(dest); derr != nil {

Conversation

dezren39 commented Oct 28, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

Tests (laptop)

Uh oh!

coderabbitai Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

dezren39 commented Oct 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dezren39 commented Oct 28, 2026 •

edited by blacksmith-sh Bot

Loading

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 28, 2026 •

edited

Loading