fix: snapshot exemptions, observation stop hang, start timeout, token reload, panic guards (#784)

Closes #784 (closed), closes #29 (closed).

Phase B of the code quality action plan: five isolated correctness bugs, each with a test that pins the corrected behavior. Depends on !1197 (merged) merging first.

1. Snapshot exemptions never applied

retrieval.go checked err != errNoJobs || !errors.As(err, &existsErr). With || both exemptions were dead: errNoJobs and SnapshotExistsError returned early, so MakeActive and InitBranching were skipped. Replaced with isSnapshotExempt, which uses errors.Is/errors.As so a wrapped errNoJobs stays exempt.

2. POST /observation/stop blocked forever

RunSession sent to done only on the normal exit path. When it returned through cannot query metrics, Stop waited on <-c.done for good and the request never answered. RunSession now defer close(c.done) on entry; Stop(ctx) selects on done and ctx.Done(), and the handler passes r.Context(). A session that was inited but never started returns an error on context cancellation instead of hanging.

3. Postgres start timeout reported success

postgres.go wrapped err on timeout, but err is nil while the instance is still in recovery, so Start returned nil. startTimeoutError now reports the last pg_is_in_recovery value or the last query error.

4. Verification token rotation ignored until restart (#29 (closed))

NewAuth captured s.Config.VerificationToken once in InitHandlers; Reload swapped the config but the middleware kept comparing against the old value. NewAuth takes a func() string accessor, and the server supplies one that reads under configMu. runci passes mw.StaticToken since it has no reload.

5. Panics: CLI flag parsing, null body, no recovery

  • splitFlags indexed parsed[1] unconditionally, so --extra-config foo / --tags foo panicked. It now returns an error for an entry without = or with an empty key.
  • startObservation/stopObservation decode into *types.…Request; a null body left it nil and the handler dereferenced it. Both answer 400 now. createClone was already covered by the validator's nil check.
  • mw.Recover logs the panic with a stack, writes a 500 JSON error, and re-raises http.ErrAbortHandler. Wired around both the engine and runci routers next to mw.Logging.

Merge request reports

Loading
Loading