diff --git a/.changeset/safe-action-stop-transport.md b/.changeset/safe-action-stop-transport.md new file mode 100644 index 00000000000..ca5dde6d57d --- /dev/null +++ b/.changeset/safe-action-stop-transport.md @@ -0,0 +1,5 @@ +--- +"@agent-native/core": patch +--- + +Preserve explicitly safe stopped-action error codes and details across browser action transport. diff --git a/packages/core/src/action.spec.ts b/packages/core/src/action.spec.ts index d7f4ccdaf5d..cc1ae1cc621 100644 --- a/packages/core/src/action.spec.ts +++ b/packages/core/src/action.spec.ts @@ -967,15 +967,17 @@ describe("defineAction — authorize", () => { // AgentActionStopError — the stop-the-turn signal used by actions. // --------------------------------------------------------------------------- describe("AgentActionStopError", () => { - it("carries the stop marker, errorCode, and toolResult", () => { + it("carries the stop marker, safe details, errorCode, and toolResult", () => { const err = new AgentActionStopError("nothing more to do", { errorCode: "DONE", + details: { reason: "complete" }, toolResult: "Stopped.", }); expect(err).toBeInstanceOf(Error); expect(err.name).toBe("AgentActionStopError"); expect(err.agentNativeStop).toBe(true); expect(err.errorCode).toBe("DONE"); + expect(err.details).toEqual({ reason: "complete" }); expect(err.toolResult).toBe("Stopped."); }); diff --git a/packages/core/src/action.ts b/packages/core/src/action.ts index 6e6b8ee01bc..6b140955c4f 100644 --- a/packages/core/src/action.ts +++ b/packages/core/src/action.ts @@ -155,8 +155,10 @@ export type ActionAuthorize = ( ) => void | boolean | Promise; export interface AgentActionStopOptions { - /** Optional stable code surfaced in run metadata and tests. */ + /** Optional stable code safe to surface in run metadata and action transports. */ errorCode?: string; + /** Safe structured context for callers. Never include secrets or raw driver errors. */ + details?: Record; /** Optional short tool-result text. Defaults to the user-facing message. */ toolResult?: string; } @@ -211,12 +213,14 @@ export function isActionContractError( export class AgentActionStopError extends Error { readonly agentNativeStop = true; readonly errorCode?: string; + readonly details?: Record; readonly toolResult?: string; constructor(message: string, options: AgentActionStopOptions = {}) { super(message); this.name = "AgentActionStopError"; this.errorCode = options.errorCode; + this.details = options.details; this.toolResult = options.toolResult; } } diff --git a/packages/core/src/client/use-action.spec.ts b/packages/core/src/client/use-action.spec.ts index ca6fc9eb31c..35dd7cb34a1 100644 --- a/packages/core/src/client/use-action.spec.ts +++ b/packages/core/src/client/use-action.spec.ts @@ -80,6 +80,33 @@ describe("serializeActionQueryParams", () => { }); describe("callAction", () => { + it("preserves safe structured action error metadata", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + jsonResponse( + { + error: "Verification timed out", + errorCode: "workspace_feature_flag_verification_timeout", + details: { phase: "verification-timeout" }, + }, + { status: 500 }, + ), + ), + ); + + const error = await callAction("set-workspace-feature-flag", {}).catch( + (caught) => caught, + ); + + expect(error).toMatchObject({ + status: 500, + errorCode: "workspace_feature_flag_verification_timeout", + details: { phase: "verification-timeout" }, + }); + expect(error.message).toContain("Verification timed out"); + }); + it("sends build compatibility and hard-refreshes once on a mismatch", async () => { Object.assign(globalThis, { __AGENT_NATIVE_BUILD_ID__: "client-build", diff --git a/packages/core/src/client/use-action.ts b/packages/core/src/client/use-action.ts index f718df3bc86..34f412e8160 100644 --- a/packages/core/src/client/use-action.ts +++ b/packages/core/src/client/use-action.ts @@ -487,6 +487,16 @@ async function performActionFetch( const error = new Error(`Action ${name} failed: ${message}`); (error as any).status = res.status; + if (typeof data?.errorCode === "string") { + (error as any).errorCode = data.errorCode; + } + if ( + data?.details && + typeof data.details === "object" && + !Array.isArray(data.details) + ) { + (error as any).details = data.details; + } throw error; } diff --git a/packages/core/src/server/action-routes.spec.ts b/packages/core/src/server/action-routes.spec.ts index 2a6945b8b84..5a86edde30e 100644 --- a/packages/core/src/server/action-routes.spec.ts +++ b/packages/core/src/server/action-routes.spec.ts @@ -384,6 +384,75 @@ describe("mountActionRoutes", () => { }); }); + it("preserves safe action contract metadata for retryable server failures", async () => { + const { ActionContractError } = await import("../action.js"); + const { mountActionRoutes } = await import("./action-routes.js"); + const mounted: Array<{ path: string; handler: any }> = []; + const nitroApp = { + use: vi.fn((path: string, handler: any) => + mounted.push({ path, handler }), + ), + }; + const unavailable = new ActionContractError("Directory unavailable", { + errorCode: "workspace_feature_flag_directory", + details: { phase: "directory" }, + statusCode: 503, + }); + const actions = { + updateItem: { + run: vi.fn().mockRejectedValue(unavailable), + http: { method: "POST" as const }, + }, + }; + mountActionRoutes(nitroApp, actions as any, { + getOwnerFromEvent: async () => "owner@example.com", + }); + const event = { _method: "POST", req: { json: async () => ({}) } }; + const result = await mounted[0].handler(event); + + expect(event._status).toBe(503); + expect(result).toEqual({ + error: "Directory unavailable", + errorCode: "workspace_feature_flag_directory", + details: { phase: "directory" }, + }); + }); + + it("preserves safe stopped-action metadata without exposing its tool result", async () => { + const { AgentActionStopError } = await import("../action.js"); + const { mountActionRoutes } = await import("./action-routes.js"); + const mounted: Array<{ path: string; handler: any }> = []; + const nitroApp = { + use: vi.fn((path: string, handler: any) => + mounted.push({ path, handler }), + ), + }; + const stopped = new AgentActionStopError("Verification timed out", { + errorCode: "workspace_feature_flag_verification_timeout", + details: { phase: "verification-timeout" }, + toolResult: "private agent-only context", + }); + const actions = { + updateItem: { + run: vi.fn().mockRejectedValue(stopped), + http: { method: "POST" as const }, + }, + }; + mountActionRoutes(nitroApp, actions as any, { + getOwnerFromEvent: async () => "owner@example.com", + }); + const event = { _method: "POST", req: { json: async () => ({}) } }; + const result = await mounted[0].handler(event); + + expect(event._status).toBe(500); + expect(result).toEqual({ + error: "Verification timed out", + errorCode: "workspace_feature_flag_verification_timeout", + details: { phase: "verification-timeout" }, + }); + expect(JSON.stringify(result)).not.toContain("private agent-only context"); + }); + it("captures uncategorized action failures with low-cardinality context", async () => { const { mountActionRoutes } = await import("./action-routes.js"); const { registerErrorCaptureProvider } = await import("./capture-error.js"); diff --git a/packages/core/src/server/action-routes.ts b/packages/core/src/server/action-routes.ts index 7fed5706a2d..1856925c2f7 100644 --- a/packages/core/src/server/action-routes.ts +++ b/packages/core/src/server/action-routes.ts @@ -731,6 +731,7 @@ export function mountActionRoutes( // Only echo the raw message for known-safe cases: // - validation errors (deterministic, parameter-shape only) + // - action contract errors (explicitly safe on every transport) // - explicit user-facing errors (AgentActionStopError / fail()) // - errors with an explicit statusCode < 500 (client errors) // For uncategorized 500s, return a generic message and keep the @@ -738,13 +739,16 @@ export function mountActionRoutes( // upstream text we must not leak to HTTP callers. const isUserFacing = isValidationError || + isActionContractError(err) || isAgentActionStopError(err) || (explicitStatus !== undefined && explicitStatus < 500); if (isUserFacing) { - return isActionContractError(err) + return isActionContractError(err) || isAgentActionStopError(err) ? { error: msg, - errorCode: err.errorCode, + ...(typeof err.errorCode === "string" + ? { errorCode: err.errorCode } + : {}), ...(err.details === undefined ? {} : { details: err.details }), diff --git a/plans/shape-cross-app-flag-error-transport.md b/plans/shape-cross-app-flag-error-transport.md new file mode 100644 index 00000000000..a38754f07af --- /dev/null +++ b/plans/shape-cross-app-flag-error-transport.md @@ -0,0 +1,121 @@ +# Cross-app feature flag error transport + +## Answer + +The best repair is a narrow shared Core transport correction, not an Analytics-only parser or a new error policy. + +`AgentActionStopError` already means “stop the agent rather than retry this action,” which is required when a mutation may have persisted. Core should additionally preserve its stable `errorCode` and a new, explicitly safe `details` object across the browser action transport. The agent-only `toolResult` must remain private to the agent runtime. The action route should keep its existing HTTP status behavior, and browser query/mutation retry behavior should not change. + +This is additive and composes the two existing contracts without pretending an uncertain mutation is an ordinary caller-correctable `ActionContractError`. + +## Response-body retry amendment + +The latest review finding does not change the shared Core architecture. It exposes one narrower Analytics bug: `callTarget()` converts every `response.json()` rejection into `{ body: null }`. That is correct for a syntactically invalid or empty legacy response, but wrong when an otherwise successful response body is interrupted or times out. In the verification path, that coercion bypasses the one permitted read retry and collapses a transport failure into generic `verification`. + +The smallest causal repair is to preserve response-body transport failures at the existing `callTarget()` boundary: + +- For a successful HTTP response, an abort or timeout while reading the body remains `timeout`; another non-syntax body-read failure remains `network`. +- A `SyntaxError` from invalid, empty, or non-JSON content continues to produce a null body so existing unsupported/legacy payload classification remains unchanged. +- For a non-success response, the HTTP status remains authoritative even if its optional body cannot be parsed; authorization, unsupported-target, and target-action classification must not be replaced by a body-read transport label. +- `readBackTarget()` may retry that preserved verification transport failure once, issuing a fresh `flags:read` delegated token. It never calls `set-feature-flag`. +- A body-read transport failure on the mutation response is mutation-uncertain: the write is not retried, verification does not begin, and the existing stopped `timeout` or `network` failure is returned. + +This repair belongs only in Analytics' target-call parsing and focused tests. It does not require a new Core error type, retry framework, action status, A2A contract, or Content change. + +## Evidence + +### Demonstrated caller + +- Analytics calls Content's `set-feature-flag`, then independently calls `list-feature-flags` to prove persistence for the designated test operator. +- A live rollback persisted `Off`, but the action returned HTTP 500 after roughly four seconds. A fresh read proved the mutation succeeded, so automatically retrying the mutation would be unsafe. +- The current Work implementation uses `WorkspaceFeatureFlagFailure extends AgentActionStopError`, runs the mutation once, and retries only the verification read once for timeout/network failure. +- Direct review evidence on head `465f5430cee0f59dc32da1007e7c8125b9c3c616` shows that fetch rejection reaches that retry loop, but a rejection from `response.json()` is swallowed and returned as a successful status with a null body. +- The Analytics rollout state must be treated as unknown until an explicit Off mutation and independent read-back prove otherwise. Live operator identity and rollout audit details belong in the access-controlled operational record, not this tracked plan. + +### Existing primitives and intent + +- `AgentActionStopError` carries `errorCode` and `toolResult` and tells the agent runtime to stop the current turn instead of retrying (`packages/core/src/action.ts`). The production agent preserves both fields (`packages/core/src/agent/production-agent.ts`). +- The HTTP action route already treats `AgentActionStopError.message` as safe for the caller, but currently returns only `{ error }` (`packages/core/src/server/action-routes.ts`). +- `ActionContractError` separately declares `errorCode` and `details` safe on every action transport. The server returns those fields, but `actionFetch` currently reconstructs a browser `Error` with only `status`, dropping both fields (`packages/core/src/client/use-action.ts`). This is a pre-existing Core server/client contract gap. +- Repository consumers of `AgentActionStopError` are limited to Core Extensions and Analytics BigQuery, save-analysis, and this workspace flag repair. Their current stable codes are categorical identifiers rather than secrets. Their `toolResult` values can contain richer provider or edit context and must not cross the HTTP boundary. + +### Compatibility and security + +- New server + old client: additive JSON fields are ignored; messages and statuses remain unchanged. +- Old server + new client: fields are absent; the client retains current behavior. +- New server + new client: callers can inspect `error.errorCode` and `error.details`; existing message rendering remains unchanged. +- HTTP status remains 500 for stopped actions. Mutation hooks do not gain automatic retries. Query retry behavior is unchanged. +- Only stable codes and explicitly sanitized details cross the boundary. Ordinary 500s remain generic, and `toolResult` remains agent-only. + +## Inferences + +- Additive response fields are unlikely to break external consumers that parse JSON normally. +- Preserving existing `ActionContractError` fields in the browser is an intended completion of its documented contract, not a new product behavior. +- A shared fix prevents every app from inventing message-prefix parsing or caller-specific rethrows for the same transport gap. + +## Uncertainties + +- Repository search cannot prove that no off-repo consumer asserts exact equality on stopped-action HTTP bodies. This is the residual compatibility risk; additive fields are the conventional compatible evolution. +- No existing type guarantees that arbitrary future `AgentActionStopError.errorCode` values are HTTP-safe. The implementation and JSDoc must establish that stable codes and `details` are caller-safe, while `toolResult` is not. + +## Architecture Constraints + +- **Service owner:** Core owns action error classification, HTTP serialization, and browser reconstruction. Analytics owns workspace flag phases and their sanitized meanings. Content continues to own flag mutation and persistence. +- **Vocabulary:** `errorCode` is a stable machine-readable category; `details` is explicitly sanitized caller context; `toolResult` is agent-only context. +- **Legacy contracts:** ordinary internal 500s stay generic; existing stopped-action messages/statuses and agent stop behavior stay unchanged; mutation remains exactly once; only the verification read may retry once; malformed or empty JSON remains an unsupported/unverified payload rather than a network failure; non-success HTTP status remains authoritative; feature-gate-off behavior remains v2. +- **Smallest compatible delta:** retain the existing Core transport work; in Analytics, stop swallowing successful-response body transport failures, preserve them as the existing timeout/network classifications, and add focused response-body regression tests. No new public type or action contract is needed. +- **Deferred:** a general typed error hierarchy, automatic retry policy based on error codes, transport of `toolResult`, changes to A2A error vocabulary, or migration of existing app errors. +- **Reversibility:** all fields are additive and optional; removing the new serialization returns clients to message-only behavior without data migration. + +## Options + +1. **Shared narrow Core transport repair — recommended.** Corrects the owning seam, preserves stop semantics, and fixes the existing `ActionContractError` browser gap. +2. **Analytics message-prefix parsing.** Avoids Core source changes but creates an app-local transport protocol and leaves the generic Core gap intact. +3. **Hybrid `AgentActionStopError` + `ActionContractError` marker in Analytics.** Makes the current route serialize fields, but misstates an uncertain post-mutation failure as deterministic/caller-correctable and still requires a Core client fix. +4. **Use only `ActionContractError`.** Rejected because the agent may retry a mutation that already persisted. + +## Recommendation + +Approve option 1 with these shipping surfaces: + +- `@agent-native/core` action error type, server action route, browser action client, focused regression tests, and patch changeset; durable destination is the published Core package, integrated by PR merge. +- Analytics workspace feature-flag transaction and focused tests; durable destination is the Analytics template/deployment, integrated by the same PR merge and deployment. + +Acceptance assertions: + +1. The target mutation is issued exactly once. +2. Verification retries at most once and only for timeout/network failure—including an abort, timeout, or interruption while reading an otherwise successful response body—with a fresh delegated read token. +3. Safe `phase` and `errorCode` survive server serialization and browser reconstruction; `toolResult` and arbitrary internal error details do not. +4. Existing `ActionContractError` metadata survives the browser transport. +5. Existing stopped-action message, status, agent-stop behavior, and Core retry behavior remain unchanged. +6. A focused regression proves: mutation response succeeds; the first verification response is HTTP 200 but its body read rejects; a second verification read with fresh authority succeeds; exactly one write and two reads occur. +7. Exhausted verification body-read failures preserve `verification-timeout` or `verification-network`; a mutation-response body-read failure stops without a second write; malformed JSON and non-success statuses retain their prior semantics. +8. Before merge, the app-local `analytics.verified-fleet-flag-mutations` rollout is explicitly set to Off, then `get-feature-flags` freshly proves the evaluated value is `false` and `list-feature-flags` proves the stored rule is Off for the designated test environment. +9. Current CI and independent technical review pass on the exact repair head. The existing repository-owner approval remains sufficient; Alice explicitly waived another exact-head approval on 2026-08-24. +10. After deployment, the full live acceptance remains: enable Content's `content.a2a-receiver-ownership` for the designated test operator, freshly read `Enabled for you`, then roll it back and freshly read `Off`. Failure phases remain distinguishable. Slack and delegated Content canaries remain out of scope until this prerequisite passes. + +Acceptance uses automated Core/Analytics contract tests plus the real beta Analytics interface for rollout rollback and the later enable/read-back/rollback story. Independent review is preferred; same-context custody is allowed because the production mutation is test-operator-scoped, reversible, and feature-gated. + +The subsequent Land may permit a **code-ready-only** merge only when assertions 1–9 are current on the exact head and the risky Analytics rollout is proved Off. Such a merge does not permit re-enablement, a `shipped` claim, or task closure. System-ready status and manual archival still require assertion 10 against the deployed result. + +## Lifecycle state + +**Work active on the approved response-body retry fingerprint.** + +- `authoritySchemaVersion`: 3 +- Previous acceptance fingerprint: verification retries one fetch-level timeout/network failure; live enable/read-back/rollback remains required. +- Proposed acceptance fingerprint: verification retry includes successful-response body transport failure; mutation-response body failure remains write-uncertain and never retries; exact regression, Analytics rollout Off proof, exact-head review/approval, and later deployed acceptance are explicit. +- Approval source: Alice's `$work the shape` instruction in the active task, revised by her 2026-08-24 instruction that the existing Steve approval is sufficient and must not block landing. +- Work authority: Analytics code/tests, focused and repository verification, commit, task-branch push, PR prose refresh, explicit rollback of `analytics.verified-fleet-flag-mutations` for the designated test operator with read-back, and exact-head independent review. Merge, deployment, re-enablement, and task closure remain Land-gated. + +## Sources + +- `packages/core/src/action.ts` +- `packages/core/src/server/action-routes.ts` +- `packages/core/src/client/use-action.ts` +- `packages/core/src/agent/production-agent.ts` +- `packages/core/src/action.spec.ts` +- `packages/core/src/server/action-routes.spec.ts` +- `packages/core/src/client/use-action.spec.ts` +- `templates/analytics/server/lib/workspace-feature-flags.ts` +- Independent bounded review in the current task diff --git a/templates/analytics/server/lib/workspace-feature-flags.spec.ts b/templates/analytics/server/lib/workspace-feature-flags.spec.ts index 15a89b897af..8efe5b9ead7 100644 --- a/templates/analytics/server/lib/workspace-feature-flags.spec.ts +++ b/templates/analytics/server/lib/workspace-feature-flags.spec.ts @@ -1,3 +1,4 @@ +import { isAgentActionStopError } from "@agent-native/core/action"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ @@ -47,6 +48,13 @@ function response(status: number, body: unknown) { } as unknown as Response; } +function responseWithBodyFailure(status: number, error: unknown) { + return { + status, + json: vi.fn().mockRejectedValue(error), + } as unknown as Response; +} + function mutationBody(rules: Record) { return { contractVersion: 2, @@ -60,6 +68,7 @@ function mutationBody(rules: Record) { describe("verified fleet feature flag transaction", () => { beforeEach(() => { vi.restoreAllMocks(); + vi.clearAllMocks(); mocks.fetchOrgApps.mockResolvedValue([app]); mocks.getOrgDomain.mockResolvedValue("example.test"); mocks.isFeatureFlagEnabled.mockResolvedValue(true); @@ -143,6 +152,219 @@ describe("verified fleet feature flag transaction", () => { ); }); + it.each([ + Object.assign(new Error("private timeout"), { name: "TimeoutError" }), + new Error("private network detail"), + ])("retries one transient verification transport failure", async (error) => { + const rules = { + mode: "off", + emails: [], + orgIds: [], + percentage: 0, + }; + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce(response(200, mutationBody(rules))) + .mockRejectedValueOnce(error) + .mockResolvedValueOnce( + response(200, { + contractVersion: 1, + status: "ready", + flags: [{ key: "new-editor", rules, enabledForCurrentUser: false }], + canManage: true, + }), + ); + + await expect( + setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }), + ).resolves.toMatchObject({ + contractVersion: 3, + status: "verified", + enabledForCurrentUser: false, + }); + expect(globalThis.fetch).toHaveBeenCalledTimes(3); + expect(mocks.signA2AToken).toHaveBeenCalledTimes(3); + }); + + it.each([ + Object.assign(new Error("private body timeout"), { name: "TimeoutError" }), + new Error("private interrupted body detail"), + ])( + "retries one transient verification response-body failure", + async (error) => { + const rules = { + mode: "off", + emails: [], + orgIds: [], + percentage: 0, + }; + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(response(200, mutationBody(rules))) + .mockResolvedValueOnce(responseWithBodyFailure(200, error)) + .mockResolvedValueOnce( + response(200, { + contractVersion: 1, + status: "ready", + flags: [{ key: "new-editor", rules, enabledForCurrentUser: false }], + canManage: true, + }), + ); + mocks.signA2AToken + .mockReset() + .mockResolvedValueOnce("mutation-token") + .mockResolvedValueOnce("verification-token-1") + .mockResolvedValueOnce("verification-token-2"); + + await expect( + setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }), + ).resolves.toMatchObject({ + contractVersion: 3, + status: "verified", + enabledForCurrentUser: false, + }); + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(fetchSpy.mock.calls.map(([url]) => String(url))).toEqual([ + "https://mail.example.com/_agent-native/actions/set-feature-flag", + "https://mail.example.com/_agent-native/actions/list-feature-flags", + "https://mail.example.com/_agent-native/actions/list-feature-flags", + ]); + expect( + fetchSpy.mock.calls.map( + ([, options]) => + (options?.headers as Record).Authorization, + ), + ).toEqual([ + "Bearer mutation-token", + "Bearer verification-token-1", + "Bearer verification-token-2", + ]); + expect(mocks.signA2AToken).toHaveBeenCalledTimes(3); + }, + ); + + it.each([ + [ + "verification-timeout", + Object.assign(new Error("private body timeout"), { + name: "TimeoutError", + }), + ], + ["verification-network", new Error("private interrupted body detail")], + ] as const)( + "preserves %s after response-body verification retries are exhausted", + async (phase, error) => { + const rules = { + mode: "off", + emails: [], + orgIds: [], + percentage: 0, + }; + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce(response(200, mutationBody(rules))) + .mockResolvedValueOnce(responseWithBodyFailure(200, error)) + .mockResolvedValueOnce(responseWithBodyFailure(200, error)); + + await expect( + setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }), + ).rejects.toMatchObject({ phase, agentNativeStop: true }); + expect(globalThis.fetch).toHaveBeenCalledTimes(3); + }, + ); + + it("does not retry a mutation whose successful response body is interrupted", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( + responseWithBodyFailure( + 200, + Object.assign(new Error("private body timeout"), { + name: "TimeoutError", + }), + ), + ); + + await expect( + setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }), + ).rejects.toMatchObject({ phase: "timeout", agentNativeStop: true }); + expect(globalThis.fetch).toHaveBeenCalledOnce(); + }); + + it("keeps invalid JSON and non-success statuses on their existing boundaries", async () => { + const input = { + appId: "mail", + key: "new-editor", + operation: "off" as const, + }; + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + responseWithBodyFailure(200, { + name: "SyntaxError", + message: "private cross-realm invalid JSON", + }), + ) + .mockResolvedValueOnce( + responseWithBodyFailure(403, new Error("private interrupted body")), + ); + + await expect(setWorkspaceFeatureFlag(admin, input)).rejects.toMatchObject({ + phase: "persistence", + }); + await expect(setWorkspaceFeatureFlag(admin, input)).rejects.toMatchObject({ + phase: "authorization", + }); + expect(globalThis.fetch).toHaveBeenCalledTimes(2); + }); + + it.each([ + [ + "verification-timeout", + "workspace_feature_flag_verification_timeout", + Object.assign(new Error("private timeout"), { name: "TimeoutError" }), + ], + [ + "verification-network", + "workspace_feature_flag_verification_network", + new Error("private network detail"), + ], + ] as const)( + "fails with the exact %s phase after verification retries are exhausted", + async (phase, errorCode, error) => { + const rules = { + mode: "off", + emails: [], + orgIds: [], + percentage: 0, + }; + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce(response(200, mutationBody(rules))) + .mockRejectedValueOnce(error) + .mockRejectedValueOnce(error); + + await expect( + setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }), + ).rejects.toMatchObject({ phase, errorCode, agentNativeStop: true }); + expect(globalThis.fetch).toHaveBeenCalledTimes(3); + }, + ); + it("accepts replacement rules independently read back for another audience", async () => { const rules = { mode: "rules", @@ -182,13 +404,40 @@ describe("verified fleet feature flag transaction", () => { ], ] as const)("preserves the %s failure boundary", async (phase, arrange) => { await arrange(); - await expect( - setWorkspaceFeatureFlag(admin, { - appId: "mail", - key: "new-editor", - operation: "off", - }), - ).rejects.toMatchObject({ phase }); + const failure = setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }); + await expect(failure).rejects.toMatchObject({ + phase, + errorCode: `workspace_feature_flag_${phase.replace("-", "_")}`, + statusCode: 503, + }); + await expect(failure).rejects.toSatisfy( + (error: unknown) => !isAgentActionStopError(error), + ); + }); + + it("keeps mutation token-signing failures retryable before any request", async () => { + mocks.signA2AToken.mockRejectedValue(new Error("private signing failure")); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + + const failure = setWorkspaceFeatureFlag(admin, { + appId: "mail", + key: "new-editor", + operation: "off", + }); + + await expect(failure).rejects.toMatchObject({ + phase: "token-generation", + errorCode: "workspace_feature_flag_token_generation", + statusCode: 503, + }); + await expect(failure).rejects.toSatisfy( + (error: unknown) => !isAgentActionStopError(error), + ); + expect(fetchSpy).not.toHaveBeenCalled(); }); it.each([ @@ -313,9 +562,17 @@ describe("verified fleet feature flag transaction", () => { }); it("uses safe phase messages without reflecting target details", () => { - expect(new WorkspaceFeatureFlagFailure("network").message).toBe( + const error = new WorkspaceFeatureFlagFailure("network"); + expect(error.message).toBe( "[network] Analytics could not reach the target app.", ); + expect(isAgentActionStopError(error)).toBe(true); + expect(error).toMatchObject({ + details: { phase: "network" }, + errorCode: "workspace_feature_flag_network", + phase: "network", + }); + expect(error.toolResult).not.toContain("private"); }); }); diff --git a/templates/analytics/server/lib/workspace-feature-flags.ts b/templates/analytics/server/lib/workspace-feature-flags.ts index d2c6abd7564..949d3c4bf6f 100644 --- a/templates/analytics/server/lib/workspace-feature-flags.ts +++ b/templates/analytics/server/lib/workspace-feature-flags.ts @@ -1,6 +1,10 @@ import { randomUUID } from "node:crypto"; import { signA2AToken } from "@agent-native/core/a2a"; +import { + ActionContractError, + AgentActionStopError, +} from "@agent-native/core/action"; import { isFeatureFlagEnabled } from "@agent-native/core/feature-flags"; import { fetchOrgApps, type OrgApp } from "@agent-native/core/mcp"; import { getOrgDomain } from "@agent-native/core/org"; @@ -9,6 +13,7 @@ import { VERIFIED_FLEET_FLAG_MUTATIONS } from "../../shared/feature-flags.js"; import type { AnalyticsAdminContext } from "./db-admin-connections.js"; const TARGET_TIMEOUT_MS = 3_000; +const VERIFICATION_ATTEMPTS = 2; const CONCURRENCY = 4; export type FleetFlagState = @@ -197,6 +202,8 @@ export type WorkspaceFeatureFlagFailurePhase = | "unsupported-target" | "target-action" | "persistence" + | "verification-timeout" + | "verification-network" | "verification"; const FAILURE_MESSAGES: Record = { @@ -211,16 +218,42 @@ const FAILURE_MESSAGES: Record = { "target-action": "The target app could not complete the feature flag action.", persistence: "The target app did not persist the requested feature flag rules.", + "verification-timeout": + "The feature flag change persisted, but the target app timed out during verification.", + "verification-network": + "The feature flag change persisted, but Analytics could not reach the target app during verification.", verification: "Analytics could not verify the persisted feature flag change.", }; -export class WorkspaceFeatureFlagFailure extends Error { +export class WorkspaceFeatureFlagFailure extends AgentActionStopError { constructor(readonly phase: WorkspaceFeatureFlagFailurePhase) { - super(`[${phase}] ${FAILURE_MESSAGES[phase]}`); + const message = `[${phase}] ${FAILURE_MESSAGES[phase]}`; + const errorCode = `workspace_feature_flag_${phase.replace("-", "_")}`; + super(message, { + errorCode, + details: { phase }, + toolResult: JSON.stringify({ error: errorCode, phase, message }), + }); this.name = "WorkspaceFeatureFlagFailure"; } } +class WorkspaceFeatureFlagSetupFailure extends ActionContractError { + readonly phase: "directory" | "token-generation"; + + constructor(phase: "directory" | "token-generation") { + const message = `[${phase}] ${FAILURE_MESSAGES[phase]}`; + const errorCode = `workspace_feature_flag_${phase.replace("-", "_")}`; + super(message, { + errorCode, + details: { phase }, + statusCode: 503, + }); + this.name = "WorkspaceFeatureFlagSetupFailure"; + this.phase = phase; + } +} + type TargetFailureReason = "token-generation" | "timeout" | "network"; class TargetCallFailure extends Error { @@ -242,6 +275,16 @@ export function classifyWorkspaceFeatureFlagTargetFailure( return "network"; } +function isSyntaxError(error: unknown): boolean { + return ( + error instanceof SyntaxError || + (!!error && + typeof error === "object" && + "name" in error && + error.name === "SyntaxError") + ); +} + function targetFailure(error: unknown): WorkspaceFeatureFlagFailure { const reason = classifyWorkspaceFeatureFlagTargetFailure(error); return new WorkspaceFeatureFlagFailure(reason); @@ -259,10 +302,10 @@ async function resolveTargetApp( serviceOrgId: admin.orgId, }); } catch { - throw new WorkspaceFeatureFlagFailure("directory"); + throw new WorkspaceFeatureFlagSetupFailure("directory"); } const app = apps.find((candidate) => candidate.id === appId); - if (!app) throw new WorkspaceFeatureFlagFailure("directory"); + if (!app) throw new WorkspaceFeatureFlagSetupFailure("directory"); return app; } @@ -302,12 +345,41 @@ async function callTarget( let parsed: unknown = null; try { parsed = await response.json(); - } catch { - // A legacy/non-action endpoint is classified below without reflecting body. + } catch (error) { + const successfulResponse = response.status >= 200 && response.status < 300; + if (successfulResponse && !isSyntaxError(error)) + throw new TargetCallFailure( + classifyWorkspaceFeatureFlagTargetFailure(error), + ); + // Invalid legacy payloads and non-success statuses are classified below. } return { status: response.status, body: parsed }; } +async function readBackTarget( + app: OrgApp, + admin: AnalyticsAdminContext, + orgDomain: string, +): Promise<{ status: number; body: unknown }> { + for (let attempt = 1; attempt <= VERIFICATION_ATTEMPTS; attempt += 1) { + try { + return await callTarget(app, admin, "list-feature-flags", {}, orgDomain); + } catch (error) { + const reason = classifyWorkspaceFeatureFlagTargetFailure(error); + const retryable = reason === "timeout" || reason === "network"; + if (retryable && attempt < VERIFICATION_ATTEMPTS) continue; + throw new WorkspaceFeatureFlagFailure( + reason === "timeout" + ? "verification-timeout" + : reason === "network" + ? "verification-network" + : reason, + ); + } + } + throw new WorkspaceFeatureFlagFailure("verification"); +} + export function classifyWorkspaceFeatureFlagList( app: OrgApp, result: { status: number; body: unknown }, @@ -404,9 +476,10 @@ export async function setWorkspaceFeatureFlag( try { orgDomain = (await getOrgDomain(admin.orgId))?.trim().toLowerCase(); } catch { - throw new WorkspaceFeatureFlagFailure("token-generation"); + throw new WorkspaceFeatureFlagSetupFailure("token-generation"); } - if (!orgDomain) throw new WorkspaceFeatureFlagFailure("token-generation"); + if (!orgDomain) + throw new WorkspaceFeatureFlagSetupFailure("token-generation"); let result: Awaited>; try { result = await callTarget( @@ -417,6 +490,8 @@ export async function setWorkspaceFeatureFlag( orgDomain, ); } catch (error) { + if (classifyWorkspaceFeatureFlagTargetFailure(error) === "token-generation") + throw new WorkspaceFeatureFlagSetupFailure("token-generation"); throw targetFailure(error); } if (result.status === 401 || result.status === 403) @@ -465,18 +540,7 @@ export async function setWorkspaceFeatureFlag( return mutation; } - let readBack: Awaited>; - try { - readBack = await callTarget( - app, - admin, - "list-feature-flags", - {}, - orgDomain, - ); - } catch (error) { - throw targetFailure(error); - } + const readBack = await readBackTarget(app, admin, orgDomain); if (readBack.status === 401 || readBack.status === 403) throw new WorkspaceFeatureFlagFailure("authorization"); if (readBack.status === 404 || readBack.status === 405)