* refactor(observe): extract shared observer skeleton
Move the observer-pattern-general pieces of the SCM observer into a new
backend/internal/observe package so the tracker observer (issue #35) can
build on the same primitives:
- StartPollLoop: goroutine supervisor with immediate-first-poll + ticker
+ ctx-done exit. SCM Observer.Start now delegates to it.
- CheckCredentialsOnce: lazy first-poll credential gate driven by a
CredentialProbe closure. SCM observer keeps credentialsChecked/disabled
as Observer fields; the shared helper mutates them via pointer so
state ownership stays single-source.
- CacheSet[V any] / CacheDelete[V any]: one generic bounded-FIFO helper
replaces the three near-identical cacheSet{String,Time,Bool} bodies
and the standalone evictStrings. The SCM-side methods are now
one-line wrappers that thread o.Cache.max into the shared helper, so
existing call sites and tests are untouched.
SCM behavior is unchanged. The full 21-test SCM suite (including the
end-to-end test added in PR #115) plus 577 backend tests stay green
under `go test -race`.
Part of #112.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(tracker): ports.TrackerObservation DTO + ApplyTrackerFacts reducer
Land the contract that the future Tracker observer (issue #35) and its
provider adapters must satisfy. No observer is wired in this PR — the
DTO + reducer are the deliverable, and locking the shape now lets the
observer + adapter work happen in small follow-up PRs.
DTO (backend/internal/ports/tracker_observations.go):
- TrackerObservation mirrors ports.SCMObservation: Fetched bool,
ObservedAt time.Time, Provider/Host/Repo, normalized Issue facts,
Comments, and a Changed{State, Assignee, Comments} discriminator.
- TrackerIssueObservation carries the minimal facts lifecycle needs
today (state, assignee, title, body, timestamps); richer
per-provider metadata stays inside each adapter.
- TrackerCommentObservation carries the comment fields needed for the
bot-mention nudge (Author, Body, IsBot, ID for dedup).
Reducer (backend/internal/lifecycle/reactions.go):
- ApplyTrackerFacts(ctx, sessionID, ports.TrackerObservation) error,
mirroring ApplySCMObservation's "Fetched gate → terminal-state →
per-bucket reactions" shape.
- Three initial reactions:
* Issue state == done | cancelled → MarkTerminated (idempotent).
* Changed.Assignee → log only via slog.Default(). The "assignee
changed away from AO" policy is reserved for #40.
* Changed.Comments with bot comments → one-time nudge with
strings.Join'd bot bodies, deduped by comment IDs.
- The nudge path reuses sendOnce with an empty prURL so the in-memory
dedup applies but the PR-row persistence path is skipped. Tracker
signature persistence will land with #35 alongside issue-row storage.
Tests in backend/internal/lifecycle/manager_test.go cover each branch:
terminate (done + cancelled), log-only assignee, nudge fires on new
bot comment, nudge suppressed on repeat, new bot comment id refires,
not-fetched is no-op, terminated session ignores observations.
Part of #112.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(observe): rename CacheSet param to avoid shadowing built-in max
golangci-lint revive flagged the CacheSet generic helper's max
parameter as shadowing the built-in max() function. Rename to
maxEntries; signature change is internal to the observe package and
the SCM observer's one-line wrappers pass the value positionally, so
no call sites need updating.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: honour disabled state in CheckCredentialsOnce + tighten bot-comment filter
Two P1 review findings on #116:
1. observe.CheckCredentialsOnce was returning (true, nil) on every
call after the gate ran, even when the probe had marked the
observer disabled, because the *checked short-circuit ignored
*disabled. The SCM observer didn't surface this in practice — its
Poll method has an independent `if o.disabled { return nil }`
guard that runs first — but a future Tracker observer that relies
on the helper's documented contract ("Observer stays disabled")
would silently flip back to "credentials available" after the
first poll. Change the short-circuit to `return !*disabled, nil`
and lock the behavior with a regression test that issues repeat
calls after the probe reported unavailable.
2. lifecycle.newBotCommentContent's "skip uninteresting comments"
filter used && where it needed ||. A bot comment with an empty ID
but a non-empty body slipped through and appended "" to the ids
slice. If every bot comment in the observation had an empty ID,
strings.Join produced "" — which matches the zero value of the
in-memory dedup map, so sendOnce treated the nudge as
already-sent and silently suppressed it forever. Switch to || so
any comment missing either an ID or a body is dropped, and add a
regression test that an empty-ID bot comment never nudges (and
does not pollute the dedup state for a follow-up comment that has
a real ID).
586 tests pass with -race.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(observe): capture deadline once in poll-error spin-wait
The `TestStartPollLoop_LogsPollErrorWithoutPanic` spin-wait was
computing the loop bound as `time.Now().Before(time.Now().Add(200ms))`
on every iteration, which is permanently true — the loop could only
exit via the `break`. Under a scheduler delay (heavy CI load or
`GOMAXPROCS=1`) where two polls never land in time, the test would
hang until the wall-clock kill rather than failing fast.
Capture the deadline once before the loop, and tighten the assertion
to actually require two polls + done-channel closure within a bounded
window, matching `TestStartPollLoop_FirstPollImmediateThenTicks`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
33a1ee6df9