[aterga]

Summary

Rebased onto current main (c6af692) — the branch is one commit on top of today's tip, so it merges cleanly. main had advanced 36 commits (project rename to imcp2, Phase-2 registration-delegation connect flow, pinned callback page + redeem, hosted-redirect allow-list, ii-auth-callbacks, OQL/structured-output tools, graceful drain); the extraction was re-derived against all of it.

Restructures the server into a Rust library crate (imcp2) plus a thin deployment binary, with the requested router structure. One McpServer serves one Internet Identity instance as two axum routers:

• McpServer::mcp_router() — one router serving …/mcp/{MCP_API} and …/mcp/{OAUTH_API}: the MCP streamable-HTTP endpoint as the router fallback (bare mount path, trailing-slash form, and sub-paths all reach it — the same breadth nest_service gives it today), bearer-gated with CORS outside the gate; the OAuth AS (authorize, pinned connect/callback page, connect/redeem, token, register) under /oauth; and the OIDC-style AS-metadata alternate at {mount}/.well-known/oauth-authorization-server.
• McpServer::well_known_router() — the /.well-known/{WELL_KNOWN_HANDLERS}, merged at the app root and parametric on the relative path to the mcp mount: the AS issuer is {public_url}{mcp_path} (RFC 8414 path issuer) and the documents live at the path-inserted locations. root_well_known_router() adds the plain-root fallback docs for the origin's default instance; auth_callbacks_router(&[…]) serves the origin-global /.well-known/ii-auth-callbacks covering every instance (II #4091 is fail-closed, so its entries must — and are tested to — match the II links' callbacks byte-for-byte).

Still axum throughout.

let server = McpServer::new(McpConfig {
    agent,                                   // inherited from the host — see below
    instance: IiInstance::beta()?,           // which Internet Identity
    public_url: "https://mcp.example.com".into(),
    mcp_path: "/mcp".into(),                 // single source of truth for every URL
    clients: SharedClients::load(),          // shared across instances on one origin
});
server.spawn_session_reaper();
let app = axum::Router::new()
    .nest_service(server.mcp_path(), server.mcp_router())
    .merge(server.well_known_router())
    .merge(server.root_well_known_router())  // default instance only
    .merge(auth_callbacks_router(&[&server]));

Agent inheritance & ic-bn-lib

The IC Agent is injected by the embedding application (McpConfig::agent), not built by the library — a boundary node or gateway passes its own route-configured client, so the process links one ic-agent. Sourcing ic-agent through ic-bn-lib is deferred: its current release pins ic-agent 0.47, and adopting it would revert #33's read-only delegation-permissions support (which needs the 0.48 this repo now uses). Recorded as a comment at the dep in Cargo.toml; revisit when ic-bn-lib releases against 0.48.

What changed

• New src/lib.rs (public API: McpServer::{new, mcp_path, mcp_router, well_known_router, root_well_known_router, spawn_session_reaper, session_gauges, shutdown}, McpConfig, IiInstance, SharedClients, auth_callbacks_router, Agent/ic_agent re-exports, IC_URL) and src/tools.rs (the MCP tools, moved byte-identical out of main.rs — verified by diff — modulo two pub(crate) keywords); src/main.rs is now a thin consumer composing beta (/mcp) + prod (/mcp-prod) and keeping the deployment machinery (landing page, /version with live/active session gauges, drain-then-cancel shutdown).
• URL topology: both instances become self-contained path issuers with OAuth under their mounts — beta /oauth/* → /mcp/oauth/* (issuer <PUBLIC_URL> → <PUBLIC_URL>/mcp), prod /prod/oauth/* → /mcp-prod/oauth/*. The 401 challenge points at the path-inserted resource-metadata doc; root discovery docs are kept for beta as a courtesy. The whole connect flow — binding-cookie Path, II link callback, pinned page's redeem URL, ii-auth-callbacks entries — derives from the mount, covered by unit + contract tests.
• IiInstance slims down to purely which II; oauth_prefix/mcp_path were deployment concerns and now live in McpConfig. PUBLIC_URL is no longer read inside the library; the management-identity derivation origin is the same value as before, so derived principals are unchanged.
• Monitoring: probes/tests updated for the new endpoints; the register probe now uses a loopback redirect_uri — the old https://example.org/callback probe is rejected by Close auth-code phishing via a hosted-redirect allow-list #69's hosted-redirect allow-list, i.e. that check is red against current main already; this fixes it in passing.
• Package name stays imcp2 (upstream's Add NOTICE file and rename project mcp-poc → IMCP2 #68 rename supersedes the earlier ic-mcp idea); adding src/lib.rs gives the same package a library target with no Dockerfile/deploy changes.

Testing

• cargo build/test --locked --all-targets (CI's exact invocation): 129 lib tests + 7 new router contract tests pass; the doc example compiles.
• Contract tests pin: per-instance discovery docs and issuers (path-inserted + OIDC alternate + beta root), the origin-global ii-auth-callbacks document listing both mounts, path-aware 401 challenges incl. trailing-slash/sub-path breadth and the RFC 6750 error="invalid_token" attribute, authorize input validation, per-mount token endpoints, and each pinned callback page posting to its own instance's redeem path.
• Live smoke test of the binary: full discovery surface, challenges, register → authorize (real 302 to II with the callback under the mount, cookie Path=/mcp/oauth), pinned pages for both instances — verified with curl.
• npm test (17 tests) and the monitoring CLI against the live server: every endpoint check passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_017h4prMHkLo6UnwnuoPgmUj

[Copilot]

Pull request overview

This PR refactors the MCP server into an embeddable Rust library crate (ic-mcp) plus a thin deployment binary (mcp-poc), exposing two axum routers: one for the MCP + OAuth surface (nestable anywhere) and one for /.well-known/* discovery (merged at the app root, parameterized by the MCP mount path).

Changes:

• Introduces ic_mcp::McpServer as the public API, moving the MCP tools into src/tools.rs and keeping src/main.rs as a deployment wrapper.
• Relocates OAuth endpoints under {mcp_mount}/oauth/* and updates discovery + monitoring probes to use the path-aware metadata and challenge.
• Adds router-level contract tests to pin mount-path behavior, discovery docs, and the 401 WWW-Authenticate challenge behavior.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/routers.rs	New router composition/contract tests for mount-path-aware discovery and auth flows.
src/tools.rs	Moves MCP tool handler implementation into the library crate.
src/main.rs	Converts binary into a thin deployment wrapper composing McpServer routers + ops niceties.
src/lib.rs	New library crate entry exposing McpServer::{new, mcp_router, well_known_router, shutdown}.
src/auth.rs	Updates OAuth flow to be mount-path-aware; adds PKCE enforcement and redirect URI hardening.
README.md	Updates docs for library usage, endpoint paths, and probing guidance.
monitoring/mcp-status/README.md	Updates monitoring docs for new OAuth endpoint paths.
monitoring/mcp-status/config.js	Updates inline references to the authorize endpoint path.
monitoring/mcp-status/checks.test.js	Updates tests to reflect new discovery + OAuth endpoint URLs and challenge location.
monitoring/mcp-status/checks.js	Updates probing logic to use new OAuth paths and path-aware resource metadata challenge.
Dockerfile	Updates build-time note to reflect tool static includes.
Cargo.toml	Renames crate to ic-mcp, adds [[bin]] for mcp-poc, adds dev-deps for router tests.
Cargo.lock	Reflects crate rename and dependency graph updates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 13 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/auth.rs:469

• The new requirement code_challenge_method == Some("S256") is security/interop-critical (RFC 7636 defaults an omitted method to plain). I couldn't find a test that exercises the valid, registered client case where code_challenge_method is missing or non-"S256" to ensure this check can’t be accidentally reordered behind client validation or removed in refactors.

Add a regression test that registers a client, calls /oauth/authorize with a missing/invalid code_challenge_method, and asserts a 400 invalid_request.

    // `token` only ever verifies S256, so require the method to say so
    // EXPLICITLY. Per RFC 7636 an *omitted* method defaults to `plain` —
    // accepting the omission and then verifying as S256 would hand a
    // spec-strict `plain` client a code it can never exchange.
    if q.code_challenge_method.as_deref() != Some("S256") {
        return oauth_err(StatusCode::BAD_REQUEST, "invalid_request", "code_challenge_method=S256 is required");
    }

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/routers.rs:27

• These integration tests call SharedClients::load(), which reads OAUTH_CLIENTS_FILE (default oauth-clients.json). That makes the tests non-hermetic: a developer’s local persisted registrations can change behavior (e.g. make a supposedly-unknown client_id valid) and cause flaky failures. Force OAUTH_CLIENTS_FILE to a temp path once for this test binary before constructing servers.

        instance,
        public_url: PUBLIC_URL.into(),
        mcp_path: mcp_path.into(),
        clients: imcp2::SharedClients::load(),
    })

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/routers.rs:29

• isolate_client_store uses a fixed filename in the system temp dir. Because Rust tests (and integration test binaries) can run in parallel, this can create cross-process interference/flakiness if another test process (or a concurrent cargo test run) touches the same OAUTH_CLIENTS_FILE path. Use a per-process-unique filename (e.g., include std::process::id()) to avoid collisions while still setting the env var only once per process.

        let mut path = std::env::temp_dir();
        path.push("imcp2-router-tests-oauth-clients.json");
        let _ = std::fs::remove_file(&path); // drop any stale file from a prior run

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/routers.rs:28

• The temp oauth-clients file name is a fixed path in the global temp dir. If two cargo test processes run concurrently on the same machine, they can race on this shared filename (delete/write) and make these router tests flaky. Include the process id (or other unique suffix) in the filename to avoid cross-process collisions.

        path.push("imcp2-router-tests-oauth-clients.json");

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/routers.rs:31

• These router tests use a fixed filename in the OS temp dir for OAUTH_CLIENTS_FILE. If the test suite is run concurrently in multiple processes (e.g., two cargo test invocations in parallel on the same machine/CI runner), they can race and interfere via the shared temp file.

        let mut path = std::env::temp_dir();
        path.push("imcp2-router-tests-oauth-clients.json");
        let _ = std::fs::remove_file(&path); // drop any stale file from a prior run
        std::env::set_var("OAUTH_CLIENTS_FILE", &path);
    });