Skip to content

[RETRACTED] fix: stop CWD config discovery, listing probes, and fallback command shadowing - #36

Merged
lydakis merged 2 commits into
mainfrom
fix/stale-security-correctness-prs
Aug 28, 2026
Merged

[RETRACTED] fix: stop CWD config discovery, listing probes, and fallback command shadowing#36
lydakis merged 2 commits into
mainfrom
fix/stale-security-correctness-prs

Conversation

@lydakis

@lydakis lydakis commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Retracted

This pull request was merged outside the permitted working window and has been removed from main.

  • The merge commit 4dca40a678f7ff236d5352a9a3f160289c698576 is no longer reachable from main.
  • The associated release and tag were deleted.
  • The head branch was deleted.
  • The changes are being re-evaluated and will only be republished as a separately reviewed change.

Do not treat this merged PR record as released or current.

…shadowing

Rewrite the still-valid security/correctness fixes from stale PRs #12, #11, and #10 onto current main. Bare mcpx stays config-only, project-local .mcp.json is no longer auto-imported, and fallback merge is skipped when managed servers already exist.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ada56b8592

ℹ️ 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".

Comment thread internal/config/fallback.go Outdated
Comment on lines +169 to +170
if len(cfg.Servers) > 0 {
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop monitoring fallbacks when merging is disabled

When config.toml contains any server, this return makes every fallback, auth, and OAuth file semantically irrelevant, but RuntimeConfigSourcePathsForCWD still returns those paths and currentRuntimeConfigStamp continues reading and fingerprinting them. Consequently, changing a Cursor fallback or refreshing Codex credentials triggers an unnecessary same-CWD reload and pool configuration update even though the resulting configuration cannot change; the monitored source list should exclude fallbacks whenever this branch disables their merge.

AGENTS.md reference: AGENTS.md:L35-L39

Useful? React with 👍 / 👎.

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.

Good catch. Skip-merge and skip-watch now share the MCPXConfig origin check, so a successful fallback merge still watches those files. Fallback/auth edits no longer stamp-reload when config.toml already has managed servers.

Comment on lines 40 to +41
func (c *Catalog) ServerNames(ctx context.Context) ([]string, error) {
_ = ctx

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve explicit operations on Codex virtual servers

When codex_apps is configured, returning only literal config keys makes explicitly requested virtual servers disappear from validation paths even though Resolve still supports them. For example, mcpx linear --help first requests list_servers with IncludeHidden and now reports linear as unknown, while shim install linear similarly rejects it through listShimKnownServers; these explicit operations need to resolve the requested virtual server rather than depend on the now config-only server list.

Useful? React with 👍 / 👎.

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.

Yep — that was the regression. --help and shim install now Resolve the explicit name via list_tools instead of treating ServerNames membership as the source of truth. Default list_servers stays config-only.

Comment thread internal/config/fallback.go Outdated
Comment on lines +169 to +171
if len(cfg.Servers) > 0 {
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep fallback servers from shadowing utility commands

This condition does not prevent fallback command shadowing in the zero-managed-server case—the only case in which fallback merging now runs. If a fallback source defines add, shim, skill, completion, or __complete, the corresponding maybeHandle*Command sees that name in cfg.Servers and defers to it, so typing a reserved utility command can instead connect to or spawn the fallback server; filter reserved names from discovered servers or make utility dispatch defer only to genuinely managed origins.

AGENTS.md reference: AGENTS.md:L5-L7

Useful? React with 👍 / 👎.

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.

Fixed. Reserved names are filtered out of fallback discovery, and maybeHandle* only defers for managed origins. A fallback server named add no longer steals mcpx add.

…servers

Skip merge and source watching only when config.toml has MCPXConfig origins, not whenever cfg.Servers is non-empty. Filter reserved CLI names from fallback discovery, and resolve explicit --help / shim install targets via list_tools so Codex virtual servers still work.
@lydakis
lydakis merged commit 4dca40a into main Aug 28, 2026
7 checks passed
@lydakis
lydakis deleted the fix/stale-security-correctness-prs branch August 28, 2026 18:40
@lydakis lydakis changed the title fix: stop CWD config discovery, listing probes, and fallback command shadowing [RETRACTED] fix: stop CWD config discovery, listing probes, and fallback command shadowing Aug 28, 2026
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