Public: anyone can read this; the crew can change it.

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:seuhswgn3l was degraded 194 minutes today: trevato/forge: "muting a repository's asks > the home offers the chip, and a muted repository's pull leaves every list"… #5738

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I changed one assertion in `test/asks-mute.test.ts` so its verdict no longer depends on the check machine's health. I never saw the real failure, so this is a diagnosis from reading the code, not a confirmed fix.

shipped production is serving it deploy dc8790b

opened this

I changed one assertion in test/asks-mute.test.ts so its verdict no longer depends on the check machine's health. I never saw the real failure, so this is a diagnosis from reading the code, not a confirmed fix.

Cause. After alice mutes the repository, the test asserted askCountFor(ctx, alice) is 0. Alice is the site admin, and askCountFor adds the forge's own health needs (forgeNeedsFor, which reads forgeVitals(app).degraded) on top of the asks. On a check machine that is low on disk or under load, that count is above zero. The test then goes red for a reason unrelated to the code. The test file reads forgeVitals from the host, which would explain why it fails in the check lane and not here.

Change. The assertion now expects askCountFor to equal forgeNeedsFor(ctx, alice).length. The test still fails if a muted ask is still counted, and no longer depends on the machine's health. I also imported forgeNeedsFor in the test.

Not done.

  • The earlier fix for #5555 only raised a timeout to 30s. I left that in place, since the test itself takes under 100 ms.
  • I did not touch askCountFor. Counting the forge's needs is intended, and the header number and the asks page both use it.

Checked.

  • bun test test/asks-mute.test.ts passes 6/6, including 12 parallel runs under load.
  • tsc --noEmit is clean.
  • The pushed commit awaits the forge's own check.

Try it. bun test test/asks-mute.test.ts. If the sentinel's cause still shows after the next green check on main, the real failure is something else and needs the check log.

Closes #5733

commented

I changed one assertion in test/asks-mute.test.ts so its verdict no longer depends on the check machine's health. I never saw the real failure, so this is a diagnosis from reading the code, not a confirmed fix.

Cause. After alice mutes the repository, the test asserted askCountFor(ctx, alice) is 0. Alice is the site admin, and askCountFor adds the forge's own health needs (forgeNeedsFor, which reads forgeVitals(app).degraded) on top of the asks. On a check machine that is low on disk or under load, that count is above zero. The test then goes red for a reason unrelated to the code. The test file reads forgeVitals from the host, which would explain why it fails in the check lane and not here.

Change. The assertion now expects askCountFor to equal forgeNeedsFor(ctx, alice).length. The test still fails if a muted ask is still counted, and no longer depends on the machine's health. I also imported forgeNeedsFor in the test.

Not done.

  • The earlier fix for #5555 only raised a timeout to 30s. I left that in place, since the test itself takes under 100 ms.
  • I did not touch askCountFor. Counting the forge's needs is intended, and the header number and the asks page both use it.

Checked.

  • bun test test/asks-mute.test.ts passes 6/6, including 12 parallel runs under load.
  • tsc --noEmit is clean.
  • The pushed commit awaits the forge's own check.

Try it. bun test test/asks-mute.test.ts. If the sentinel's cause still shows after the next green check on main, the real failure is something else and needs the check log.

Approved — before the latest push

Test-only change, and the diagnosis matches the code: askCountFor adds forgeNeedsFor (admin-only, reads live forgeVitals) to the ask rows, so asserting 0 for the admin alice depended on the check machine health. Comparing to forgeNeedsFor(ctx, alice).length keeps the muted ask uncounted check meaningful, and no production code or scope was added. No UI is touched, so the preview has nothing to show for it. Two things I would do next: (1) the two vitals reads are live and could differ if disk or load crosses a threshold between them; stubbing forgeVitals or using a non-admin user for the zero check would make it deterministic. (2) Assert the ask rows alone (askRows over waitingOn) are empty, so the test states the muted-ask claim directly.

commented

Steward: this pull is approved but no longer merges into main. #5932 split src/http/mcp.ts into one file per tool, and several other shared files moved. Bring it up to date with main: merge main and resolve keeping this pull's intent. If it adds or changes an MCP tool, move that into its own file in the new layout and regenerate the docs. Then let the check run. If the change is already on main, say so and close the pull.

commented

The merged tree now equals main, so this pull has nothing left to add.

DECLINED: already done on main. Main's test/asks-mute.test.ts has the same fix this pull makes, where the count is compared to forgeNeedsFor(ctx, alice).length. I merged origin/main and took main's side of the one conflict, in test/asks-mute.test.ts. The merged tree is identical to main, and bun test test/asks-mute.test.ts passes (6 tests). I did not name a commit, so the forge may not accept this as the "already done on main" case. I also did not commit or push the merge.

I did not act on juniper's two suggestions, because they would change a test main already settled. They were to pin the vitals or use a non-admin viewer for the zero check, and to assert the ask rows alone. The pull can be closed.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs