Skip to content

Fallback to deployment manifest when workspace registry denies read #5039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
builder-io-integration wants to merge 5 commits into from
+252 −26
Closed

Fallback to deployment manifest when workspace registry denies read #5039

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c9d11f7
fix(dispatch): keep the Apps page readable when the registry denies a…
Sep 15, 2026
9c2269d
Merge remote-tracking branch 'refs/remotes/origin/main' into ai_main_…
builderio-bot Sep 15, 2026
ae19172
fix(dispatch): never mutate registry state from an unverified fallback
Sep 15, 2026
f703a0d
fix(dispatch): report the apps an unverified registry read cannot res…
Sep 15, 2026
038ae7f
test(dispatch): keep SQL capture live across the whole registry spec
Sep 15, 2026
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 .changeset/dispatch-registry-denial-fallback.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@agent-native/dispatch": patch
---

Keep the Apps page readable when the hosted workspace registry denies a read. A gateway 401/403 now falls back to the deployment-owned app manifest, which is still access-filtered per caller, and only throws when no source can answer the registry at all.
160 changes: 128 additions & 32 deletions packages/dispatch/src/server/lib/app-creation-store.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
startWorkspaceAppCreation,
updateWorkspaceAppMetadata,
} from "./app-creation-store.js";
import { listCuratedWorkspaceTemplates } from "./curated-workspace-templates.js";

const originalFetch = globalThis.fetch;
const settingsKey = "dispatch-app-creation-settings:user:dev@example.test";
Expand All @@ -24,9 +25,28 @@ const mocks = vi.hoisted(() => {
const state = {
orgRole: "admin" as string | null,
};
const executedSql: string[] = [];
const defaultDbExec = () => ({
execute: vi.fn(async (statement: unknown) => {
const sql =
typeof statement === "string"
? statement
: String((statement as { sql?: unknown })?.sql ?? "");
executedSql.push(sql);
if (sql.includes("SELECT id FROM workspace_apps")) {
return { rows: [], rowsAffected: 0 };
}
return {
rows: state.orgRole ? [{ role: state.orgRole }] : [],
rowsAffected: 0,
};
}),
});
return {
settings,
state,
executedSql,
defaultDbExec,
getSetting: vi.fn(async (key: string) => settings.get(key) ?? null),
mutateSetting: vi.fn(
async (key: string, updater: (current: any) => any) => {
Expand All @@ -44,21 +64,7 @@ const mocks = vi.hoisted(() => {
role: "viewer",
resource: {},
})),
getDbExec: vi.fn(() => ({
execute: vi.fn(async (statement: unknown) => {
const sql =
typeof statement === "string"
? statement
: String((statement as { sql?: unknown })?.sql ?? "");
if (sql.includes("SELECT id FROM workspace_apps")) {
return { rows: [], rowsAffected: 0 };
}
return {
rows: state.orgRole ? [{ role: state.orgRole }] : [],
rowsAffected: 0,
};
}),
})),
getDbExec: vi.fn(defaultDbExec),
resolveBuilderCredentialsDetailed: vi.fn(async () => ({
privateKey: null as string | null,
publicKey: null as string | null,
Expand Down Expand Up @@ -156,6 +162,7 @@ afterEach(() => {
vi.unstubAllEnvs();
vi.unstubAllGlobals();
vi.clearAllMocks();
mocks.executedSql.length = 0;
mocks.settings.clear();
mocks.getOrgSetting.mockReset();
mocks.getOrgSetting.mockResolvedValue(null);
Expand All @@ -171,21 +178,7 @@ afterEach(() => {
);
mocks.state.orgRole = "admin";
mocks.getDbExec.mockReset();
mocks.getDbExec.mockImplementation(() => ({
execute: vi.fn(async (statement: unknown) => {
const sql =
typeof statement === "string"
? statement
: String((statement as { sql?: unknown })?.sql ?? "");
if (sql.includes("SELECT id FROM workspace_apps")) {
return { rows: [], rowsAffected: 0 };
}
return {
rows: mocks.state.orgRole ? [{ role: mocks.state.orgRole }] : [],
rowsAffected: 0,
};
}),
}));
mocks.getDbExec.mockImplementation(mocks.defaultDbExec);
mocks.resolveAccess.mockReset();
mocks.resolveAccess.mockResolvedValue({ role: "viewer", resource: {} });
mocks.resolveBuilderCredentialsDetailed.mockResolvedValue({
Expand Down Expand Up @@ -424,28 +417,131 @@ describe("listWorkspaceApps", () => {
});

it.each([401, 403])(
"surfaces hosted registry authorization failures instead of using local manifests (%i)",
"serves the deployment manifest when the hosted registry denies the read (%i)",
async (status) => {
const fetchMock = vi.fn(async () => new Response("denied", { status }));
vi.stubGlobal("fetch", fetchMock);
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
vi.stubEnv("A2A_SECRET", "test-a2a-secret");
vi.stubEnv("WORKSPACE_GATEWAY_URL", "https://agent-workspace.builder.io");
stubManifest([
{ id: "dispatch", name: "Dispatch", path: "/dispatch" },
{ id: "clips", name: "Clips", path: "/clips" },
]);

const apps = await runWithRequestContext(
{ userEmail: "dev@example.test" },
() => listWorkspaceApps({ includeAgentCards: false }),
);

expect(apps.map((app) => app.id)).toEqual(["dispatch", "clips"]);
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(warn).toHaveBeenCalledWith(
expect.stringContaining(
`workspace apps gateway denied the registry read with HTTP ${status}`,
),
);
warn.mockRestore();
},
);

// A read the caller could not authenticate must not write the access rows it
// is then filtered by, and must not delete rows or shares the denied
// registry never confirmed are gone.
it("never mutates registry state from the unverified fallback manifest", async () => {
const fetchMock = vi.fn(
async () => new Response("denied", { status: 403 }),
);
vi.stubGlobal("fetch", fetchMock);
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
vi.stubEnv("A2A_SECRET", "test-a2a-secret");
vi.stubEnv("WORKSPACE_GATEWAY_URL", "https://agent-workspace.builder.io");
stubManifest([
{ id: "dispatch", name: "Dispatch", path: "/dispatch" },
{ id: "clips", name: "Clips", path: "/clips" },
]);

await runWithRequestContext(
{ userEmail: "dev@example.test", orgId: "builder_io" },
() => listWorkspaceApps({ includeAgentCards: false }),
);

const mutations = mocks.executedSql.filter((sql) =>
Comment thread
builder-io-integration[bot] marked this conversation as resolved.
/\b(INSERT|UPDATE|DELETE)\b/i.test(sql),
);
expect(mutations).toEqual([]);
warn.mockRestore();
});

it("still reconciles registry state when the gateway is merely unavailable", async () => {
const fetchMock = vi.fn(
async () => new Response("not found", { status: 404 }),
);
vi.stubGlobal("fetch", fetchMock);
vi.stubEnv("A2A_SECRET", "test-a2a-secret");
vi.stubEnv("WORKSPACE_GATEWAY_URL", "https://agent-workspace.builder.io");
stubManifest([
{ id: "dispatch", name: "Dispatch", path: "/dispatch" },
{ id: "clips", name: "Clips", path: "/clips" },
]);

await runWithRequestContext(
{ userEmail: "dev@example.test", orgId: "builder_io" },
() => listWorkspaceApps({ includeAgentCards: false }),
);

expect(mocks.executedSql.some((sql) => /\bINSERT\b/i.test(sql))).toBe(true);
});

it.each([401, 403])(
"still rejects a denied registry read when no deployment manifest can answer (%i)",
async (status) => {
const fetchMock = vi.fn(async () => new Response("denied", { status }));
vi.stubGlobal("fetch", fetchMock);
vi.stubEnv("A2A_SECRET", "test-a2a-secret");
vi.stubEnv("WORKSPACE_GATEWAY_URL", "https://agent-workspace.builder.io");
vi.stubEnv("AGENT_NATIVE_WORKSPACE_APPS_JSON", "");

await expect(
runWithRequestContext({ userEmail: "dev@example.test" }, () =>
listWorkspaceApps({ includeAgentCards: false }),
),
).rejects.toThrow(
`Workspace apps gateway rejected the request with HTTP ${status}.`,
);
expect(fetchMock).toHaveBeenCalledTimes(1);
},
);

// The Apps page rendered two "Couldn't load data" cards from one failure:
// the curated catalog reads the same registry only to mark apps installed.
it("keeps the curated template catalog readable when the registry denies the read", async () => {
const fetchMock = vi.fn(
async () => new Response("denied", { status: 403 }),
);
vi.stubGlobal("fetch", fetchMock);
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
vi.stubEnv("A2A_SECRET", "test-a2a-secret");
vi.stubEnv("WORKSPACE_GATEWAY_URL", "https://agent-workspace.builder.io");
stubManifest([
{ id: "dispatch", name: "Dispatch", path: "/dispatch" },
{ id: "mail", name: "Mail", path: "/mail" },
]);

const templates = await runWithRequestContext(
{ userEmail: "dev@example.test" },
() => listCuratedWorkspaceTemplates(),
);

expect(templates.length).toBeGreaterThan(0);
expect(
templates.find((template) => template.id === "mail")?.installed,
).toBe(true);
expect(
templates.find((template) => template.id === "calendar")?.installed,
).toBe(false);
warn.mockRestore();
});

it("falls back to local manifests when the hosted registry route is missing", async () => {
const fetchMock = vi.fn(
async () => new Response("not found", { status: 404 }),
Expand Down
79 changes: 70 additions & 9 deletions packages/dispatch/src/server/lib/app-creation-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,20 @@
statusCode: 401 | 403;
}

/**
* A denial that a fallback source answered is still a misconfiguration worth
* diagnosing, so it must stay visible in logs even though the read succeeded.
*/
function warnWorkspaceAppsGatewayDenial(
denial: WorkspaceAppsGatewayAuthorizationError | null,
source: string,
): void {
if (!denial) return;
console.warn(
`[dispatch] workspace apps gateway denied the registry read with HTTP ${denial.statusCode}; served the ${source} instead`,
);
}

type WorkspaceAppAudience = "internal" | "public";
type WorkspaceAppVisibility = "private" | "org";

Expand Down Expand Up @@ -150,6 +164,13 @@
workspaceSso?: boolean;
}

interface FinalizeWorkspaceAppsOptions {
/** Delete org rows absent from an authoritative manifest. */
reconcile?: boolean;
/** Write registry rows. False for a source the caller could not authenticate. */
persist?: boolean;
}

interface WorkspaceAppDiscovery {
apps: WorkspaceAppSummary[];
authoritative: boolean;
Expand Down Expand Up @@ -1252,12 +1273,17 @@
*/
async function ensureWorkspaceAppRecords(
apps: WorkspaceAppSummary[],
options: { reconcile?: boolean } = {},
options: { reconcile?: boolean; persist?: boolean } = {},
): Promise<WorkspaceAppSummary[]> {
const readyApps = apps.filter(
(app) => app.status !== "pending" && !app.isDispatch,
);
const shouldReconcile = options.reconcile === true;
// A source the caller could not authenticate annotates from existing rows
// only. Minting a row here would create the very authorization the access
// filter then checks, and reconciling would delete rows and shares on the
// word of a manifest no authoritative registry confirmed.
const shouldPersist = options.persist !== false;
const shouldReconcile = options.reconcile === true && shouldPersist;
if (!shouldReconcile && readyApps.length === 0) return apps;

const orgId = currentOrgId();
Expand Down Expand Up @@ -1303,6 +1329,7 @@
for (const app of readyApps) {
const existing = existingRecords.get(app.id);
if (!existing) {
if (!shouldPersist) continue;
const override = metadata.apps[app.id];
// Never infer ownership from the person who happened to list apps.
// Legacy manifests without trusted creation metadata remain
Expand Down Expand Up @@ -1340,7 +1367,7 @@
const existingOrgId = cleanOptionalText(existing.org_id) ?? null;
// A registry row belongs to the org that created it. Never reassign a
// row from another org just because a caller listed the same manifest.
if (existingOrgId && existingOrgId !== orgId) {
if (!shouldPersist || (existingOrgId && existingOrgId !== orgId)) {
records.set(app.id, {
ownerEmail: existingOwnerEmail,
orgId: existingOrgId,
Expand Down Expand Up @@ -1976,11 +2003,17 @@
export async function listWorkspaceApps(
options: ListWorkspaceAppsOptions = {},
): Promise<WorkspaceAppSummary[]> {
const finalize = async (apps: WorkspaceAppSummary[], reconcile = false) => {
const finalize = async (
apps: WorkspaceAppSummary[],
{ reconcile = false, persist = true }: FinalizeWorkspaceAppsOptions = {},
) => {
// Reconcile from the complete manifest. Archive and audience filters only
// control the response; treating hidden apps as absent deletes their rows.
const annotated = await applyArchivedAndPending(apps);
const recorded = await ensureWorkspaceAppRecords(annotated, { reconcile });
const recorded = await ensureWorkspaceAppRecords(annotated, {
reconcile,
persist,
});
const listed = options.includeArchived
? recorded
: recorded.filter((app) => !app.archived);
Expand All @@ -1989,26 +2022,54 @@
);
return maybeIncludeAgentCards(visible, options);
};
const gatewayApps = await readWorkspaceAppsFromGateway();
let gatewayDenial: WorkspaceAppsGatewayAuthorizationError | null = null;
let gatewayApps: WorkspaceAppDiscovery | null = null;
try {
gatewayApps = await readWorkspaceAppsFromGateway();
} catch (error) {
if (!(error instanceof WorkspaceAppsGatewayAuthorizationError)) throw error;
// A denial answers for the gateway hop, not for what this caller may see.

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Classify local gateway authorization failures as denials

The local gateway branch only handles localResponse.ok; a local 401/403 falls through and returns null, so gatewayDenial is never set. The caller then treats the deployment manifest as authoritative and may persist/reconcile rows despite an explicit authorization failure. Propagate local 401/403 through WorkspaceAppsGatewayAuthorizationError, while retaining trusted fallback behavior for unavailable/404 responses.

Additional Info 
Found by 1 of 2 parallel reviewers; verified in readWorkspaceAppsFromGateway local branch at lines 1747-1765.

Fix in Builder

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required — not fixing. The mechanics are as you describe: a local 401/403 falls through, and with no Authorization header or a same-origin gateway the function returns null, so the manifest is treated as authoritative. But I don't think the denial downgrade should extend there.

That branch is gated on isLocalWorkspaceGateway, which is loopback only — localhost, 127.0.0.1, 0.0.0.0, ::1. The reason a denial downgrades trust in the hosted path is specifically a cross-deployment trust gap: the receiver resolves org membership against its own database, so its refusal tells us nothing reliable about this caller. A loopback gateway is the developer's own dev server in the same process group; there is no tenant boundary being crossed and nothing to be misled about. The local branch is also normally called unauthenticated, so a 401 there means the dev gateway is misconfigured rather than that a trust relationship failed.

The change would also carry a real cost. Downgrading to persist: false in local dev means registry rows never get created, filterWorkspaceAppsByAccess then drops every app, and the local Apps page goes empty — reintroducing exactly the symptom this PR exists to fix, on the one path where it is most likely to be hit by accident.

This behaviour is also unchanged by this PR; the local branch predates it. If we do want loopback denials classified, it deserves its own change with local-dev verification rather than riding along here.

// The receiver resolves org membership against its own database, so a
// cross-deployment trust gap reads as 403 for every user at once. Serve
// the deployment manifests below rather than blanking the workspace, but
// treat them as unverified: read-only, so an unauthenticated read never
// writes the access rows it is about to be filtered by. A registry
// nothing can answer still throws.
gatewayDenial = error;
}
if (gatewayApps) {
return finalize(gatewayApps.apps, gatewayApps.authoritative);
return finalize(gatewayApps.apps, { reconcile: gatewayApps.authoritative });
}
const unverified = gatewayDenial !== null;

const workspaceRoot = findWorkspaceRoot();
const localFilesystemApps =
workspaceRoot && isLocalAppCreationRuntime()
? readWorkspaceAppsFromFilesystem(workspaceRoot)
: null;
if (localFilesystemApps) {
return finalize(localFilesystemApps, true);
warnWorkspaceAppsGatewayDenial(gatewayDenial, "local filesystem");
return finalize(localFilesystemApps, {
reconcile: !unverified,
persist: !unverified,
Comment on lines +2065 to +2067

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Preserve visibility for manifest apps without existing registry rows

When a gateway 401/403 selects this read-only fallback, persist: false skips creating rows for manifest apps, but filterWorkspaceAppsByAccess() still resolves access through workspace-app records. For a newly deployed app with no existing row, that lookup returns no access and filters the app out, so the fallback can show only Dispatch and fail to keep the Apps page readable. Add a read-only access projection/decision for manifest entries (without minting rows), and cover the absent-row case with resolveAccess returning null.

Additional Info 
Found by 1 of 2 parallel reviewers; the other found no new issues. Focused app-creation-store.spec.ts run reported 60/60 passing, but its resolveAccess mock grants all candidates and does not exercise absent rows.

Fix in Builder

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required — not fixing the visibility, fixed the silence.

I don't agree with restoring visibility here. The two options that would do it are both worse than the current behaviour:

  • Mint the row — that is exactly the circular authorization the 🔴 thread above asked me to remove. We would be creating the access record that the access filter then accepts.
  • Exempt rowless apps from the access check — that grants visibility with no access record at all, for precisely the apps we know least about, on a read we could not authenticate.

Dropping them is the fail-closed answer, and it matches what filterWorkspaceAppsByAccess already does when a per-app lookup fails (allowed: false). Scope is also narrower than it reads: this only affects an app deployed since the last successful registry read, during an active denial. Every app with an existing row still renders, which is the reported beta case.

Where you're right is the silence. Access for those apps is unknown, not denied, and dropping them without a word is the same coercion this PR is fixing in the other direction. ensureWorkspaceAppRecords now collects them and logs the count and the ids, so an operator can tell a benign degraded read from one hiding the whole workspace. Covered by names the apps an unverified read cannot resolve access for, mutation-checked.

Good catch on the mock, too — resolveAccess granting every candidate did mean the focused run never exercised absent rows. The new assertions key off the warning rather than the access filter, so they don't depend on that mock. While fixing this I also found the spec's afterEach was re-installing a second, drifted copy of the database mock; that's now a single shared factory.

});
}

const manifestApps =
readWorkspaceAppsFromEnv() ?? readWorkspaceAppsFromManifestFile();
if (manifestApps) {
return finalize(manifestApps, true);
warnWorkspaceAppsGatewayDenial(gatewayDenial, "deployment manifest");
return finalize(manifestApps, {
reconcile: !unverified,
persist: !unverified,
Comment on lines +2074 to +2077

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Do not reconcile manifests after transient gateway failures

readWorkspaceAppsFromGateway() returns null for non-401/403 failures such as 5xx, 429, timeouts, and malformed responses. This branch then treats gatewayDenial === null as authoritative and calls finalize with persistence and reconciliation enabled, so a temporary registry outage can make a stale manifest insert/update records or delete org-owned rows and shares. Distinguish route absence from operational failure and keep outage fallbacks read-only or fail closed; add coverage for 500/429/timeout cases.

Additional Info 
Found by 1 of 2 parallel reviewers; focused suite reported 61 passing tests.

Fix in Builder

@builder-io-integration builder-io-integration Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required — not fixing here, but I agree it's a real gap and it should be the follow-up.

You're right on the mechanics, and right that the line I drew is not the fully principled one. The honest taxonomy is:

  • 404 / no gateway configured — the registry genuinely is not there, so the deployment manifest is the authority and reconciling is correct.
  • 401/403 — the registry exists and refused us.
  • 5xx, 429, timeout, malformed body — the registry exists and failed to answer us.

The last two are the same epistemic state: we have no authoritative list, so deleting rows and shares on the manifest's word is unjustified in both. I'm not going to argue otherwise.

The reason I'm not extending it in this PR is scope and shape, not disagreement. readWorkspaceAppsFromGateway() collapses seven distinct outcomes into a single null — unset URL, malformed URL, local non-ok, same-origin self-fetch, missing authorization, 404, 5xx/throw. Telling "never configured" apart from "configured and failed" means turning that into a discriminated return and re-testing each exit, and every one of those paths needs real coverage rather than the vacuous kind this spec just taught me to distrust. That is a bigger change than I want riding on a hotfix for a regression that is breaking beta right now (#5009, 4.5 hours between merge and the user report).

The 401/403 case was in scope precisely because it is the one this PR introduced. The transient-failure case is pre-existing on main and unchanged by this diff, so landing this does not make it worse.

Happy to open the follow-up for the discriminated-outcome refactor if you'd like it tracked.

});
}

// Every remaining branch synthesizes a registry instead of reading one, so a
// denial must stay a denial rather than become an empty or Dispatch-only
// workspace the caller cannot tell apart from a real answer.
if (gatewayDenial) throw gatewayDenial;

if (!workspaceRoot) {
return finalize([
{
Expand Down Expand Up @@ -2480,7 +2541,7 @@
throw new Error(`Builder app creation returned a blank ${fieldName}`);
}
const trimmed = value.trim();
if (/[\u0000-\u001f\u007f]/.test(trimmed)) {

Check warning on line 2544 in packages/dispatch/src/server/lib/app-creation-store.ts

View workflow job for this annotation

GitHub Actions / Lint & format

eslint(no-control-regex) 

Unexpected control characters
throw new Error(`Builder app creation returned a malformed ${fieldName}`);
}
return trimmed;
Expand Down
Loading