Skip to content

Commit fe49d88

Browse files
os-helpclaude
andauthored
fix(showcase): resolve connector self-URL from the environment (#7538) (#7621)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent ae66145 commit fe49d88

5 files changed

Lines changed: 229 additions & 10 deletions

File tree

examples/app-showcase/objectstack.config.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { allEmails } from './src/system/emails/index.js';
3535
import { allBooks } from './src/system/books/index.js';
3636
import { allApis } from './src/system/apis/index.js';
3737
import { allConnectors } from './src/system/connectors/index.js';
38+
import { resolveShowcaseSelfUrl } from './src/system/self-url.js';
3839
import {
3940
allPositions,
4041
allPermissionSets,
@@ -110,7 +111,11 @@ export default defineStack({
110111
// ships the dispatch node + an empty registry; these plugins populate it.
111112
// • rest → points at the running server itself, so the REST connector
112113
// flow's call + response are observable on the flow run with no
113-
// external dependency. Override the target with SHOWCASE_SELF_URL.
114+
// external dependency. The target is resolved by
115+
// src/system/self-url.ts — SHOWCASE_SELF_URL, else the CLI's
116+
// own OS_PORT / PORT, else http://127.0.0.1:3000 — and the
117+
// declarative connector instances in src/system/connectors/
118+
// resolve through the SAME helper (#7538).
114119
// • slack → registered so TaskCompletedSlackFlow resolves its connector;
115120
// live posting needs a real bot token (set SLACK_BOT_TOKEN).
116121
// • openapi → option-less: contributes only the `openapi` provider factory
@@ -132,7 +137,10 @@ export default defineStack({
132137
new ConnectorMcpPlugin({ declarativeStdio: ['node'] }),
133138
new ConnectorRestPlugin({
134139
name: 'rest',
135-
baseUrl: process.env.SHOWCASE_SELF_URL ?? 'http://127.0.0.1:3000',
140+
// Shared with the declarative connector instances in
141+
// src/system/connectors/ (#7538) — one resolver, so the two self-URL
142+
// sources cannot drift apart.
143+
baseUrl: resolveShowcaseSelfUrl(),
136144
}),
137145
new ConnectorSlackPlugin({
138146
token: process.env.SLACK_BOT_TOKEN ?? 'xoxb-showcase-demo-token',

examples/app-showcase/src/system/connectors/index.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import { defineConnector, type Connector } from '@objectstack/spec/integration';
44

5+
import { resolveShowcaseSelfUrl } from '../self-url.js';
6+
57
/**
68
* Declarative `connectors:` — the collection now holds BOTH kinds (ADR-0097):
79
*
@@ -50,11 +52,16 @@ export const StatusApiConnector = defineConnector({
5052
'connector_action and appears in GET /connectors.',
5153
provider: 'rest',
5254
providerConfig: {
53-
// Points at the running server itself (the showcase dev port is 3000), so
54-
// the dispatch is observable with no external dependency. Kept a literal
55-
// because metadata files don't read env — the env-driven `rest` plugin
56-
// connector in objectstack.config.ts is the tunable one.
57-
baseUrl: 'http://127.0.0.1:3000',
55+
// Points at the running server itself, so the dispatch is observable with
56+
// no external dependency. Resolved from the environment (#7538) via the
57+
// same helper objectstack.config.ts's `rest` plugin uses, so the two
58+
// self-URL sources cannot diverge: SHOWCASE_SELF_URL, else the CLI's own
59+
// OS_PORT / PORT, else http://127.0.0.1:3000. A literal here made every
60+
// self-ping flow fail `fetch failed` on any instance not listening on 3000
61+
// — and metadata modules DO read env: this file is evaluated by whichever
62+
// process loads objectstack.config.ts (see ../self-url.ts for when that is
63+
// boot time vs build time).
64+
baseUrl: resolveShowcaseSelfUrl(),
5865
},
5966
auth: { type: 'none' },
6067
});
@@ -85,8 +92,13 @@ export const StatusOpenApiConnector = defineConnector({
8592
// holds objectstack.config.ts (the CLI passes it as the automation
8693
// service's packageRoot). Inline documents and http(s) URLs stay valid.
8794
spec: './src/system/connectors/status-openapi.json',
88-
// Same self-pointing literal rationale as StatusApiConnector above.
89-
baseUrl: 'http://127.0.0.1:3000',
95+
// Same env-resolved self URL as StatusApiConnector above (#7538). This
96+
// OVERRIDES the document's own `servers[0].url` — createOpenApiConnector
97+
// resolves `config.baseUrl ?? document.servers?.[0]?.url`
98+
// (packages/connectors/connector-openapi/src/openapi-connector.ts) — so the
99+
// static literal in status-openapi.json stays a documentation default and
100+
// is not what the dispatch actually uses.
101+
baseUrl: resolveShowcaseSelfUrl(),
90102
},
91103
auth: { type: 'none' },
92104
});

examples/app-showcase/src/system/connectors/status-openapi.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"info": {
44
"title": "Showcase Status API",
55
"version": "1.0.0",
6-
"description": "Minimal OpenAPI document for the showcase's own health probe. Referenced by the StatusOpenApiConnector declarative instance (src/system/connectors/index.ts) as a package-relative file path — the #3016 / ADR-0096 spec form resolved and confined to this package's root at boot."
6+
"description": "Minimal OpenAPI document for the showcase's own health probe. Referenced by the StatusOpenApiConnector declarative instance (src/system/connectors/index.ts) as a package-relative file path — the #3016 / ADR-0096 spec form resolved and confined to this package's root at boot. NOTE (#7538): a static document cannot follow the port this instance actually bound, so `servers[0].url` below is only the documentation default. The connector supplies an env-resolved `providerConfig.baseUrl` (src/system/self-url.ts), and createOpenApiConnector resolves `config.baseUrl ?? document.servers[0].url` — so the value below is overridden on every dispatch."
77
},
88
"servers": [{ "url": "http://127.0.0.1:3000" }],
99
"paths": {
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The showcase's **single source of truth for its own base URL** (#7538).
5+
*
6+
* Several showcase surfaces point at the running server itself so their demo is
7+
* observable with no external dependency: the hand-wired `rest` connector
8+
* plugin in objectstack.config.ts, and the two declarative REST/OpenAPI
9+
* connector instances in `src/system/connectors/`. Before #7538 the plugin read
10+
* `SHOWCASE_SELF_URL` while the declarative instances carried the literal
11+
* `http://127.0.0.1:3000` — so on any instance NOT listening on 3000 (CI, QA,
12+
* any dev boot on an isolated port) every flow dispatching through those
13+
* connectors failed with `fetch failed`. That failure is indistinguishable from
14+
* a sandbox egress block, which is what made it expensive: the QA run in #7516
15+
* only proved it was an address problem by putting a TCP forwarder on 3000.
16+
*
17+
* Resolution order — most explicit first:
18+
*
19+
* 1. `SHOWCASE_SELF_URL` — a full base URL. The escape hatch for anything the
20+
* port alone cannot express (a different host/interface, https, a proxy
21+
* prefix). Kept as the primary knob because objectstack.config.ts already
22+
* documented it.
23+
* 2. `OS_PORT`, then its deprecated alias `PORT` — **the same names, in the
24+
* same order, that the CLI itself reads to choose the listen port**
25+
* (`packages/cli/src/commands/serve.ts`: `readEnvWithDeprecation('OS_PORT',
26+
* 'PORT') ?? '3000'`). Following the CLI's own inputs is what makes the
27+
* isolated-port boot self-ping correctly with no extra configuration — the
28+
* exact case #7538 was filed for.
29+
* 3. `http://127.0.0.1:3000` — the historical literal, unchanged, so a plain
30+
* `pnpm dev` behaves exactly as before.
31+
*
32+
* **When this is read matters.** These are ordinary Node modules evaluated at
33+
* config load, so the read happens in whichever process loads
34+
* objectstack.config.ts. On the `os dev` / `os serve` path that is the serving
35+
* process itself, so the value follows the live environment. On the
36+
* artifact-only path (`os build` once, then `os start --artifact`) the
37+
* connector metadata is serialized into `dist/objectstack.json`, so the value
38+
* is frozen at BUILD time — set the env for the build, not just the boot.
39+
* (The plugin in `plugins:` is code and cannot be serialized at all, so it only
40+
* exists on the config-load path.)
41+
*/
42+
43+
// Ambient `process` for the env reads below — the showcase tsconfig doesn't
44+
// pull in `@types/node`, but the CLI provides the real `process` at runtime.
45+
// Same idiom (and same reason) as the declaration in objectstack.config.ts:
46+
// keeps `pnpm typecheck` green without widening the type surface.
47+
declare const process: { env: Record<string, string | undefined> };
48+
49+
/** The listen port the CLI defaults to when neither `OS_PORT` nor `PORT` is set. */
50+
export const SHOWCASE_DEFAULT_PORT = '3000';
51+
52+
/** The base URL used when nothing in the environment says otherwise. */
53+
export const SHOWCASE_DEFAULT_SELF_URL = `http://127.0.0.1:${SHOWCASE_DEFAULT_PORT}`;
54+
55+
/**
56+
* Resolve the base URL at which this showcase instance can reach itself.
57+
*
58+
* @param env - Environment to read. Defaults to `process.env`; injectable so
59+
* tests can assert each precedence rung without mutating the real process.
60+
*/
61+
export function resolveShowcaseSelfUrl(env: Record<string, string | undefined> = process.env): string {
62+
const explicit = env.SHOWCASE_SELF_URL?.trim();
63+
if (explicit) return explicit.replace(/\/+$/, '');
64+
65+
const port = env.OS_PORT?.trim() || env.PORT?.trim() || SHOWCASE_DEFAULT_PORT;
66+
return `http://127.0.0.1:${port}`;
67+
}
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, afterEach, vi } from 'vitest';
4+
import {
5+
resolveShowcaseSelfUrl,
6+
SHOWCASE_DEFAULT_SELF_URL,
7+
SHOWCASE_DEFAULT_PORT,
8+
} from '../src/system/self-url.js';
9+
10+
/**
11+
* #7538 — the showcase's self-pointing connectors must follow the port the
12+
* instance actually bound.
13+
*
14+
* Before the fix, `StatusApiConnector` and `StatusOpenApiConnector` carried the
15+
* literal `http://127.0.0.1:3000` in `providerConfig`, so every flow that
16+
* dispatched through them failed with `fetch failed` on any instance not
17+
* listening on 3000 — CI, QA, any dev boot on an isolated port. The failure is
18+
* indistinguishable from a sandbox egress block, which is what made it
19+
* expensive to diagnose (the QA run in #7516 needed a TCP forwarder on 3000 to
20+
* prove the address was the whole problem).
21+
*
22+
* The guards below pin both halves of the contract the fix must hold:
23+
* a non-3000 environment moves the connectors' `baseUrl`, and an empty
24+
* environment still resolves to the historical literal.
25+
*
26+
* Reverse verification (expected direction: RED on revert). Restoring the
27+
* literal at src/system/connectors/index.ts:57 / :89 turns the two
28+
* "follows the environment" cases below red while the two "defaults" cases stay
29+
* green — a literal is, by construction, still correct in the default case.
30+
* That asymmetry is the point: the default-case assertions alone can never
31+
* detect the bug, so both halves are required.
32+
*/
33+
34+
// Ambient `process` with `env` — test/node-shim.d.ts declares the global as
35+
// `{ cwd(): string }` only, and this module-scoped declaration shadows it
36+
// rather than widening the shared shim (the same idiom objectstack.config.ts
37+
// uses for its own env reads).
38+
declare const process: { env: Record<string, string | undefined> };
39+
40+
/** Load the connector metadata fresh under a given environment. */
41+
async function connectorsUnderEnv(env: Record<string, string | undefined>) {
42+
vi.resetModules();
43+
const saved: Record<string, string | undefined> = {};
44+
for (const key of ['SHOWCASE_SELF_URL', 'OS_PORT', 'PORT']) {
45+
saved[key] = process.env[key];
46+
if (env[key] === undefined) delete process.env[key];
47+
else process.env[key] = env[key];
48+
}
49+
try {
50+
return await import('../src/system/connectors/index.js');
51+
} finally {
52+
for (const [key, value] of Object.entries(saved)) {
53+
if (value === undefined) delete process.env[key];
54+
else process.env[key] = value;
55+
}
56+
}
57+
}
58+
59+
function baseUrlOf(connector: { providerConfig?: Record<string, unknown> }): unknown {
60+
return connector.providerConfig?.baseUrl;
61+
}
62+
63+
afterEach(() => {
64+
vi.resetModules();
65+
});
66+
67+
describe('resolveShowcaseSelfUrl precedence (#7538)', () => {
68+
it('prefers an explicit SHOWCASE_SELF_URL over any port', () => {
69+
expect(resolveShowcaseSelfUrl({ SHOWCASE_SELF_URL: 'https://showcase.internal', OS_PORT: '4711' }))
70+
.toBe('https://showcase.internal');
71+
});
72+
73+
it('trims a trailing slash so callers can join paths without doubling it', () => {
74+
expect(resolveShowcaseSelfUrl({ SHOWCASE_SELF_URL: 'http://127.0.0.1:8080/' }))
75+
.toBe('http://127.0.0.1:8080');
76+
});
77+
78+
it("follows the CLI's own OS_PORT when no explicit URL is set", () => {
79+
expect(resolveShowcaseSelfUrl({ OS_PORT: '4711' })).toBe('http://127.0.0.1:4711');
80+
});
81+
82+
it("follows the CLI's deprecated PORT alias when OS_PORT is absent", () => {
83+
expect(resolveShowcaseSelfUrl({ PORT: '5822' })).toBe('http://127.0.0.1:5822');
84+
});
85+
86+
it('prefers OS_PORT over PORT — the same order readEnvWithDeprecation uses', () => {
87+
expect(resolveShowcaseSelfUrl({ OS_PORT: '4711', PORT: '5822' })).toBe('http://127.0.0.1:4711');
88+
});
89+
90+
it('falls back to the historical literal on an empty environment', () => {
91+
expect(resolveShowcaseSelfUrl({})).toBe('http://127.0.0.1:3000');
92+
expect(SHOWCASE_DEFAULT_SELF_URL).toBe('http://127.0.0.1:3000');
93+
expect(SHOWCASE_DEFAULT_PORT).toBe('3000');
94+
});
95+
96+
it('ignores a blank value rather than resolving to a broken URL', () => {
97+
expect(resolveShowcaseSelfUrl({ SHOWCASE_SELF_URL: ' ', OS_PORT: ' ' }))
98+
.toBe('http://127.0.0.1:3000');
99+
});
100+
});
101+
102+
describe('self-pointing connector instances follow the environment (#7538)', () => {
103+
it('StatusApiConnector resolves against a non-3000 port', async () => {
104+
const mod = await connectorsUnderEnv({ OS_PORT: '4711' });
105+
expect(baseUrlOf(mod.StatusApiConnector)).toBe('http://127.0.0.1:4711');
106+
});
107+
108+
it('StatusOpenApiConnector resolves against a non-3000 port', async () => {
109+
const mod = await connectorsUnderEnv({ OS_PORT: '4711' });
110+
expect(baseUrlOf(mod.StatusOpenApiConnector)).toBe('http://127.0.0.1:4711');
111+
});
112+
113+
it('both instances honour an explicit SHOWCASE_SELF_URL', async () => {
114+
const mod = await connectorsUnderEnv({ SHOWCASE_SELF_URL: 'http://127.0.0.1:8123' });
115+
expect(baseUrlOf(mod.StatusApiConnector)).toBe('http://127.0.0.1:8123');
116+
expect(baseUrlOf(mod.StatusOpenApiConnector)).toBe('http://127.0.0.1:8123');
117+
});
118+
119+
it('both instances still default to 127.0.0.1:3000 with nothing set', async () => {
120+
const mod = await connectorsUnderEnv({});
121+
expect(baseUrlOf(mod.StatusApiConnector)).toBe('http://127.0.0.1:3000');
122+
expect(baseUrlOf(mod.StatusOpenApiConnector)).toBe('http://127.0.0.1:3000');
123+
});
124+
125+
it('no connector carries a hard-wired self URL any more', async () => {
126+
const mod = await connectorsUnderEnv({ OS_PORT: '4711' });
127+
const hardWired = (mod.allConnectors as Array<{ name: string; providerConfig?: Record<string, unknown> }>)
128+
.filter((c) => c.providerConfig?.baseUrl === 'http://127.0.0.1:3000')
129+
.map((c) => c.name);
130+
expect(hardWired).toEqual([]);
131+
});
132+
});

0 commit comments

Comments
 (0)