fix(mcp): close out the #952 manual verification findings#981
Merged
Conversation
PR #971 review (codex): man install only copies packaged groff files, but as a top-level subcommand it ran ensure_initialization first — a locked or broken local database would block installing the offline docs. Add man to skip_init_commands with a regression test that fails if initialization ever runs for it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
The session-local index was built once and never refreshed, so projects created out-of-band (CLI, a teammate in a shared workspace) stayed invisible and unroutable by project_id until session restart — and the error asserted nonexistence of a project that demonstrably existed. A miss is exactly the signal the cache may be stale: tag absent-project errors as WorkspaceProjectLookupMiss (distinct from ambiguity, which a refresh cannot fix), rebuild the index once, and retry. A second miss is authoritative. Hits never trigger rebuilds. This also un-breaks read_note's fuzzy fallback, which re-resolves the active project by external_id through this index. Fixes #956 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
get_client(workspace=...) silently dropped the selector on the default fallback path: a create_memory_project targeting a team workspace from a local MCP server (OAuth-only, no flags, no project route) was served by the local ASGI app — failing on cloud-style paths like '/manual' or, worse, silently creating a local project where the user asked for a shared cloud one. A workspace selector now implies cloud routing: route to the cloud proxy when credentials exist, raise with the auth hint when they don't. Explicit --local and per-project routing still take precedence (priority order is documented in the docstring). _resolve_workspace_routing now resolves slugs/names to tenant ids whenever credentials allow discovery — not only under factory mode or explicit --cloud — so X-Workspace-ID always carries the tenant id. Fixes #954 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
build_context printed unresolved forward references as [[None]]: the
relation's literal target text (Relation.to_name) was embedded only in the
composite row title ('see_also: edit-note(3)'), to_id is NULL until the
target exists, and the formatter rendered the resolved-entity title — None.
Select to_name as a real column in both context CTEs (SQLite and
Postgres), thread it through ContextResultRow, SearchIndexRow, and
RelationSummary (additive, optional — JSON consumers gain the field too),
and fall back to it in the text formatter. Forward references are a
first-class part of the format; they now render as written.
Regression tests at the service layer (both backends) and the formatter.
Fixes #955
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
folder/* patterns returned nothing on cloud workspace projects: memory URL canonicalization qualified them as <workspace>/<project>/<pattern>, but the search index stores project-qualified permalinks without the workspace prefix, so the pattern-match path (which has no link-resolver fallback, unlike direct lookups) could never match a row. Patterns now canonicalize to the index form — project prefix only — in both the active-route and workspace-qualified-URL paths. Direct URLs keep full workspace qualification, which the link resolver understands. Fixes #957 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
In CLI-only sessions no sync coordinator runs (it lives in the server lifespan), so pending changes never drain and --wait always times out — a dead end that looks like a hung indexer. The timeout error now says so and hands over the command that actually indexes: 'bm reindex --project <name>'. Closes the code half of #959 (the stale 'basic-memory sync' doc references were fixed in #971). Fixes #959 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
test_create_project_accepts_workspace_in_local_mode asserted the old behavior — workspace as a local no-op — which is precisely what #954 classifies as the bug (silent local create when a cloud team workspace was requested). The test now pins the new contract: the schema still accepts the parameter over the MCP wire, the create fails fast with auth guidance when no credentials exist, and no local project is left behind. Tool docstrings updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
CI caught an interaction the first #957 cut missed: when the server-side workspace permalink contextvar is active (hosted MCP / factory mode), stored permalinks ARE workspace-qualified, so workspace-qualified patterns are correct there. The stored form follows the contextvar at write time; the client's cached_workspace is display state and must not qualify patterns. Patterns now drop the workspace prefix only when no contextvar is active — exactly the local-stdio-to-cloud case from the field repro. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efe7a08667
ℹ️ 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".
…gacy rows PR #981 review (codex): cloud routes run inside the workspace permalink contextvar, so workspace-qualified patterns are correct for rows written under that context — but rows written before workspace canonicalization (or via clients that didn't forward the headers) store project-qualified permalinks, and a qualified pattern can never match them. The client cannot know which form the server stores. Server-side answer: when a wildcard search matches nothing and a workspace permalink context is active, retry once with the workspace prefix stripped. New-consistent data matches on the first attempt; legacy data matches on the fallback. Regression test writes legacy-form rows and queries under an active workspace context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
This was referenced Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all five remaining bugs found during the #952 manual verification run (the sixth, #958, shipped in #971). One commit per bug, each with a regression test that mirrors the field failure.
WorkspaceProjectLookupMiss(subclass distinguishing miss from ambiguity) → rebuild once, retry; second miss is authoritative. Also un-breaksread_note's fuzzy fallback.get_client: selector + credentials → cloud proxy; selector without credentials →RuntimeErrorwith the auth hint._resolve_workspace_routingresolves slugs→tenant ids whenever credentials allow, soX-Workspace-IDalways carries the tenant id.[[None]]to_nameselected in both context CTEs (SQLite + Postgres), threaded throughContextResultRow/SearchIndexRow/RelationSummary(additive), formatter falls backto_entity or to_name. Postgres path verified via testcontainers.folder/*patterns work on cloud workspace projectsstatus --waittimeout points atbm reindexAlso included:
bm manadded toskip_init_commands(PR #971 review follow-up from codex — installing offline docs must not require a working database), with a regression test.Verification
tests/mcp/test_project_context.py(refresh-on-miss ×4 scenarios, pattern qualification ×3),tests/mcp/test_async_client_modes.py(selector routing ×2),tests/services/test_context_service.py(to_name, run on SQLite and Postgres),tests/mcp/test_tool_build_context.py(formatter),tests/cli/test_status_wait_timeout.py,tests/cli/test_man_command.py(skip-init)just lint/just typecheckcleanAfter merge: the #952 manual's live verification gets re-run against this code and the affected pages'
[bug]observations andverified:stamps get refreshed.Fixes #954, #955, #956, #957, #959. Refs #952.
🤖 Generated with Claude Code