From 86675ee89c6ee67e57e1560c10d6bcfa61f13c8e Mon Sep 17 00:00:00 2001 From: Release Engineer Date: Wed, 23 Sep 2026 02:25:22 +0000 Subject: [PATCH] test(agents): share the two log-access action strings, so an absence guard cannot pass vacuously (BLO-35509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BLO-34901 widened both log-less-404 absence assertions from `result: "allowed"` to any result, and pinned the exercised path with a `readLog` positive. What neither change touched is the guard's SELECTOR: each of the seven assertions in this file carried its own inline copy of the action string, so a typo confined to one of the two `not.toHaveBeenCalledWith` guards made that guard true by construction — nobody emits the typo'd action — while the five positives kept their own correct copies and kept passing. Reproduced on master before the change, one site at a time: - typo :637 alone -> 35/35 pass - typo :780 alone -> 35/35 pass Declaring both strings once and referencing them from all seven sites makes that shape unreachable: a typo at a call site is now an undefined identifier, caught by `pnpm check:test-undefined-symbols` (TS2552) and by vitest itself (ReferenceError, 1 failed | 34 passed). A typo in the const VALUE breaks the five positives instead of hiding in a guard. Test-only; no change under server/src/routes or server/src/services. Co-Authored-By: Claude --- .../__tests__/agent-live-run-routes.test.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index d40298c09bf6..5520d3303aa2 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -55,6 +55,17 @@ const routeAgentId = "11111111-1111-4111-8111-111111111111"; const WORKSPACE_OPERATION_LOG_SENTINEL = "TOKEN_FIXTURE=sentinel-operation-log-not-a-real-credential ./deploy.sh"; +/** + * BLO-35509. Source of truth: `server/src/routes/agents.ts` (`logRunLogAccessAudit`). Declared + * once because two of the uses below are `not.toHaveBeenCalledWith` absence guards, and an + * absence guard keyed on a literal nobody emits passes vacuously — a typo confined to that one + * site is silent, since the positives hold their own copies and keep passing. Verified on + * `master`: typo `:637` or `:780` alone and all 35 still pass. Sharing the const makes that + * shape a reference error instead. + */ +const HEARTBEAT_RUN_LOG_ACCESSED = "heartbeat.run_log_accessed"; +const WORKSPACE_OPERATION_LOG_ACCESSED = "workspace_operation.log_accessed"; + function workspaceOperationLogFixture(overrides: Record = {}) { return { id: "operation-1", @@ -549,7 +560,7 @@ describe("agent live run routes", () => { actorType: "user", actorId: "local-board", agentId: null, - action: "heartbeat.run_log_accessed", + action: HEARTBEAT_RUN_LOG_ACCESSED, entityType: "heartbeat_run", entityId: "run-1", runId: "run-1", @@ -588,7 +599,7 @@ describe("agent live run routes", () => { actorType: "agent", actorId: routeAgentId, agentId: routeAgentId, - action: "heartbeat.run_log_accessed", + action: HEARTBEAT_RUN_LOG_ACCESSED, entityType: "heartbeat_run", entityId: "run-1", runId: "run-1", @@ -634,7 +645,7 @@ describe("agent live run routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(404); expect(mockHeartbeatService.readLog).toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ - action: "heartbeat.run_log_accessed", + action: HEARTBEAT_RUN_LOG_ACCESSED, })); }); @@ -669,7 +680,7 @@ describe("agent live run routes", () => { // disclosed. Without `withheld` the record is indistinguishable from a real disclosure, and // "who read this log" over-reports. expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ - action: "workspace_operation.log_accessed", + action: WORKSPACE_OPERATION_LOG_ACCESSED, details: expect.objectContaining({ result: "allowed", withheld: true }), })); }); @@ -690,7 +701,7 @@ describe("agent live run routes", () => { companyId: "company-1", actorType: "user", actorId: "local-board", - action: "workspace_operation.log_accessed", + action: WORKSPACE_OPERATION_LOG_ACCESSED, entityType: "workspace_operation", entityId: "operation-1", // The operation's own run, so an audit reader can join back to the run that produced it. @@ -730,7 +741,7 @@ describe("agent live run routes", () => { actorType: "agent", actorId: routeAgentId, agentId: routeAgentId, - action: "workspace_operation.log_accessed", + action: WORKSPACE_OPERATION_LOG_ACCESSED, entityType: "workspace_operation", entityId: "operation-1", details: expect.objectContaining({ @@ -777,7 +788,7 @@ describe("agent live run routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(404); expect(mockWorkspaceOperationService.readLog).toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ - action: "workspace_operation.log_accessed", + action: WORKSPACE_OPERATION_LOG_ACCESSED, })); });