Skip to content

fix(plugins): prefer PostgreSQL and use canonical viz identity - #351

Merged
cdeust merged 6 commits into
mainfrom
fix/codex-backend-autoselect
Aug 4, 2026
Merged

fix(plugins): prefer PostgreSQL and use canonical viz identity#351
cdeust merged 6 commits into
mainfrom
fix/codex-backend-autoselect

Conversation

@cdeust

@cdeust cdeust commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Summary

Make the Codex distribution follow Cortex's canonical package identities and
runtime policy: PostgreSQL is attempted first when available, SQLite is the
implicit fallback when no PostgreSQL target is configured and reachable, and
the visualizer publication is hypermnesia-mcp-viz rather than the retired
cortex-viz identity. Claude Code remains the primary/full installation,
Codex is additive, and Gemini is documented third.

This review revision adds the visualization migration machinery:

  • hypermnesia-mcp-viz 3.0.0 points to durable cortex-viz/main commit
    1c1940e278979f35cdecea6146d7fb5f749907e9;
  • cortex-viz 2.8.0 remains only as a frozen, notice-only migration shim with
    no MCP server, commands, skills, or agents;
  • the two complete Claude tool-name migrations are documented; and
  • current companion references use ai-architect-mcp-spec and
    hypermnesia-mcp-viz.

Closes: none (direct maintainer distribution fix).

Type of change

  • Bug fix
  • New feature
  • Breaking change (visualization publication/tool identity migration)
  • Refactor
  • Documentation only
  • Audit-finding closure

Test plan

  • Script suite: 589 passed, 121 subtests passed, 5 PostgreSQL-gated skips.
  • Codex/Claude/viz contract tests: 8 passed on the final head.
  • Marketplace pin tests: 19 passed.
  • PostgreSQL-first host smoke covers both claude-code and codex-cli.
  • Ruff lint/format, JSON validation, documentation claims, and diff checks pass.
  • The reviewed external viz head was verified for name and version.
  • The pin is the squash result reachable from cortex-viz/main.

Deployment order — completed

cdeust/cortex-viz#107 was squash-merged first. Its resulting main commit is
the durable source pin used by this PR.

  1. Completed: squash-merge cortex-viz#107 as
    1c1940e278979f35cdecea6146d7fb5f749907e9.
  2. Completed: pin the marketplace, contract-test expectation, and changelog to
    that main commit.
  3. Completed: every required check passed on the durable-pin head; fix(plugins): prefer PostgreSQL and use canonical viz identity #351 is
    ready to merge.

The notice-only shim remains part of this PR. The canonical source pin no
longer depends on a PR-head commit.

Breaking changes

Memory storage and PostgreSQL configuration are unchanged. Existing
visualizer installs must run:

claude plugin uninstall cortex-viz@cortex-plugins
claude plugin marketplace update cortex-plugins
claude plugin install hypermnesia-mcp-viz@cortex-plugins

Claude allowlists, hooks, skills, and agents must also replace the full
mcp__plugin_cortex-viz_cortex-viz__* names with
mcp__plugin_hypermnesia-mcp-viz_hypermnesia-mcp-viz__*.

Reviewer checklist

  • Documentation updated.
  • No secrets / credentials / PII in the diff.
  • CI passes on the latest commit.

@cdeust cdeust left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review (via Claude Code cross-harness pass). LGTM on content: Codex packaging is purely additive, zero MCP server code touched, CORTEX_RUNTIME is a pre-existing capability flag (not harness branching), timing constants properly sourced, and the dedicated test proves the primary Claude plugin manifest is unchanged.

One merge constraint: the marketplace.json entry rename to hypermnesia-mcp-viz is NOT cosmetic — it must land atomically with cdeust/cortex-viz#107 (which renames the plugin itself). Neither PR alone: each breaks the other's install path during the gap. Coordinate the two merges and the migration note for installs pinned to cortex-viz@cortex-plugins.

Minor, no urgency: "cowork" as a CORTEX_RUNTIME value now serves Codex too — consider a harness-neutral name (local/db-optional) in a future pass.

@cdeust cdeust left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second cross-harness pass (Claude Code, Opus 5), independent re-verification of the earlier maintainer review. Head reviewed: 46835162. Verdict: REQUEST CHANGES — one blocking class, and it is not the storage change.

Agreed with the previous review (re-verified, not taken on testimony)

  • "Purely additive, zero MCP server code touched" — confirmed against the file list and by diffing mcp_server/infrastructure/memory_store.py (selection path, lines 149–242) against main: byte-identical.
  • "Primary Claude plugin manifest unchanged" — confirmed by reading .claude-plugin/plugin.json directly, not only the test that asserts it: still CORTEX_RUNTIME: "", DATABASE_URL: "${user_config.database_url}", python3 launcher.py mcp_server, all 6 hook groups.
  • "CORTEX_RUNTIME is a pre-existing capability flag, not harness branching" — confirmed at mcp_server/infrastructure/memory_config.py:20-27 and scripts/docker_smoke.sh:61.
  • "Timing constants properly sourced"tests_py/scripts/test_codex_plugin_contract.py:78-80 and docs/codex-plugin.md both name date, OS, arch, uv version.
  • The viz rename is the merge constraint, and it must land atomically with cdeust/cortex-viz#107. Confirmed — and escalated below.

Extended (constraint was right; here is the mechanism and the exact failure)

  • Pointer: .claude-plugin/marketplace.json:61-65 (name: hypermnesia-mcp-viz, source.repo: cdeust/cortex-viz, version: 2.8.0).
  • The installed cortex-plugins pin carries no ref/tag/commit — it tracks the default branch. So the gap window is not "between merge and the deferred marketplace update"; merge itself is the deploy, in both orderings:
    • #351 first: the marketplace advertises hypermnesia-mcp-viz at a repo whose .claude-plugin/plugin.json still says cortex-viz (verified locally). Fresh claude plugin install hypermnesia-mcp-viz hits an identity mismatch; existing cortex-viz@cortex-plugins installs are orphaned immediately.
    • cortex-viz#107 first: that PR renames both plugin.json name and the mcpServers key, so every viz tool moves from mcp__plugin_cortex-viz_cortex-viz__* to mcp__plugin_hypermnesia-mcp-viz_hypermnesia-mcp-viz__* while this marketplace still advertises cortex-viz.
  • CI cannot catch this: I ran claude plugin validate . on the PR worktree — it passes (4 pre-existing warnings) and never fetches the github source, so the marketplace-name ≠ remote-plugin.json-name divergence is undetectable by the gate. scripts/check_marketplace_pins.py compares by repo, not by name, so it will not catch it either.

Added — blocking, not raised before

B1 — .claude-plugin/marketplace.json:61: plugin rename with no deprecation alias, contradicting this repo's own established migration pattern.
Claude Code keys installs as <plugin>@<marketplace> (installed_plugins.json, e.g. hypermnesia-mcp@cortex-plugins). After merge, cortex-viz@cortex-plugins no longer exists in the listing: no update path, no uninstall target, and the replacement shows as not-installed. This repo already solved this exact problem oncemarketplace.json keeps a frozen cortex entry pointing at plugins/cortex-deprecated (hooks.json emits a migration notice), pinned by scripts/check_marketplace_pins.py:55 FROZEN_PINS, plus the user-facing note at README.md:64. Viz gets none of it. A migration note alone is weaker than the precedent this repo set for itself.

B2 — tests_py/scripts/test_codex_plugin_contract.py:105 (assert "cortex-viz" not in names) encodes the hard break as a contract, and forbids B1's remedy.
The fix cannot be applied without also changing this test. Replace it with an assertion that the cortex-viz entry, if present, is the deprecation shim (source == "./plugins/cortex-viz-deprecated", version frozen) — i.e. mirror the treatment cortex already gets.

B3 — atomicity is real and unenforced. The PR body's "release/tag/marketplace publication happens after merge" defers exactly this, with no tracking issue number. Note that cdeust/cortex-viz#107 has since added the reciprocal statement to its own CHANGELOG ("must not be published independently"); this PR still carries no such line. The durable fix is a ref pin on the cortex-plugins viz entry, so a default-branch merge stops being a live deploy.

Refuted / qualified

  • "LGTM on content" understates one point in the PR's favour. Pre-PR, the Codex manifest ran with CORTEX_RUNTIME unset → cli (memory_config.py:27) → autopostgresql (memory_store.py:168) → default URL truthy → allow_fallback = not url False → hard RuntimeError on any machine without PostgreSQL. The old CI gate never caught it because verify_mcp_hosts forced CORTEX_MEMORY_STORE_BACKEND=sqlite. Removing that pin (verify_mcp_hosts.py:100-107) is this PR's real value — not merely "preferring PostgreSQL".
  • The PR body's "new contract tests cover … SQLite fallback" is overstated: tests_py/infrastructure/test_sqlite_backend.py:543-573 monkeypatches _try_pg_verbose and characterizes untouched production code — it would not fail if this PR's actual change (the manifest env block) were reverted. The manifest change is genuinely covered by test_codex_plugin_contract.py:65-82, which does fail on revert.

Non-blocking

  • The new PG-first gate (.github/workflows/ci.yml:213-223) is genuinely strong: job-level DATABASE_URL (ci.yml:27) + real pgvector (ci.yml:44-87) → _database_url_is_explicit True → memory_store.py:218-227 refuses fallback. A green memory_stats does prove PostgreSQL. Accurate comment.
  • Same gate runs --clients codex-cli only. The client name only sets clientInfo in the frames, so --clients codex-cli,claude-code extends PG-first proof to the primary host for one word of diff. Recommended.
  • ci.yml:465-470 (cold-start) also gained --storage-selection auto, but that job has no PostgreSQL and no DATABASE_URL → the non-explicit branch (memory_store.py:234-242) silently lands on SQLite. It proves startup timing, not backend selection — do not read it as a second PG proof.
  • docs/codex-plugin.md:55-57"Claude Code … manifest, hooks, agents, and full tool profile are unchanged" is true of plugin.json, but .claude-plugin/marketplace.json is a Claude Code artifact and it did change. Reword.
  • The same latent hard-fail described above still exists for Claude Code whenever ~/.claude/methodology/backend.json is absent (backend_marker.py precedence 5 → engine default auto; launcher.py:128-140) — e.g. a failed postInstall. Worth a follow-up issue; not caused or worsened here.
  • After both merges, viz is installable from two marketplaces (cortex-plugins and the already-registered hypermnesia-mcp-viz-marketplace) — two installs of the same plugin name, two identical MCP servers. Pick one publication path.
  • "cowork" as a CORTEX_RUNTIME value: agreed with the earlier review, cosmetic, defer.
  • Sizes/lint clean; layers untouched; no unsourced constant.

Claude Desktop

OK by non-interference, unproven by any gate here. manifest.json (MCPB 0.4, hypermnesia-mcp 4.17.2) is not in the diff and Desktop already carries both extras, so this PR cannot regress it. But scripts/verify_mcp_hosts.py:28 still has CLIENTS = ("claude-code", "gemini-cli", "codex-cli") with no claude-desktop, and no .mcpb pack step outside release.yml:386-426. The PR makes no Desktop claim, so this is a gap, not a false claim.

Could not verify

  • Whether Claude Code hard-errors or silently skips a github-sourced marketplace entry whose name diverges from the remote plugin.json name. The divergence is proven; its user-visible shape is not (determining it requires a real install).
  • PR-body figures ("62 passed", "17,744 memories with pgvector", "controlled implicit PostgreSQL failure selected SQLite") — local-run testimony, not reproducible from the diff.

CI at review time: 20 SUCCESS / 1 SKIPPED.

@cdeust
cdeust marked this pull request as draft August 3, 2026 23:06
@cdeust
cdeust marked this pull request as ready for review August 3, 2026 23:18
@cdeust
cdeust merged commit b29dfce into main Aug 4, 2026
21 checks passed
cdeust added a commit that referenced this pull request Aug 4, 2026
…e default branch (#354)

The gate compared versions only, so a pin that named an unmerged PR head
read as current on every run. Cortex #351 pinned cortex-viz twice this way
(ee0d41db, then 7e297ebc); both were `ahead` of main, and the squash-merge
of #107 did orphan 7e297ebc — it now reports `diverged`. The marketplace
would have kept serving a commit no branch reaches, with a green gate.

New check PIN_SHA_UNREACHABLE: for any github source carrying a `sha`,
require compare/{default_branch}...{sha} in {identical, behind}. `ahead`
and `diverged` mean the pin carries commits the branch does not.

Source for the status vocabulary: GitHub REST "Compare two commits" —
status is exactly one of ahead / behind / identical / diverged.

Degradation contract unchanged: an unresolvable repo or a network failure
emits a NOTICE and exits 0, matching every other network leg of this gate.

Proof: suite 26/26; mutation (REACHABLE_FROM_DEFAULT widened to accept
ahead/diverged) fails exactly the 2 new tests; end-to-end against the live
API — 7e297ebc FAIL `diverged`, 1c1940e2 PASS, absent sha FAIL; full gate
on the real marketplace.json at b29dfce exits 0 with no false positive.


Claude-Session: https://claude.ai/code/session_01JMNwLEZS6SsrWMLwd1cQ6p

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant