tenancy/heartbeat: reportFirstDmSent test.todo at heartbeat.test.ts:131 — propose scalar-locals refactor matching sibling test

[truffle-dev]

What

src/tenancy/__tests__/heartbeat.test.ts:131-141 has a test.todo with a FIXME explaining that the reportFirstDmSent "POSTs to /v1/tenant_status/first_dm_sent..." test fails on Bun 1.3.13 in CI with expect(count).toBe(1) receiving 0. Two refactors during PR #99 (removing the mock() cast, then switching to a globalThis.fetch override) did not fix it.

The structurally-identical sibling test reportAgentReady "POSTs to /v1/tenant_status/agent_ready..." at heartbeat.test.ts:43-88 does pass on the same Bun 1.3.13 CI. The sibling exercises the same postBestEffort code path (src/tenancy/heartbeat.ts:84-104).

Why the sibling probably passes and this one doesn't

The sibling test uses two things together:

1. globalThis.fetch override (the second refactor the FIXME mentions trying).
2. Scalar locals for recording: let count = 0; let lastUrl = ""; let lastBody = ""; etc. No closure-captured arrays.

The FIXME notes "Two refactors did not fix it" — but reads as override-with-array-recording, not override-plus-scalar-locals. Plain reading of the comment text:

Two refactors did not fix it (removing the mock() cast, then switching to a globalThis.fetch override).

The sibling's added discipline is the scalar-locals layout. If the closure-mutation interaction the comment hypothesizes is specifically array-mutation across the push boundary, switching the reportFirstDmSent test to the same scalar-locals layout would close it.

Proposed shape

Replace heartbeat.test.ts:131-141 with the same pattern as the sibling test at heartbeat.test.ts:43-88. The body:

test("POSTs to /v1/tenant_status/first_dm_sent with the slack_message_ts", async () => {
  let count = 0;
  let lastUrl = "";
  let lastBody = "";
  let lastMethod = "";
  let lastContentType = "";

  const originalFetch = globalThis.fetch;
  try {
    globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => {
      count++;
      lastUrl = typeof input === "string" ? input : input.toString();
      lastBody = String(init?.body ?? "");
      lastMethod = String(init?.method ?? "");
      const headers = init?.headers as Record<string, string> | undefined;
      lastContentType = headers?.["content-type"] ?? "";
      return new Response(null, { status: 204 });
    }) as typeof fetch;

    await reportFirstDmSent({
      metadataBaseUrl: "http://169.254.169.254",
      slackMessageTs: "1715000000.000123",
    });

    expect(count).toBe(1);
    expect(lastUrl).toBe("http://169.254.169.254/v1/tenant_status/first_dm_sent");
    expect(lastMethod).toBe("POST");
    expect(lastContentType).toBe("application/json");
    expect(JSON.parse(lastBody)).toEqual({ slack_message_ts: "1715000000.000123" });
  } finally {
    globalThis.fetch = originalFetch;
  }
});

This is the literal byte-for-byte sibling shape with the URL, body, and call swapped.

Local verification

On Bun 1.3.12 the refactored test passes alongside the seven existing passing tests in the file. I ran the proposed body as a standalone test importing reportFirstDmSent directly from src/tenancy/heartbeat.ts. It passes 1/0/0 (1 pass, 0 fail, 0 skip).

I cannot verify Bun 1.3.13 CI from here — the workflow uses oven-sh/setup-bun@v2 with bun-version: latest, so the only way to confirm is to push the change and watch the CI run.

Shapes

1. Try the scalar-locals refactor in a PR, accept that the CI run is the actual verification. If green, merge. If red, close the PR and reopen this issue with the new evidence.
2. Leave the test.todo standing and pin Bun to a known-good version in CI until Bun 1.3.x diagnoses the closure-mutation interaction (likely upstream-bun work, not phantom work).
3. Drop the test entirely and rely on the sibling reportAgentReady test plus the production callers as coverage for postBestEffort. The two functions share postBestEffort so the route-specific assertions on reportFirstDmSent are not strictly required.

I'd lean shape (1). The CI run is cheap, the refactor is mechanical, and if it passes it removes one known-flake reference from the codebase. Happy to push the PR if you'd like; the open-PR queue I have on this repo is already at four (#96, #101, #123, #127), so I'd rather take the signal first.

Repro of the FIXME's pre-refactor failure

If you want to see the original failure mode end-to-end before applying the refactor: revert the sibling reportAgentReady "POSTs to..." test (lines 43-88) to the pre-refactor makeFetchOk() shape (the same calls: RecordedCall[] array that the helper functions still use for the other tests). Push to CI. Bun 1.3.13 should reproduce the expect(count).toBe(1) receiving 0. That confirms the closure-mutation hypothesis is array-shape-specific and the scalar-locals pattern is the right workaround.

Skip the repro if the local-pass evidence above is sufficient.