Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 35 additions & 15 deletions commands/awol.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ const loaCacheUnavailableFooter = "⚠️ LOA cache unavailable; On LOA column m
// loaCacheReader is the minimal LOA-cache surface /awol consumes. Production
// wires *utils.LOACache (GlobalLOACache); tests substitute a fake with canned
// entries and a forced health verdict so the handler stays deterministic
// without touching the process-global singleton. IsHealthy gates whether the
// per-member On LOA column can be trusted (#96): when the cache is stale,
// IsOnLOA/GetEntry silently return all-clear, so the column is rendered as
// "unknown" rather than a misleading "false".
// without touching the process-global singleton. /awol reads each member's LOA
// state with a SINGLE GetEntry call and derives both the On LOA verdict
// (entry.IsActive) and the [[LOA]] link from that one snapshot (#158/S4), so the
// two can't disagree across a concurrent refresh. IsHealthy gates whether the
// per-member On LOA column can be trusted (#96): it is an age check on the last
// successful refresh, so an unhealthy cache is stale, not empty — GetEntry may
// still return (now possibly outdated) windows. When unhealthy, the render path
// gates the column to "unknown" rather than trusting those stale reads.
type loaCacheReader interface {
IsOnLOA(username string) bool
GetEntry(username string) (utils.LOAEntry, bool)
IsHealthy(maxAge time.Duration) (bool, time.Time)
}
Expand All @@ -45,6 +48,12 @@ type AwolUser struct {
TimeSinceLastPost string
LastPostDate time.Time
OnLOA bool
// LOAEntry is the single cache snapshot read for this user (see the GetEntry
// call in runAwol). HasLOAEntry records whether that read found a window, so
// the [[LOA]] link can reuse the same snapshot instead of a second, possibly
// inconsistent, cache lookup. OnLOA is derived from this snapshot's window.
LOAEntry utils.LOAEntry
HasLOAEntry bool
}

func Awol() Command {
Expand Down Expand Up @@ -97,11 +106,13 @@ func runAwol(r utils.InteractionResponder, cache loaCacheReader, now time.Time,
return
}

// #96: probe cache health once per invocation. When unhealthy, IsOnLOA
// returns false for everyone silently, so the On LOA column would lie. We do
// NOT abort — /awol's primary signal (lastForumPostDate) is independent — but
// render the column as "unknown" and warn. No Sentry capture: operational
// degradation, not an internal error (ADR 0001).
// #96: probe cache health once per invocation. IsHealthy is an age check on
// the last successful refresh, so an unhealthy cache is stale, not empty —
// per-user GetEntry reads may still return (possibly outdated) windows that
// the On LOA column would otherwise present as current truth. We do NOT abort
// — /awol's primary signal (lastForumPostDate) is independent — but the render
// path below gates the column to "unknown" via cacheHealthy and warns. No
// Sentry capture: operational degradation, not an internal error (ADR 0001).
cacheHealthy, lastRefresh := cache.IsHealthy(loaCacheMaxAge)
if !cacheHealthy {
utils.Debug("AWOL served with unhealthy LOA cache",
Expand Down Expand Up @@ -141,12 +152,19 @@ func runAwol(r utils.InteractionResponder, cache loaCacheReader, now time.Time,
utils.HandleError(r, i, "❌ Failed to parse uniform URL")
return
}
// #158/S4: read the LOA cache ONCE per user. Deriving OnLOA from this
// same snapshot (rather than a separate active-window lookup) means the
// On LOA verdict and the [[LOA]] link below can never disagree, even if a
// 15-min refresh lands mid-loop now that ended windows are retained.
entry, hasEntry := cache.GetEntry(member.User.Username)
awolUsers = append(awolUsers, AwolUser{
Username: member.User.Username,
MilpacUrl: fmt.Sprintf("https://7cav.us/rosters/profile/%s", matches[1]),
TimeSinceLastPost: utils.FormatTimeSinceDuration(lastPostDate),
LastPostDate: lastPostDate,
OnLOA: cache.IsOnLOA(member.User.Username),
OnLOA: hasEntry && entry.IsActive(now),
LOAEntry: entry,
HasLOAEntry: hasEntry,
})
}
}
Expand Down Expand Up @@ -185,8 +203,10 @@ func runAwol(r utils.InteractionResponder, cache loaCacheReader, now time.Time,
case !cacheHealthy:
loaTag = "On LOA: unknown — "
case user.OnLOA:
if entry, ok := cache.GetEntry(user.Username); ok && entry.ThreadID != 0 {
loaTag = fmt.Sprintf("**[[LOA]](https://7cav.us/threads/%d/)** ", entry.ThreadID)
// Reuse the single per-user snapshot captured above — no second cache
// read — so the link can't point at a thread the row's OnLOA disagrees with.
if user.HasLOAEntry && user.LOAEntry.ThreadID != 0 {
loaTag = fmt.Sprintf("**[[LOA]](https://7cav.us/threads/%d/)** ", user.LOAEntry.ThreadID)
} else {
loaTag = "**[LOA]** "
}
Expand Down Expand Up @@ -263,8 +283,8 @@ func sendAwolFile(r utils.InteractionResponder, i *discordgo.InteractionCreate,
}

for _, user := range awolUsers {
// #96: only trust IsOnLOA when the cache is healthy; otherwise the
// column is unknown, not all-clear.
// #96: only trust the OnLOA snapshot when the cache is healthy; otherwise
// the column is unknown, not all-clear.
loaTag := ""
switch {
case !cacheHealthy:
Expand Down
89 changes: 63 additions & 26 deletions commands/awol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ func serveAwolRoster(t *testing.T, roster utils.LiteRosterResponse, rosterStatus

// fakeLOACache is a deterministic loaCacheReader for /awol integration tests.
// entries are keyed by lowercased username (matches the production cache's
// case-folding); IsOnLOA returns true iff an entry exists, decoupling the test
// from time.Now since the handler's "now" is already injected separately.
// case-folding). The handler derives OnLOA from GetEntry's window via
// IsActive(now), so entries whose dates straddle the injected `now` read active.
// healthy/lastRefresh drive the IsHealthy staleness guard; the zero value is
// unhealthy, so existing tests that want the original behavior should construct
// via healthyCache.
Expand All @@ -69,11 +69,6 @@ func (f *fakeLOACache) GetEntry(username string) (utils.LOAEntry, bool) {
return e, ok
}

func (f *fakeLOACache) IsOnLOA(username string) bool {
_, ok := f.entries[strings.ToLower(username)]
return ok
}

func (f *fakeLOACache) IsHealthy(_ time.Duration) (bool, time.Time) {
return f.healthy, f.lastRefresh
}
Expand All @@ -84,15 +79,13 @@ func healthyCache(entries map[string]utils.LOAEntry) *fakeLOACache {
return &fakeLOACache{entries: entries, healthy: true, lastRefresh: awolRefDate}
}

// dateAwareLOACache is a loaCacheReader whose IsOnLOA respects each entry's
// StartDate/EndDate window evaluated at a FIXED `at` instant — unlike fakeLOACache,
// whose IsOnLOA returns true for any existing entry regardless of dates. This lets
// an /awol test exercise the "entry exists but is not active today" path (upcoming
// or expired), proving such a member is excluded from the (N on LOA) tally and the
// [LOA] decoration on the healthy path. This fake substitutes its own fixed clock
// `at` for the wall clock that production's utils.LOACache.IsOnLOA reads internally
// (the real method takes no time argument). The two are fidelity-equivalent only
// because the test windows are weeks wide relative to the `at`-vs-wall-now skew.
// dateAwareLOACache is a loaCacheReader returning entries whose StartDate/EndDate
// windows are meaningful relative to a FIXED `at` instant. The handler derives
// OnLOA from GetEntry's window via IsActive(now), so this lets an /awol test
// exercise the "entry exists but is not active today" path (upcoming or expired),
// proving such a member is excluded from the (N on LOA) tally and the [LOA]
// decoration on the healthy path. `at` is also returned as the IsHealthy
// timestamp so the handler's injected `now` and the cache clock agree.
type dateAwareLOACache struct {
entries map[string]utils.LOAEntry
at time.Time
Expand All @@ -103,15 +96,6 @@ func (f *dateAwareLOACache) GetEntry(username string) (utils.LOAEntry, bool) {
return e, ok
}

func (f *dateAwareLOACache) IsOnLOA(username string) bool {
e, ok := f.entries[strings.ToLower(username)]
if !ok {
return false
}
// Mirror utils.LOACache's inclusive active window evaluated at `at`.
return !f.at.Before(e.StartDate) && !f.at.After(e.EndDate)
}

func (f *dateAwareLOACache) IsHealthy(_ time.Duration) (bool, time.Time) {
return true, f.at // always healthy: this fake exercises the date-window path
}
Expand Down Expand Up @@ -446,7 +430,7 @@ func TestRunAwol_InactiveLOAEntryNotCountedOrTagged(t *testing.T) {

// unhealthyCache builds an unhealthy fakeLOACache: entries may exist (and the
// roster member may even have a "real" LOA) but the health probe reports stale,
// so the handler must NOT trust IsOnLOA/GetEntry and must render "unknown".
// so the handler must NOT trust GetEntry's window and must render "unknown".
func unhealthyCache(entries map[string]utils.LOAEntry, lastRefresh time.Time) *fakeLOACache {
return &fakeLOACache{entries: entries, healthy: false, lastRefresh: lastRefresh}
}
Expand Down Expand Up @@ -553,3 +537,56 @@ func TestRunAwol_UnhealthyCacheFileOutputCarriesWarning(t *testing.T) {
t.Fatalf("file should mark On LOA unknown.\nGot:\n%s", body)
}
}

// countingLOACache wraps fakeLOACache to count GetEntry calls per username,
// proving the S4 single-snapshot invariant: /awol reads each member's LOA state
// exactly ONCE (deriving both OnLOA and the [[LOA]] link from that one snapshot),
// not once for the verdict and again for the link.
type countingLOACache struct {
*fakeLOACache
getEntryCalls map[string]int
}

func (c *countingLOACache) GetEntry(username string) (utils.LOAEntry, bool) {
if c.getEntryCalls == nil {
c.getEntryCalls = map[string]int{}
}
c.getEntryCalls[strings.ToLower(username)]++
return c.fakeLOACache.GetEntry(username)
}

// TestRunAwol_S4_SingleSnapshotPerUser pins the #158/S4 fix: an active LOA member
// with a thread renders the [[LOA]] link, and the handler reads the cache for that
// member exactly once — so a concurrent refresh can't make the link and the OnLOA
// verdict disagree.
func TestRunAwol_S4_SingleSnapshotPerUser(t *testing.T) {
roster := utils.LiteRosterResponse{
LiteProfiles: map[string]utils.LiteProfileResponse{
"100": awolMember("Trooper.A", "100", "2026-03-01 12:00:00"),
},
}
serveAwolRoster(t, roster, http.StatusOK)

cache := &countingLOACache{
fakeLOACache: healthyCache(map[string]utils.LOAEntry{
"trooper.a": {
Username: "Trooper.A",
StartDate: mustParseAwolDate("2026-05-01 00:00:00"),
EndDate: mustParseAwolDate("2026-06-01 00:00:00"), // active at awolRefDate
ThreadID: 4242,
},
}),
}
f := &fakeResponder{}
i := fakeAppCommandInteraction(stringOption("position", "1-7"))

runAwol(f, cache, awolRefDate, i)

if n := cache.getEntryCalls["trooper.a"]; n != 1 {
t.Fatalf("S4: GetEntry must be called exactly once per member, got %d", n)
}
desc := (*f.Calls()[1].Edit.Embeds)[0].Description
if !strings.Contains(desc, "**[[LOA]](https://7cav.us/threads/4242/)**") {
t.Fatalf("active member must render the [[LOA]] link from the single snapshot.\nGot:\n%s", desc)
}
}
13 changes: 11 additions & 2 deletions commands/loa.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ import (
// entry lookup plus a health probe for the staleness guard. Production wires
// *utils.LOACache (GlobalLOACache); tests substitute a fake with canned
// entries and a forced health verdict so the handler stays deterministic
// without touching the process-global singleton. Distinct from /awol's
// loaCacheReader, which needs IsOnLOA but not IsHealthy.
// without touching the process-global singleton. /awol's loaCacheReader is a
// separate, per-command minimal surface over the same cache with the same
// {GetEntry, IsHealthy} shape today; the two are kept distinct so each command's
// dependency stays scoped to what it actually reads (and so they can diverge —
// see #159, which adds GetEntries to loaCacheReader). /awol additionally gates on
// IsHealthy to render its On-LOA column (#96); /loa uses it for the staleness
// guard above.
type loaCacheView interface {
GetEntry(username string) (utils.LOAEntry, bool)
IsHealthy(maxAge time.Duration) (bool, time.Time)
Expand Down Expand Up @@ -119,6 +124,10 @@ func runLoa(r utils.InteractionResponder, cache loaCacheView, now time.Time, i *
var activeLOAs, upcomingLOAs []LOAUser

for _, member := range roster.LiteProfiles {
// #158/S3: /loa renders exactly one window per user via the single-value
// GetEntry (most-relevant) contract, deliberately NOT iterating GetEntries.
// This preserves the pre-history-store rendering; pinned by
// TestGetEntry_SingleWindowContract_ForLoaRendering in utils.
entry, ok := cache.GetEntry(member.User.Username)
if !ok {
continue
Expand Down
Loading