fix(test): unbreak nightly daemon integration + chaos tests#228
Merged
Conversation
The Nightly Validate workflow (dagger call validate — chaos + mutation +
snapshots) has been red every day for weeks. Per-PR CI only runs Tier 0/1
(dagger call test), which excludes the integration/chaos tier, so these
failures never gated a PR and accumulated silently.
Three stacked, test-only root causes in the daemon tests:
1. Socket not isolated. DefaultSocketPath is frozen at package init from
~/.hookwise, so the tests' t.Setenv("HOOKWISE_STATE_DIR", ...) never
moves it. Since #67 made socket-bind the single-instance authority, the
three daemon tests (integration x2 + chaos x1) all bound the shared real
~/.hookwise/daemon.sock and collided — with each other under parallel
`go test ./...`, or with a live local daemon. Fix: SetSocketPath into a
short /tmp dir (matching every feeds/bridge daemon test), short enough to
stay under the macOS 104-byte unix-socket limit.
2. Weather defaults to disabled. FeedEnabled("weather") reads
cfg.Weather.Enabled; the lifecycle test passed an empty FeedsConfig{}, so
the daemon skipped the weather producer and weather.json was never
written, failing the "both cache files" wait. Fix: enable weather in the
test config (as a real user must).
3. ttl_seconds type mismatch. After the JSON cache round-trip ttl_seconds is
a float64; the assertion compared it against the untyped-int
DefaultTTLSeconds. Value is correct (60s interval -> 180 -> floored 300);
fix: cast to float64, matching the sibling assertion.
No production code touched; changes are behind //go:build integration.
Test evidence: full integration + chaos suites green under
GOMEMLIMIT=4GiB go test -race -p 2 -tags integration, including run together
(the parallel-daemon collision scenario), with a real daemon holding the
default socket.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EjWqH4FMkdczkSagzXw3dQ
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a ChangesTest socket path isolation and fixes
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced Jul 10, 2026
vishnujayvel
added a commit
that referenced
this pull request
Jul 11, 2026
* test: cover feedData/renderCalendarSegment stale+missing (hw-c195) Scout hw-xthx (blind spot #4) found the calendar status-line segment had no direct coverage of its freshness gate: cli_test.go exercises only the fresh render paths, so a regression in feedData's TTL check (the sole thing standing between a dead daemon and a permanently-stale calendar segment) would pass every existing test. Adds direct unit tests for feedData — malformed/missing envelopes, default-TTL expiry, custom ttl_seconds override in both directions, and placeholder-source suppression — and for renderCalendarSegment across fresh/stale/missing cache plus the next_event shape variants (string, time-fallback, name-only, empty, unexpected type). The stale case uses the same payload as the fresh case with only the embedded timestamp moved, proving omission is caused by the freshness gate alone. Validation: GOMEMLIMIT=4GiB go test -race -p 2 ./cmd/hookwise/ passes; golangci-lint clean. Test-only change, no production code touched. * test: dead-daemon calendar e2e for Go status line (hw-c195) No integration test covered the calendar feed end-to-end (scout hw-xthx blind spot #4): TestIntegration_DaemonLifecycleWithCacheBridge registers only pulse/weather mocks, so the daemon->cache->status-line pipeline for calendar was validated by inspection only. Adds an integration-tagged e2e test in cmd/hookwise (it must live here: renderBuiltinSegment is package main): a real daemon writes a calendar envelope, the daemon stops, and the test asserts (1) the fresh leftover envelope still renders the segment — the presence detector that keeps the omission assertion honest — and (2) after aging the EMBEDDED timestamp past the daemon-injected ttl_seconds, the segment is omitted even though rewriting the file makes its mtime current. That mtime-fresh/content-stale divergence is exactly what a dead daemon leaves behind; freshness must key on content (bridge.IsEnvelopeFresh), never mtime. The TTL offset is derived from the on-disk ttl_seconds the daemon actually injected, not a hardcoded constant. Honors the #228 isolation contract: HOOKWISE_STATE_DIR=t.TempDir(), and an explicit short /tmp socket path because DefaultSocketPath is frozen at package init and ignores the env override. //go:build integration keeps it in the nightly tier, off the per-PR path. Validation: GOMEMLIMIT=4GiB go test -race -p 2 -tags integration ./cmd/hookwise/ passes (new test + full package). * test: add calendar to chaos matrix (hw-c195) internal/chaos had zero calendar references while weather/pulse were exercised (scout hw-xthx blind spot #4). Mirrors the existing chaos patterns for calendar: corrupt-cache-skipped (valid calendar.json survives corrupt siblings), producer-panic-recovery (a panicking calendar producer neither crashes the daemon nor blocks sibling feeds, and leaves no cache file), and a new dead-daemon staleness transition (envelope fresh right after daemon stop, stale once its embedded timestamp ages past the daemon-injected ttl_seconds, with file mtime deliberately fresh to pin content-based freshness). The panic test enables Calendar in FeedsConfig — calendar is a recognised feed that defaults to disabled, and a skipped producer would never exercise the panic-recovery path. Reuses the package's existing goodProducer/panicProducer/shortSocketPath helpers; #228 isolation (HOOKWISE_STATE_DIR + explicit socket path) throughout. Validation: GOMEMLIMIT=4GiB go test -race -p 2 -tags integration ./internal/chaos/ passes (new tests + full package).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the Nightly Validate workflow, which has been failing every day for
weeks (back to at least June 23). Per-PR
CIonly runs Tier 0/1 (dagger call test), which excludes the integration/chaos tier, so these failures nevergated a PR and went unnoticed.
The nightly's
dagger call validate(chaos + mutation + snapshots) was redbecause two integration tests + one chaos test fail. Mutation and snapshots
pass. All fixes are test-only (behind
//go:build integration); noproduction code is touched.
Root causes (three, stacked in the lifecycle test)
DefaultSocketPathis frozen at package init from~/.hookwise, so the tests't.Setenv("HOOKWISE_STATE_DIR", …)never movesit. Since feat: daemon connect-or-start with Unix socket IPC #67 made socket-bind the single-instance authority (SOCKET-1), the
three daemon tests all bound the shared real
~/.hookwise/daemon.sockandcollided — with each other under parallel
go test ./..., or with a livelocal daemon. Fix:
SetSocketPathinto a short/tmpdir (matching everyfeeds/bridgedaemon test), short enough for the macOS 104-byteunix-socket limit.
FeedEnabled("weather")readscfg.Weather.Enabled; the lifecycle test passed an emptyFeedsConfig{},so the daemon skipped the weather producer and
weather.jsonwas neverwritten. Fix: enable weather in the test config (as a real user must).
ttl_secondstype mismatch. After the JSON cache round-trip it's afloat64; the assertion compared against untyped-intDefaultTTLSeconds.Value is correct (60s → 180 → floored 300); fix: cast to
float64, matchingthe sibling assertion.
Test evidence
Full integration + chaos suites green:
Verified with a real daemon holding the default socket and run together
(the exact parallel-daemon collision the nightly hits).
Note (not addressed here)
In production the daemon socket path doesn't honor
HOOKWISE_STATE_DIRatruntime the way the TUI PID does after #227 —
DefaultSocketPathis frozen atinit. That's a separate daemon-coordination question, not needed to green the
nightly, and left for a supervised follow-up.
🤖 Generated with Claude Code
Summary by CodeRabbit