Skip to content

fix(daemon): reuse cached service binary on startup#896

Merged
wizzomafizzo merged 4 commits into
mainfrom
fix/service-binary-cache-startup
Jun 6, 2026
Merged

fix(daemon): reuse cached service binary on startup#896
wizzomafizzo merged 4 commits into
mainfrom
fix/service-binary-cache-startup

Conversation

@wizzomafizzo

@wizzomafizzo wizzomafizzo commented Jun 5, 2026

Copy link
Copy Markdown
Member

Summary

  • keep daemon startup/restart executing the cached service binary instead of the source zaparoo.sh
  • tolerate cached service ctime drift while preserving strict source metadata checks
  • add startup timing logs and daemon tests for cache reuse, ctime drift, and source replacement while running

Checked

  • go test ./pkg/service/daemon/ ./pkg/service/
  • golangci-lint run pkg/service/daemon pkg/service

Summary by CodeRabbit

  • Bug Fixes

    • Improved service-binary cache validation and manifest handling to more reliably reuse cached launch copies and tolerate benign timestamp drift without forcing recopy.
  • Chores

    • Added detailed startup timing and debug logs for various initialization phases.
  • Tests

    • Added unit tests covering cached-binary/manifest validity and cache-miss launch behavior; simplified a test helper to use port 0.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0831b99f-b66a-4244-b0bb-91302ba70069

📥 Commits

Reviewing files that changed from the base of the PR and between 2319057 and ce4a318.

📒 Files selected for processing (2)
  • pkg/api/client/client_test.go
  • pkg/service/daemon/daemon_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/service/daemon/daemon_test.go

📝 Walkthrough

Walkthrough

Refactors manifest-based service-binary cache validation into a reusable helper with launch-specific matching; updates prepareBinary to delegate cache lookups and prune caches on hit; adds tests covering cache reuse behavior; and instruments service startup with timing/debug logs.

Changes

Service Binary Caching Refactor and Startup Instrumentation

Layer / File(s) Summary
Cache validation helpers and metadata matching
pkg/service/daemon/daemon.go
New cachedServiceBinaryFromManifest centralizes manifest and cached-binary validation; new matchesServiceMetadataForLaunch implements launch-specific rules that treat ServiceModTimeNS==0 as a wildcard while requiring size.
prepareBinary refactor and debug logging
pkg/service/daemon/daemon.go
prepareBinary delegates manifest cache lookup to cachedServiceBinaryFromManifest (with launchValidation=true); on cache hit prunes other cached binaries. Adds debug logs: restartExecConfig logs selected serviceBin path and Start logs serviceBin with preparation duration.
Cache behavior test cases
pkg/service/daemon/daemon_test.go
Adds TestPrepareBinary_UsesValidManifestCache, TestPrepareBinary_UsesCacheWhenOnlyChangeTimeDrifts, and TestStart_CacheMissLaunchesCachedCopy to validate manifest-based cache reuse, tolerance for change-time drift, and startup behavior with cached copies.
Service startup timing and observability
pkg/service/service.go
Adds timing instrumentation and structured debug duration logs around environment setup, platform pre-start, DB open, mappings/launcher load, launcher cache init, API readiness timing, and per-cache loads for tag/slug caches; preserves existing error handling.
Client test simplification
pkg/api/client/client_test.go
Replaces dynamic ephemeral port probing with unusedPort() returning 0 and removes the net import, simplifying tests that expect no listener.

Sequence Diagram

sequenceDiagram
  participant prepareBinary
  participant cachedServiceBinaryFromManifest
  participant matchesServiceMetadataForLaunch
  prepareBinary->>cachedServiceBinaryFromManifest: call with launchValidation=true
  cachedServiceBinaryFromManifest->>cachedServiceBinaryFromManifest: validate manifest source metadata
  cachedServiceBinaryFromManifest->>cachedServiceBinaryFromManifest: check ServicePath equals cache path
  cachedServiceBinaryFromManifest->>cachedServiceBinaryFromManifest: validate ServicePath is cache-contained
  cachedServiceBinaryFromManifest->>matchesServiceMetadataForLaunch: check cached binary metadata
  matchesServiceMetadataForLaunch-->>cachedServiceBinaryFromManifest: metadata match result
  alt Cache hit
    cachedServiceBinaryFromManifest->>cachedServiceBinaryFromManifest: ensure executable, log cache usage
    cachedServiceBinaryFromManifest-->>prepareBinary: return cached path
  else Cache miss
    cachedServiceBinaryFromManifest->>cachedServiceBinaryFromManifest: log metadata mismatch details
    cachedServiceBinaryFromManifest-->>prepareBinary: return no cache hit
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰
I hopped through manifests, small and neat,
Found cached binaries snug and sweet,
I pruned the piles and timed the start,
A hopping log of every part—
Cache held fast, the service beats its heart.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(daemon): reuse cached service binary on startup' directly aligns with the primary objective: refactoring daemon startup to execute cached service binaries instead of the source script, with manifest-based caching logic.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/service-binary-cache-startup

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/service/daemon/daemon_test.go`:
- Around line 256-280: The test TestStart_CacheMissLaunchesCachedCopy currently
overwrites the source script at sourcePath but never verifies that the cached
copy is still used; after writing the new contents to sourcePath, either call
svc.Restart() (or svc.Start() again) and assert the running service still logs
the original "started:" entry in eventLog and/or the PID from svc.Pid() remains
from the cached binary, or assert that the eventLog does not contain the string
"replaced" to prove the new source was not executed; update assertions
accordingly to validate cached-copy behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09ba3f7c-fcf5-4592-ae0b-8f8d246f9f68

📥 Commits

Reviewing files that changed from the base of the PR and between 46b1d7c and 2319057.

📒 Files selected for processing (3)
  • pkg/service/daemon/daemon.go
  • pkg/service/daemon/daemon_test.go
  • pkg/service/service.go

Comment thread pkg/service/daemon/daemon_test.go
@sentry

sentry Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.90361% with 20 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/service/service.go 54.16% 11 Missing ⚠️
pkg/service/daemon/daemon.go 84.74% 5 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@wizzomafizzo
wizzomafizzo merged commit e66d290 into main Jun 6, 2026
12 checks passed
@wizzomafizzo
wizzomafizzo deleted the fix/service-binary-cache-startup branch June 6, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant