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

A test pins contributing when the fork never arrives or its mirror fails: the answer says so and nothing is half-made #7161

Merged opened this

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

Merged

Two new tests pin both fork failures. The clone-failure test found a bug, which I fixed in the same pull.

shipped production is serving it deploy 4c3a9d3

opened this

Two new tests pin both fork failures. The clone-failure test found a bug, which I fixed in the same pull.

What the tests cover (test/contribute-fork-failures.test.ts, run against the fake origin):

  • fork_timeout: the fake origin accepts the fork but never shows it. The API answers 504 fork_timeout with a message in words. No branch, commit or pull is made at the origin, and no repository, pull or directory is added here.
  • mirror_failed: the fork exists at the origin but its git host is unreachable. The API answers 502 mirror_failed with a message in words, and no local mirror row or directory is left behind. After the fork's git works again, the same contribute call succeeds with 201, so nothing blocks a retry.

The bug: a clone failure never reached mirror_failed. forkOrigin returned the half-made, empty mirror, and contribute went on and failed at the branch step. The caller got a misleading 422 branch_failed, and an empty alice/gears stayed behind. In src/actions/contribute.ts, when the awaited mirror job fails, forkOrigin now deletes the repository it just made. contribute then answers mirror_failed. Mirrors that people ask for directly keep their existing behaviour.

Test seam: the 30 s fork wait is now a mutable forkDeadline export, the same pattern as firstFetchRetry. The test shortens it to 100 ms and restores it afterwards. Production keeps 30 s.

Not done: I did not add a per-request override for the wait, and I did not touch the direct (push-rights) path's mirror_failed.

Checks run: tsc --noEmit is clean, the new test passes, and the 17 neighbouring suites that mention contribute or fork pass (257 tests). I did not run the whole bun test.

Closes #7152

commented

Two new tests pin both fork failures. The clone-failure test found a bug, which I fixed in the same pull.

What the tests cover (test/contribute-fork-failures.test.ts, run against the fake origin):

  • fork_timeout: the fake origin accepts the fork but never shows it. The API answers 504 fork_timeout with a message in words. No branch, commit or pull is made at the origin, and no repository, pull or directory is added here.
  • mirror_failed: the fork exists at the origin but its git host is unreachable. The API answers 502 mirror_failed with a message in words, and no local mirror row or directory is left behind. After the fork's git works again, the same contribute call succeeds with 201, so nothing blocks a retry.

The bug: a clone failure never reached mirror_failed. forkOrigin returned the half-made, empty mirror, and contribute went on and failed at the branch step. The caller got a misleading 422 branch_failed, and an empty alice/gears stayed behind. In src/actions/contribute.ts, when the awaited mirror job fails, forkOrigin now deletes the repository it just made. contribute then answers mirror_failed. Mirrors that people ask for directly keep their existing behaviour.

Test seam: the 30 s fork wait is now a mutable forkDeadline export, the same pattern as firstFetchRetry. The test shortens it to 100 ms and restores it afterwards. Production keeps 30 s.

Not done: I did not add a per-request override for the wait, and I did not touch the direct (push-rights) path's mirror_failed.

Checks run: tsc --noEmit is clean, the new test passes, and the 17 neighbouring suites that mention contribute or fork pass (257 tests). I did not run the whole bun test.

Approved

Approved. Both fork failures are pinned. The fix is small and sits only on the awaited-mirror path, so the job and watch paths used by the page and terminal are unchanged. The preview answers 200 on /. It is a test and API-path change with no UI, so I checked no page for it. Next two things: (1) check the result of destroyRepo, and log it if the delete fails, so a failed cleanup is not silent. (2) Simplify the retry setup in the second test. The cloneUrl .replace dance should just be a template string with ME. Neither blocks landing.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs