Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `tools-call --task` now prints the task ID and recovery commands when interrupted with Ctrl+C, so you can fetch or cancel the server-side task later
- `tasks-get` no longer suggests `tasks-cancel` when the task has already reached a terminal state (completed, failed, or cancelled)

### Fixed

- Sessions using a static bearer token (via `--header "Authorization: ..."`) no longer flip between `unauthorized` and `connecting` on every `mcpc` invocation — they stay `unauthorized` since retrying the same rejected token cannot succeed without `mcpc login` or reconnecting. OAuth-profile sessions still auto-retry because tokens may have been refreshed by another session
- Server authentication errors now include the path to the bridge log file, so you can inspect it for more detail when investigating why a session was rejected

### Removed

- Removed `tools`, `resources`, and `prompts` shorthand commands — use the full names (`tools-list`, `resources-list`, `prompts-list`) instead
Expand Down
4 changes: 3 additions & 1 deletion src/cli/commands/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
validateProfileName,
isProcessAlive,
getServerHost,
getLogsDir,
redactHeaders,
} from '../../lib/index.js';
import { DISCONNECTED_THRESHOLD_MS } from '../../lib/types.js';
Expand Down Expand Up @@ -457,7 +458,8 @@ export async function connectSession(
// Fallback: check error message for auth patterns (error may have been wrapped
// as ClientError/ServerError during bridge IPC serialization)
if (detailsError instanceof Error && isAuthenticationError(detailsError.message)) {
throw createServerAuthError(serverConfig.url || target, { sessionName: name });
const logPath = `${getLogsDir()}/bridge-${name}.log`;
throw createServerAuthError(serverConfig.url || target, { sessionName: name, logPath });
}

// Non-auth failure: session was created but server didn't respond properly.
Expand Down
5 changes: 4 additions & 1 deletion src/lib/bridge-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ async function classifyAndThrowSessionError(
logger.warn(`Failed to mark session ${sessionName} as unauthorized:`, e)
);
const target = session.server.url || session.server.command || sessionName;
const logPath = `${getLogsDir()}/bridge-${sessionName}.log`;
throw createServerAuthError(target, {
sessionName,
logPath,
...(originalError && { originalError }),
});
}
Expand Down Expand Up @@ -598,7 +600,8 @@ export async function ensureBridgeReady(sessionName: string): Promise<string> {

if (session.status === 'unauthorized') {
const target = session.server.url || session.server.command || sessionName;
throw createServerAuthError(target, { sessionName });
const logPath = `${getLogsDir()}/bridge-${sessionName}.log`;
throw createServerAuthError(target, { sessionName, logPath });
}

if (session.status === 'expired') {
Expand Down
9 changes: 6 additions & 3 deletions src/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,11 @@ export function isAuthenticationError(errorMessage: string): boolean {
* Create an AuthError with helpful login guidance for server auth failures
*
* @param target - Server URL or target for login command
* @param options - Optional session name for session-specific guidance
* @param originalError - Original error for debugging
* @param options - Optional session name for session-specific guidance and log path for debugging
*/
export function createServerAuthError(
target: string,
options?: { sessionName?: string; originalError?: Error }
options?: { sessionName?: string; logPath?: string; originalError?: Error }
): AuthError {
let hint: string;
if (options?.sessionName) {
Expand All @@ -117,6 +116,10 @@ export function createServerAuthError(
`To authenticate, run:\n` + ` mcpc login ${target}\n\n` + `Then run your command again.`;
}

if (options?.logPath) {
hint += `\n\nFor details, check logs at ${options.logPath}`;
}

return new AuthError(
`Authentication required by server.\n\n` + hint,
options?.originalError ? { originalError: options.originalError } : undefined
Expand Down
11 changes: 8 additions & 3 deletions src/lib/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,15 @@ export async function consolidateSessions(
}

// Identify crashed or unauthorized sessions eligible for automatic reconnection.
// Unauthorized sessions are included because another session sharing the same OAuth
// profile may have refreshed the tokens — the bridge reads from keychain on startup.
// Unauthorized sessions are only included when the session has an OAuth profile —
// another session sharing the same profile may have refreshed the tokens, so a
// retry can succeed as the bridge re-reads from keychain on startup. Sessions
// without a profile (e.g. static bearer token via --header) cannot self-heal, so
// retrying would just flip the status back to 'connecting' on every `mcpc` call
// and hide the real state from the user.
for (const [name, session] of Object.entries(storage.sessions)) {
if ((session?.status === 'crashed' || session?.status === 'unauthorized') && !session.pid) {
const isRetryableUnauthorized = session?.status === 'unauthorized' && !!session.profileName;
if ((session?.status === 'crashed' || isRetryableUnauthorized) && !session.pid) {
// Skip if a connection was already attempted within the cooldown window
const lastAttempt = session.lastConnectionAttemptAt
? new Date(session.lastConnectionAttemptAt).getTime()
Expand Down
98 changes: 98 additions & 0 deletions test/e2e/suites/sessions/unauthorized-persist.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#!/bin/bash
# Test: Sessions that fail with 401 and have no OAuth profile stay 'unauthorized'
# across `mcpc` invocations, even after the auto-retry cooldown has elapsed.
# Also verifies the auth error message points at the bridge log file.

source "$(dirname "$0")/../../lib/framework.sh"
test_init "sessions/unauthorized-persist" --isolated

# Start test server requiring auth. The server's auth check accepts any
# "Bearer <anything>" value, so sending an Authorization header that doesn't
# match that shape is enough to trigger 401.
start_test_server REQUIRE_AUTH=true

SESSION=$(session_name "unauth")
_SESSIONS_CREATED+=("$SESSION")

# =============================================================================
# Test: bearer-only session fails with auth error containing log path
# =============================================================================

test_case "connect with bad bearer token fails with auth error + log path"
run_mcpc connect "$TEST_SERVER_URL" "$SESSION" \
--header "X-Test: true" \
--header "Authorization: InvalidScheme not-a-bearer-token"
assert_failure
assert_exit_code 4 "should fail with auth exit code (4)"
assert_contains "$STDERR" "Authentication required by server"
# The error should point at the bridge log file so the user can investigate
assert_contains "$STDERR" "check logs at"
assert_contains "$STDERR" "bridge-${SESSION}.log"
test_pass

# =============================================================================
# Test: session is marked unauthorized right after the failed connect
# =============================================================================

test_case "failed bearer session reports 'unauthorized' status"
run_mcpc --json
assert_success
session_status=$(echo "$STDOUT" | jq -r ".sessions[] | select(.name == \"$SESSION\") | .status")
if [[ "$session_status" != "unauthorized" ]]; then
test_fail "expected status 'unauthorized', got: '$session_status'"
exit 1
fi
test_pass

# =============================================================================
# Test: unauthorized bearer session does NOT flip back to 'connecting'
# after the auto-retry cooldown expires.
#
# Before the fix, consolidateSessions() treated all unauthorized sessions as
# retry candidates and reset the status to 'connecting' on every `mcpc` call
# (after the 10s cooldown), which both hid the real state from the user and
# triggered pointless background bridge restarts. Bearer-only sessions cannot
# self-heal because the token never changes.
# =============================================================================

test_case "unauthorized bearer session stays unauthorized past the cooldown"
sessions_file="$MCPC_HOME_DIR/sessions.json"
assert_file_exists "$sessions_file"

# Age the lastConnectionAttemptAt timestamp well past the 10s cooldown and
# clear the bridge pid to simulate the bridge having exited — that's the real
# state consolidateSessions() sees when the user runs `mcpc` well after the
# original failed connect. Leaving pid set would short-circuit the retry path
# via `!session.pid`, hiding the bug we are trying to catch.
old_iso="2000-01-01T00:00:00.000Z"
tmp_file="$TEST_TMP/sessions.json.$$"
jq --arg name "$SESSION" --arg when "$old_iso" \
'.sessions[$name].lastConnectionAttemptAt = $when
| del(.sessions[$name].pid)' \
"$sessions_file" > "$tmp_file"
mv "$tmp_file" "$sessions_file"

# Re-run the list command — consolidateSessions() runs on every list.
run_mcpc --json
assert_success
session_status=$(echo "$STDOUT" | jq -r ".sessions[] | select(.name == \"$SESSION\") | .status")
if [[ "$session_status" != "unauthorized" ]]; then
test_fail "expected status to stay 'unauthorized' after cooldown, got: '$session_status'"
exit 1
fi
test_pass

# =============================================================================
# Test: using an unauthorized session surfaces the auth error + log path
# =============================================================================

test_case "using unauthorized session surfaces auth error with log path"
run_mcpc "$SESSION" tools-list
assert_failure
assert_exit_code 4 "should fail with auth exit code (4)"
assert_contains "$STDERR" "Authentication required by server"
assert_contains "$STDERR" "check logs at"
assert_contains "$STDERR" "bridge-${SESSION}.log"
test_pass

test_done
Loading