Let dev builds shorten the connector's handoff grace, and test requests from before a restart

RO
4d ago
+265-3

PR 5 of a stack of 8 that tests basecamp connect end to end. Stacked on #823 (the e2e harness), which is stacked on #822 (the fake Basecamp). It changes no behaviour of a release build.

Why

The connector hands off a request only if it was made at most HandoffGrace (one minute) before the run started. A request older than that is closed as before_this_run: nobody was there to take it, and it is not run late.

That matters most after a crash between admission and handoff. The connector admits a request, dies before it writes the request line, and starts again. Within the grace, the request is handed off (PR 3 tests that, with the default grace). After the grace, it must be closed and never written. Nothing tested that second half end to end, because a test would have to wait out a minute.

What

  • HandoffOptions.Grace (internal/connector/handoff.go). Zero means HandoffGrace, so every caller that does not set it keeps the minute. The handoff reads it through a small grace() method, as it reads its logger.
  • A dev-only knob. connect_handoff_grace_dev.go (//go:build dev) reads BASECAMP_CONNECT_HANDOFF_GRACE as a Go duration. connect_handoff_grace_stub.go (//go:build !dev) reads no environment at all and returns zero, so a release build (goreleaser builds without -tags dev) cannot be affected. This is the same pattern as dev_tag.go / dev_tag_stub.go. There is no hidden flag.
  • runConnect reads it before it touches the account. A value that is not a positive duration is a usage error. A run with the override logs connector: the handoff grace is overridden for this dev build on stderr, with the grace and the default.
  • Log order. The handoff logged a request from before this run started is not handed off before the ledger write. A run stopped between the two left the record admitted while its log said it was closed. The e2e test found this. The log now comes after the write.

Tests

  • e2e/connect/grace_test.go (a new file; no harness changes). With a 2s grace: connect --hold, a trusted mention, wait for its admitted event line, kill -9, basecamp connect release -P agent, then a restart. On the restart there is no request line, the connector logs the close for that event id, and connect status --json shows no admitted or queued record.
    • No sleeping. The fake (WithClock) stamps the request 10s in the past instead of the test waiting out the grace. To the connector it is the same request: the handoff judges only the event's created_at, as the feed served it, against when the run started, and nothing else in intake or admission reads created_at. Before the restart, the test checks from the pointer line's created_at that the request is already older than the grace.
    • It needs the knob. 10s is well inside the default minute, so without the shortened grace the request would be handed off and the test would fail.
    • Without -tags dev the test skips and says why: a release build reads no environment for the grace. make test, bin/ci, the unit test job, the race shards and the release workflow all run go test -tags dev, so it always runs there.
  • handoff_test.go: a grace of 30s hands off a request made 30s before the run (the boundary included), a grace of 29s closes it as before_this_run, and no grace means HandoffGrace.
  • connect_handoff_grace_dev_test.go (dev-tagged): the parse accepts Go durations and empty (zero), refuses 2, soon, 0s and -1s with the variable's name in the error, and reads the environment.

go test -tags dev -race -count=10 passes on the new tests. bin/ci is green.

Summary by cubic

Lets dev builds shorten the connector's handoff grace so an end-to-end test can reach a crash-after-grace restart in seconds; release builds read no environment for the grace and keep the one-minute default.

  • Adds HandoffOptions.Grace; zero means the existing HandoffGrace, and only a dev build sets it from BASECAMP_CONNECT_HANDOFF_GRACE, with invalid values a usage error.
  • Moves the before_this_run log after the ledger write, so a stopped run no longer logs a close it has not yet written.
  • Adds unit tests for the boundary and default and an e2e test that holds and kills a run, then restarts past a 2s grace; the fake backdates the request instead of the test sleeping.