Skip to content

Commit 2da17cd

Browse files
committed
Merge branch 'fix/357-org-scope-singleton' into 'main'
fix(cli): global-token org selection was dropped for most commands (duplicate org-scope module in the bundle) Closes #357 See merge request postgres-ai/postgresai!412
2 parents 356e6af + 43e635f commit 2da17cd

2 files changed

Lines changed: 135 additions & 6 deletions

File tree

‎cli/lib/org-scope.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,17 +146,24 @@ export function requireOrgScope(opts: OrgOptions = {}, apiKey?: string | null):
146146
* The org selected for the command currently running, set once by the CLI's
147147
* preAction hook. Threading a scope argument through the ~40 exported lib
148148
* functions instead would give 40 chances to forget one, and forgetting is
149-
* silent. Module state is safe because the CLI runs one command per process;
150-
* the MCP server serves many, so it passes its scope explicitly instead.
149+
* silent. One command per process, so a single slot is enough; the MCP server
150+
* serves many, so it passes its scope explicitly instead.
151+
*
152+
* Held on globalThis, not in a module binding: `bun build` emits this module
153+
* more than once, and a module-level `let` is then per-copy — the hook wrote
154+
* one copy while the request path read another, so the header was dropped
155+
* (#357). A registry symbol is shared by every copy.
151156
*/
152-
let activeOrgScope: OrgScope | undefined;
157+
const ACTIVE_ORG_SCOPE_KEY = Symbol.for("postgres-ai.cli.activeOrgScope");
158+
159+
type ScopeHost = { [ACTIVE_ORG_SCOPE_KEY]?: OrgScope };
153160

154161
export function setActiveOrgScope(scope: OrgScope | undefined): void {
155-
activeOrgScope = scope;
162+
(globalThis as ScopeHost)[ACTIVE_ORG_SCOPE_KEY] = scope;
156163
}
157164

158165
export function getActiveOrgScope(): OrgScope | undefined {
159-
return activeOrgScope;
166+
return (globalThis as ScopeHost)[ACTIVE_ORG_SCOPE_KEY];
160167
}
161168

162169
/**
@@ -204,7 +211,7 @@ export function buildAuthHeaders(
204211
Connection: "close",
205212
// Falls back to the invocation's selection, so a lib function that never
206213
// received an explicit scope still sends the right org.
207-
...orgScopeHeaders(scope ?? activeOrgScope),
214+
...orgScopeHeaders(scope ?? getActiveOrgScope()),
208215
...extra,
209216
};
210217
}

‎cli/test/org-scope-bundle.test.ts‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { describe, test, expect, beforeAll, afterAll } from "bun:test";
2+
import { mkdtempSync, readFileSync } from "fs";
3+
import { tmpdir } from "os";
4+
import { join, resolve } from "path";
5+
import { createServer, type Server } from "http";
6+
7+
/**
8+
* The org selector must survive BUNDLING, not just module wiring (#357).
9+
*
10+
* Every other org-scope test spawns `bin/postgres-ai.ts` through bun, where each
11+
* module exists once. `bun build` emits `lib/org-scope.ts` more than once, so a
12+
* module-level binding is per-copy: the preAction hook wrote one copy and the
13+
* request path read another, and `x-pgai-org` was silently dropped from most
14+
* commands while every source-level test passed. These tests run the BUILT
15+
* artifact -- the thing users actually execute -- against a stub server.
16+
*/
17+
18+
const CLI_DIR = resolve(import.meta.dir, "..");
19+
const ALIAS = "acme-test-org";
20+
const GLOBAL_TOKEN = "pai_global_0000000000000000000000000000000000000000";
21+
22+
let bundlePath: string;
23+
let server: Server;
24+
let port: number;
25+
/** Headers of the first request of the current spawn, reset per command. */
26+
let firstHeaders: Record<string, string> | null = null;
27+
28+
function buildBundle(): string {
29+
const outDir = mkdtempSync(join(tmpdir(), "pgai-bundle-"));
30+
const result = Bun.spawnSync(
31+
[process.execPath, "build", "./bin/postgres-ai.ts", "--outdir", outDir, "--target", "node"],
32+
{ cwd: CLI_DIR }
33+
);
34+
if (result.exitCode !== 0) {
35+
throw new Error(`bun build failed: ${new TextDecoder().decode(result.stderr)}`);
36+
}
37+
return join(outDir, "postgres-ai.js");
38+
}
39+
40+
/**
41+
* Async, never spawnSync: the stub server lives in THIS process, so a blocking
42+
* spawn would deadlock -- the CLI waits for a response the blocked event loop
43+
* cannot send.
44+
*/
45+
async function runBuilt(args: string[], env: Record<string, string> = {}): Promise<void> {
46+
firstHeaders = null;
47+
const proc = Bun.spawn([process.execPath, bundlePath, ...args, "--api-base-url", `http://127.0.0.1:${port}/`], {
48+
env: { ...process.env, PGAI_API_KEY: GLOBAL_TOKEN, PGAI_ORG: "", PGAI_ORG_ID: "", ...env },
49+
cwd: CLI_DIR,
50+
stdout: "ignore",
51+
stderr: "ignore",
52+
});
53+
await proc.exited;
54+
}
55+
56+
beforeAll(async () => {
57+
bundlePath = buildBundle();
58+
server = createServer((req, res) => {
59+
if (firstHeaders === null) {
60+
firstHeaders = req.headers as Record<string, string>;
61+
}
62+
res.writeHead(200, { "Content-Type": "application/json" });
63+
// An empty array is a valid, terminal answer for every listing below, so
64+
// the CLI exits instead of following up with a second request.
65+
res.end("[]");
66+
});
67+
await new Promise<void>((done) => server.listen(0, "127.0.0.1", done));
68+
port = (server.address() as { port: number }).port;
69+
});
70+
71+
afterAll(() => {
72+
server?.close();
73+
});
74+
75+
/**
76+
* One command per org-scoped transport module. `issues list` used to be the
77+
* only passing command and was the one the feature's manual e2e sampled, so it
78+
* is deliberately NOT representative on its own -- each module gets an entry.
79+
*/
80+
const COMMANDS: Array<{ label: string; argv: string[] }> = [
81+
{ label: "projects (lib/joe)", argv: ["projects"] },
82+
{ label: "issues list (lib/issues)", argv: ["issues", "list"] },
83+
{ label: "issues view (lib/issues)", argv: ["issues", "view", "1"] },
84+
{ label: "reports list (lib/reports)", argv: ["reports", "list"] },
85+
{ label: "joe activity (lib/joe)", argv: ["joe", "activity", "--project", "1"] },
86+
{ label: "dblab clone list (lib/dblab)", argv: ["dblab", "clone", "list", "--project", "1"] },
87+
];
88+
89+
describe("the org selector survives bundling", () => {
90+
for (const { label, argv } of COMMANDS) {
91+
test(`${label} sends x-pgai-org from the built bundle`, async () => {
92+
await runBuilt([...argv, "--org", ALIAS]);
93+
94+
expect(firstHeaders).not.toBeNull();
95+
expect(firstHeaders?.["x-pgai-org"]).toBe(ALIAS);
96+
});
97+
}
98+
99+
test("--org-id rides the same path", async () => {
100+
await runBuilt(["projects", "--org-id", "5225"]);
101+
102+
expect(firstHeaders?.["x-pgai-org-id"]).toBe("5225");
103+
});
104+
105+
test("PGAI_ORG reaches the wire from the built bundle too", async () => {
106+
await runBuilt(["projects"], { PGAI_ORG: ALIAS });
107+
108+
expect(firstHeaders?.["x-pgai-org"]).toBe(ALIAS);
109+
});
110+
});
111+
112+
describe("the scope is not held in a per-copy module binding", () => {
113+
test("the built bundle declares no module-level activeOrgScope", () => {
114+
const src = readFileSync(bundlePath, "utf8");
115+
116+
// A bare `let activeOrgScope;` (or a renamed `activeOrgScope2`) means the
117+
// selection is back in module state, which the duplicate-module emit makes
118+
// per-copy. The registry symbol is the only form shared across copies.
119+
expect(src).not.toMatch(/\n\s*(?:let|var)\s+activeOrgScope\d*\s*[;=]/);
120+
expect(src).toContain("postgres-ai.cli.activeOrgScope");
121+
});
122+
});

0 commit comments

Comments
 (0)