Skip to content

Commit d6158fc

Browse files
anatoly314claude
andcommitted
feat(tunnel): auto-relogin on session_expired during first connect
When the initial tunnel connection fails with session_expired (dead or invalid refresh token), the CLI previously printed "please run `ankimcp --login`" and exited, forcing a manual re-auth. Now, in an interactive tunnel session, it runs the existing device-flow login automatically and retries the connection once. Behavior: - On a first-connect session_expired, if stdout is a TTY and no relogin has been attempted yet, run performLogin (prints URL + code, opens browser, polls, saves creds) and retry connect() once with the fresh credentials. - Single-shot: a second session_expired, a failed login, a non-TTY run, or any other error falls back to the previous error-and-exit. No loop. - Scope is first-connect only; the mid-session error-event path is unchanged. TunnelClient is untouched — this lives entirely in the command layer. Also extract the duplicated device-flow error reporting into a shared reportLoginError(cli, error) helper, now used by handleLogin, ensureCredentials, and the relogin path. Tests: fix the now-stale ../perform-login module mocks to export reportLoginError; add coverage for relogin success, single-shot bound, login-failure, non-TTY gate, non-session errors, and reportLoginError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f675baa commit d6158fc

6 files changed

Lines changed: 448 additions & 64 deletions

File tree

src/tunnel/commands/__tests__/login.command.spec.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,38 @@
11
// Mocks must be declared before module imports that depend on them.
2-
jest.mock("../perform-login", () => ({
3-
performLogin: jest.fn(),
4-
translateDeviceFlowError: jest.fn(
2+
//
3+
// `reportLoginError` is now imported by `login.command.ts`, so the factory MUST
4+
// export it — otherwise it resolves to `undefined` and the catch path throws
5+
// `TypeError: reportLoginError is not a function`. We give it a real-ish stub
6+
// that mirrors production (DeviceFlowError → translated line via the SAME
7+
// `translateDeviceFlowError` jest.fn the tests spy on; otherwise the generic
8+
// `Login failed: <msg>` line with the Error forwarded as the 2nd arg). This
9+
// keeps the existing `mockedTranslate.toHaveBeenCalledWith` / `cli.error`
10+
// assertions valid while exercising the new symbol.
11+
jest.mock("../perform-login", () => {
12+
const translateDeviceFlowError = jest.fn(
513
(err: { message: string }) => `translated: ${err.message}`,
6-
),
7-
}));
14+
);
15+
return {
16+
performLogin: jest.fn(),
17+
translateDeviceFlowError,
18+
reportLoginError: jest.fn(
19+
(
20+
cli: { error: (msg: string, err?: unknown) => void },
21+
error: unknown,
22+
) => {
23+
const { DeviceFlowError } = jest.requireMock("@/tunnel");
24+
if (error instanceof DeviceFlowError) {
25+
cli.error(translateDeviceFlowError(error as { message: string }));
26+
} else {
27+
cli.error(
28+
`Login failed: ${error instanceof Error ? error.message : String(error)}`,
29+
error instanceof Error ? error : undefined,
30+
);
31+
}
32+
},
33+
),
34+
};
35+
});
836

937
jest.mock("@/tunnel", () => {
1038
class DeviceFlowError extends Error {

src/tunnel/commands/__tests__/perform-login.spec.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { performLogin, translateDeviceFlowError } from "../perform-login";
1+
import {
2+
performLogin,
3+
reportLoginError,
4+
translateDeviceFlowError,
5+
} from "../perform-login";
26
import {
37
CredentialsService,
48
DeviceFlowError,
@@ -73,6 +77,58 @@ describe("translateDeviceFlowError", () => {
7377
});
7478
});
7579

80+
describe("reportLoginError", () => {
81+
let cli: jest.Mocked<Cli>;
82+
let exitSpy: jest.SpyInstance;
83+
84+
beforeEach(() => {
85+
cli = makeStubCli();
86+
// Spy as a no-op (NOT a throw): reportLoginError must only PRINT, never
87+
// terminate the process. If it ever called exit, the assertions below
88+
// would catch it without killing the Jest worker.
89+
exitSpy = jest
90+
.spyOn(process, "exit")
91+
.mockImplementation((() => undefined) as never);
92+
});
93+
94+
afterEach(() => {
95+
jest.restoreAllMocks();
96+
});
97+
98+
it("renders a DeviceFlowError via translateDeviceFlowError (single message arg)", () => {
99+
const err = new DeviceFlowError("denied", "access_denied");
100+
101+
reportLoginError(cli, err);
102+
103+
// Exact wording the real translateDeviceFlowError produces for this code —
104+
// pins that reportLoginError delegates to it for DeviceFlowErrors.
105+
expect(cli.error).toHaveBeenCalledTimes(1);
106+
expect(cli.error).toHaveBeenCalledWith(
107+
"Authentication was denied. Please try again with 'ankimcp --login'",
108+
);
109+
// The DeviceFlow branch passes no second (Error) argument.
110+
expect(cli.error.mock.calls[0]).toHaveLength(1);
111+
expect(exitSpy).not.toHaveBeenCalled();
112+
});
113+
114+
it("renders a generic Error as `Login failed: <msg>` with the Error forwarded as 2nd arg", () => {
115+
const err = new Error("boom");
116+
117+
reportLoginError(cli, err);
118+
119+
expect(cli.error).toHaveBeenCalledTimes(1);
120+
expect(cli.error).toHaveBeenCalledWith("Login failed: boom", err);
121+
expect(exitSpy).not.toHaveBeenCalled();
122+
});
123+
124+
it("stringifies a non-Error value and forwards no Error argument", () => {
125+
reportLoginError(cli, "weird");
126+
127+
expect(cli.error).toHaveBeenCalledWith("Login failed: weird", undefined);
128+
expect(exitSpy).not.toHaveBeenCalled();
129+
});
130+
});
131+
76132
describe("performLogin", () => {
77133
let credentialsService: jest.Mocked<CredentialsService>;
78134
let deviceFlowService: jest.Mocked<DeviceFlowService>;

0 commit comments

Comments
 (0)