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

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:13750cmn0i is past its budget: trevato/forge: "MCP backups > a listing that fails is a short sentence with no key in it" failed on 6 pulls in the last… #8146

Merged opened this

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

Merged

The MCP `backups` tool now tries a listing once. The named test ("a listing that fails is a short sentence with no key in it") no longer waits out the retry backoff. `tsc --noEmit` is clean, and `test/mcp-backups.test.ts` and `test/mcp-size.test.ts` pass (10 tests). I did not run `scripts/gate.ts`, the design lint or the full suite, and the forge runs the check on the push.

shipped production is serving it deploy 5e7033f

opened this

The MCP backups tool now tries a listing once. The named test ("a listing that fails is a short sentence with no key in it") no longer waits out the retry backoff. tsc --noEmit is clean, and test/mcp-backups.test.ts and test/mcp-size.test.ts pass (10 tests). I did not run scripts/gate.ts, the design lint or the full suite, and the forge runs the check on the push.

Cause. The test points the store at a bad S3 endpoint and expects the listing to fail at once. The store retries every call five times with doubling waits, which is 500 ms + 1 s + 2 s + 4 s, about 7.5 s of sleeping before it gives up. The test's limit is 20 s. On a loaded check machine that margin runs out, so the verdict depended on the machine's load as well as the code. That would explain why it failed across many unrelated pulls.

Change.

  • s3Store(target, attempts?) in src/snapshot/store.ts takes an optional attempt count.
  • Sources.attempts in src/snapshot/destination.ts passes that count through resolveDestination.
  • The backups tool in src/http/mcp/tools/backups.ts asks for one attempt.

Left alone. Nightly snapshots, restores and the Backups page keep the five-attempt retry. A status read of a dead store now answers immediately and says so in its short sentence, with no key in it. The test is unchanged, and no MCP tool or ceiling was added or raised.

Try it. bun test test/mcp-backups.test.ts. The failing case now takes milliseconds instead of about 7.5 s.

Closes #8113

commented

I found nothing in the pull to fix, so I haven't pushed again. The check's install step failed because its checkout had no package.json.

package.json and bun.lock are in the branch's tree and in origin/main at fd794a7. The pull changes only src/http/mcp/tools/backups.ts, src/snapshot/destination.ts and src/snapshot/store.ts. None of those touch the install. The log shows "cache not filled: no bun lockfile in the checkout", so the failure happened while the checkout was being assembled, not in the code. I can't see the check machine from here.

A push with no real change would only repeat the same check. If it fails the same way again, a person should look at how the check builds the merge of fcaeef6 into main. I'll fix any real failure in the full-suite log when it comes.

The fix itself is as before. The MCP backups tool lists with one attempt, so a dead store answers at once instead of after about 7.5 s of retry waits. tsc --noEmit is clean, and test/mcp-backups.test.ts and test/mcp-size.test.ts pass locally.

commented

The MCP backups tool now reads the store with one try. A dead store answers at once with its short, key-free sentence, instead of after 7.5 s of retry waits. This fixes the failing test "MCP backups > a listing that fails is a short sentence with no key in it" (P0 Trust).

Cause. The test points S3_ENDPOINT at something that is not a URL. Every store call goes through withRetries: five attempts, with 500 ms + 1 s + 2 s + 4 s of real sleeping between them. That left about 7.5 s of fixed waiting inside a 20 s test limit. On a busy check machine the rest of the margin ran out, so the result depended on machine load rather than on the code.

Change.

  • s3Store(target, attempts?) in src/snapshot/store.ts takes an optional number of attempts.
  • resolveDestination in src/snapshot/destination.ts passes a new Sources.attempts through to it.
  • The backups tool in src/http/mcp/tools/backups.ts asks for attempts: 1. A status read should report the store's state, not wait to recover it.
  • test/mcp-backups.test.ts now checks that the listing says after 1 attempt:. Without the fix the test fails after 7.5 s with "names snapshots/ failed after 5 attempts"; with it, the test passes in milliseconds. The check reads the error text, not a timer.

Not changed. Nightly snapshots, restore, the restore drill and the Backups page keep the five-attempt retry. No MCP tool was added and no size limit was raised; test/mcp-size.test.ts passes.

Earlier red check. It failed at bun install with "could not find a package.json" while assembling the merge checkout. package.json and bun.lock are in both this branch and main, so I treated that as a problem on the check machine, not in this pull.

Try it. bun test test/mcp-backups.test.ts. bun x tsc --noEmit is clean, and that file and test/mcp-size.test.ts pass locally. I did not run scripts/gate.ts here, and the design lint is not needed because nothing under src/web/ changed.

🤖 Generated with Claude Code

Approved

Clean, minimal fix for a time-dependent flake. The MCP backups status read now makes one store attempt, so a dead store answers at once. Restore, nightly snapshots and the Backups page keep five attempts. The attempts option is optional and defaults to the old behaviour, and the test asserts on the error text ("after 1 attempt:"), not on a timer. No security surface changed: the admin gate is untouched and the listing stays capped and key-free. No new dependency, no dead code, no tool-list growth.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs