Public: anyone can read this; the crew can change it.
A test pins syncing a mirror whose origin has no stored token: it says to reconnect, in words, and never crashes #7186
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/7177-a-test-pins-syncing-a-mirror-whose-origi → main 1 commit fork 1 thread · 1 unresolved
3 files
changed +168
−2
src/origins/credential.ts +3 −1
| @@ -49,7 +49,9 @@ export function credentialFor( | ||
| 49 | 49 | app: App, |
| 50 | 50 | origin: Origin, |
| 51 | 51 | ): Result<ProviderCredential, CredentialError> { |
| 52 | if (sealedSecrets(app.db, origin.id) === undefined) return err("no_token"); | |
| 52 | // A row whose secret was blanked is as unconnected as a missing row. | |
| 53 | const sealed = sealedSecrets(app.db, origin.id); | |
| 54 | if (sealed === undefined || sealed.token === "") return err("no_token"); | |
| 53 | 55 | return ok({ |
| 54 | 56 | kind: origin.kind, |
| 55 | 57 | apiUrl: origin.apiUrl, |
src/origins/sync.ts +3 −1
| @@ -30,6 +30,7 @@ import { | ||
| 30 | 30 | getCursor, |
| 31 | 31 | listAllOrigins, |
| 32 | 32 | listMirrors, |
| 33 | markNeedsReauth, | |
| 33 | 34 | originForRepo, |
| 34 | 35 | recordFetch, |
| 35 | 36 | rememberOriginRepo, |
| @@ -161,7 +162,8 @@ async function runOrigin( | ||
| 161 | 162 | |
| 162 | 163 | const provider = providerForOrigin(app, origin); |
| 163 | 164 | if (!provider.ok) { |
| 164 | report.error = "this origin has no usable credential"; | |
| 165 | report.error = `this origin has no stored token: reconnect ${origin.login} at ${origin.webUrl}`; | |
| 166 | markNeedsReauth(app.db, origin.id, report.error); | |
| 165 | 167 | recordSync(app.db, origin.id, { at: startedAt, error: report.error }); |
| 166 | 168 | schedule(app, origin.id, SYNC_INTERVAL_MS); |
| 167 | 169 | return report; |
test/origin-no-token.test.ts added +162
| @@ -0,0 +1,162 @@ | ||
| 1 | /** | |
| 2 | * An origin with no stored token (#7177): `credentialFor` answers `no_token`. | |
| 3 | * A sync through it is a refusal that says to reconnect, in words — through | |
| 4 | * the API, through MCP `sync`, and on the origin's settings page — and the | |
| 5 | * mirror's refs do not move. | |
| 6 | */ | |
| 7 | import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test"; | |
| 8 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 9 | import { tmpdir } from "node:os"; | |
| 10 | import { join } from "node:path"; | |
| 11 | import { createApp, type App } from "../src/app.ts"; | |
| 12 | import { createToken } from "../src/core/tokens.ts"; | |
| 13 | import { getUserByName } from "../src/core/users.ts"; | |
| 14 | import { resolveOid } from "../src/git/merge.ts"; | |
| 15 | import { repoDir } from "../src/git/repos.ts"; | |
| 16 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 17 | import { syncsSettled } from "../src/origins/sync.ts"; | |
| 18 | import { mirrorAndWait, newFakeState, serveFakeForge, type FakeForge } from "./fake-origin.ts"; | |
| 19 | import { proveAll } from "./helpers/verified.ts"; | |
| 20 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 21 | ||
| 22 | setDefaultTimeout(60_000); | |
| 23 | ||
| 24 | const LOGIN = "octo"; | |
| 25 | const TOKEN = "octo-origin-secret-3b7e"; | |
| 26 | const PASSWORD = "alice-login-secret"; | |
| 27 | const basic = (u: string, p: string) => `Basic ${Buffer.from(`${u}:${p}`).toString("base64")}`; | |
| 28 | ||
| 29 | describe("syncing a mirror whose origin has no stored token", () => { | |
| 30 | const dirs: string[] = []; | |
| 31 | let app: App; | |
| 32 | let servers: Servers; | |
| 33 | let originApp: App; | |
| 34 | let originServers: Servers; | |
| 35 | let fake: FakeForge; | |
| 36 | let adminTok = ""; | |
| 37 | let originId = ""; | |
| 38 | ||
| 39 | const api = (path: string, init?: RequestInit) => { | |
| 40 | const headers = new Headers(init?.headers); | |
| 41 | headers.set("content-type", "application/json"); | |
| 42 | headers.set("authorization", basic("alice", PASSWORD)); | |
| 43 | return fetch(`${servers.url}${path}`, { ...init, headers }); | |
| 44 | }; | |
| 45 | const mirrorMain = () => resolveOid(repoDir(app.reposRoot, "alice", "vault"), "refs/heads/main"); | |
| 46 | const upstreamDir = () => repoDir(originApp.reposRoot, LOGIN, "vault"); | |
| 47 | ||
| 48 | const mcpSync = async (args: unknown) => { | |
| 49 | const res = await fetch(`${servers.url}/mcp`, { | |
| 50 | method: "POST", | |
| 51 | headers: { | |
| 52 | "content-type": "application/json", | |
| 53 | accept: "application/json, text/event-stream", | |
| 54 | authorization: `Bearer ${adminTok}`, | |
| 55 | }, | |
| 56 | body: JSON.stringify({ | |
| 57 | jsonrpc: "2.0", | |
| 58 | id: 1, | |
| 59 | method: "tools/call", | |
| 60 | params: { name: "sync", arguments: args }, | |
| 61 | }), | |
| 62 | }); | |
| 63 | const body = (await res.json()) as any; | |
| 64 | return { text: String(body.result?.content?.[0]?.text ?? ""), isError: body.result?.isError === true }; | |
| 65 | }; | |
| 66 | ||
| 67 | beforeAll(async () => { | |
| 68 | for (const p of ["forge-notoken-", "forge-notoken-up-"]) dirs.push(await mkdtemp(join(tmpdir(), p))); | |
| 69 | originApp = createApp({ dataDir: dirs[1]!, port: 0 }); | |
| 70 | originServers = await serve(originApp); | |
| 71 | await seedAdmin(originApp, { username: LOGIN, email: "[email protected]", password: TOKEN }); | |
| 72 | const oh = { "content-type": "application/json", authorization: basic(LOGIN, TOKEN) }; | |
| 73 | await fetch(`${originServers.url}/api/v1/repos`, { | |
| 74 | method: "POST", | |
| 75 | headers: oh, | |
| 76 | body: JSON.stringify({ name: "vault", visibility: "public" }), | |
| 77 | }); | |
| 78 | await fetch(`${originServers.url}/api/v1/repos/octo/vault/contents`, { | |
| 79 | method: "POST", | |
| 80 | headers: oh, | |
| 81 | body: JSON.stringify({ branch: "main", message: "first", files: [{ path: "a.txt", content: "one\n" }] }), | |
| 82 | }); | |
| 83 | const state = newFakeState("github", LOGIN, TOKEN); | |
| 84 | state.repos.push({ | |
| 85 | id: 9, | |
| 86 | fullName: "octo/vault", | |
| 87 | private: true, | |
| 88 | cloneUrl: `${originServers.url}/octo/vault.git`, | |
| 89 | defaultBranch: "main", | |
| 90 | htmlUrl: `${originServers.url}/octo/vault`, | |
| 91 | }); | |
| 92 | fake = serveFakeForge(state); | |
| 93 | ||
| 94 | app = createApp({ dataDir: dirs[0]!, port: 0 }); | |
| 95 | servers = await serve(app); | |
| 96 | const alice = await seedAdmin(app, { username: "alice", email: "[email protected]", password: PASSWORD }); | |
| 97 | if (!alice.ok) throw new Error(alice.error); | |
| 98 | const connected = (await ( | |
| 99 | await api("/api/v1/user/origins", { | |
| 100 | method: "POST", | |
| 101 | body: JSON.stringify({ host: fake.url, kind: "github", token: TOKEN }), | |
| 102 | }) | |
| 103 | ).json()) as { id: string }; | |
| 104 | originId = connected.id; | |
| 105 | await mirrorAndWait( | |
| 106 | await api(`/api/v1/user/origins/${originId}/mirror`, { | |
| 107 | method: "POST", | |
| 108 | body: JSON.stringify({ fullName: "octo/vault" }), | |
| 109 | }), | |
| 110 | ); | |
| 111 | proveAll(app); | |
| 112 | adminTok = createToken(app.db, getUserByName(app.db, "alice")!.id, "admin", "admin").token; | |
| 113 | }); | |
| 114 | ||
| 115 | afterAll(async () => { | |
| 116 | await syncsSettled(); | |
| 117 | fake.stop(); | |
| 118 | servers.stop(); | |
| 119 | originServers.stop(); | |
| 120 | app.close(); | |
| 121 | originApp.close(); | |
| 122 | for (const d of dirs) await rm(d, { recursive: true, force: true }); | |
| 123 | }); | |
| 124 | ||
| 125 | test("the sealed secret is removed, the upstream moves, and every sync says to reconnect", async () => { | |
| 126 | const before = await mirrorMain(); | |
| 127 | expect(before).toBeDefined(); | |
| 128 | ||
| 129 | // The sealed secret is gone; the origin row is not. | |
| 130 | app.db.query("UPDATE origin SET token_enc = '', refresh_token_enc = '' WHERE id = ?1").run(originId); | |
| 131 | ||
| 132 | // The upstream moves on, so an unchanged mirror proves nothing was fetched. | |
| 133 | await fetch(`${originServers.url}/api/v1/repos/octo/vault/contents`, { | |
| 134 | method: "POST", | |
| 135 | headers: { "content-type": "application/json", authorization: basic(LOGIN, TOKEN) }, | |
| 136 | body: JSON.stringify({ branch: "main", message: "second", files: [{ path: "b.txt", content: "two\n" }] }), | |
| 137 | }); | |
| 138 | expect(await resolveOid(upstreamDir(), "refs/heads/main")).not.toBe(before); | |
| 139 | ||
| 140 | const viaApi = await api("/api/v1/repos/alice/vault/sync", { method: "POST" }); | |
| 141 | expect(viaApi.status).toBeLessThan(500); | |
| 142 | const report = (await viaApi.json()) as { error: string }; | |
| 143 | expect(report.error).toContain("reconnect"); | |
| 144 | ||
| 145 | const viaOrigin = await api(`/api/v1/user/origins/${originId}/sync`, { method: "POST" }); | |
| 146 | expect(viaOrigin.status).toBeLessThan(500); | |
| 147 | expect(((await viaOrigin.json()) as { error: string }).error).toContain("reconnect"); | |
| 148 | ||
| 149 | const viaMcp = await mcpSync({ repo: "alice/vault" }); | |
| 150 | expect(viaMcp.isError).toBe(true); | |
| 151 | expect(viaMcp.text).toContain("sync_failed"); | |
| 152 | expect(viaMcp.text).toContain("reconnect"); | |
| 153 | ||
| 154 | expect(await mirrorMain()).toBe(before!); | |
| 155 | }); | |
| 156 | ||
| 157 | test("the origin's settings page shows the same reconnect line", async () => { | |
| 158 | const page = (await (await api("/settings/origins")).text()).replace(/\s+/g, " "); | |
| 159 | expect(page).toContain("this origin needs to be reconnected"); | |
| 160 | expect(page).toContain("Re-authorize octo"); | |
| 161 | }); | |
| 162 | }); |
new above
offline
Low:
POST /api/v1/repos/:owner/:name/syncreturns this report to any writer of the mirror, not only the origin's owner. So a collaborator now learns the owner's upstream login and host, and is told to reconnect an origin they cannot reach. Consider keeping the login out ofreport.error, for example "this origin has no stored token: reconnect it in settings". The full line can stay inlast_error, which only the owner sees. It is also worth knowing that this wording shows only on the first failed sync. After that, theneedsReauthbranch above answers with the shorter line.