fix(hooks): correct save-nudge timezone parsing and first-observation gating#669
Open
Alexter2003 wants to merge 1 commit into
Open
Conversation
… gating parse_epoch() in the claude-code and codex user-prompt-submit hooks parsed sqlite's naive UTC timestamps (`started_at`, observation `created_at`) as local time. On any host not in UTC+0, this skews the computed epoch by the host's offset, so the >5min session-age gate and the >15min stale-save threshold compare against a wrong reference point — negative offsets (e.g. UTC-6) push the parsed time into the future and the nudge never fires; positive offsets (e.g. UTC+1/+2) push it into the past and the nudge fires too eagerly. Force -u on every naive-timestamp parse path so it's always read as UTC, matching how it was written. Separately, all three hook implementations (claude-code, codex, opencode) treated "no observations yet for this project" as "session just started" and skipped the nudge unconditionally. For a brand-new project this means the nudge that is supposed to prompt the very first save can never fire, since it only fires once at least one observation already exists. Treat "never saved" as maximally stale instead, so first-time projects can still get nudged after the existing 5-minute session-age gate has passed. Verified by exercising the hook against a live engram server with fixture sessions/observations for both directions of the timezone offset (UTC-6 and UTC+1/+2) and for the zero-observation case; go/node toolchains were not available in this environment to run the existing test suite.
📝 WalkthroughWalkthroughThe Claude Code and Codex hooks now parse timezone-less SQLite timestamps as UTC. All three integrations allow save reminders for projects without prior observations while preserving existing age and cooldown checks. ChangesSave nudge fixes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Fixes save-nudge behavior across Engram’s Claude Code, Codex, and OpenCode plugins by making timestamp parsing timezone-safe (for sqlite “naive” UTC timestamps) and by allowing the nudge to fire for projects with zero observations (so the first save can be prompted).
Changes:
- Force UTC parsing (
date -u) for sqlite-style naive timestamps inparse_epoch()in both bash hook implementations. - Treat “no observations yet” as stale (instead of “skip forever”) in both bash hooks by setting
ELAPSEDto a value that will pass the nudge threshold. - Update the OpenCode plugin gating so “no observations yet” no longer short-circuits the nudge.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| plugin/claude-code/scripts/user-prompt-submit.sh | Parses naive sqlite timestamps as UTC; allows nudging when a project has never saved an observation. |
| plugin/codex/scripts/user-prompt-submit.sh | Same fixes mirrored for Codex hook. |
| plugin/opencode/engram.ts | Adjusts “no observations yet” gating so the save-nudge can fire for brand-new projects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // observations can never reach its first one. | ||
| // | ||
| // Only nudge if last save was more than 15 minutes ago (or never). | ||
| if (lastObsEpoch !== 0 && nowSecs - lastObsEpoch < 900) return |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@plugin/claude-code/scripts/user-prompt-submit.sh`:
- Around line 246-262: Move save-nudge eligibility into the core Go API/tool so
adapters no longer calculate first-save staleness or observation age locally. In
plugin/claude-code/scripts/user-prompt-submit.sh lines 246-262 and
plugin/codex/scripts/user-prompt-submit.sh lines 177-193, replace the local
sentinel and elapsed-time decisions with the shared core eligibility call; in
plugin/opencode/engram.ts lines 481-488, consume that same core result instead
of evaluating age locally. Keep each adapter limited to parsing input, invoking
the core API/tool, and returning the result.
In `@plugin/opencode/engram.ts`:
- Around line 481-488: Update the observation-staleness logic around
lastObsEpoch to distinguish “no observation row” from an existing row whose
created_at failed to parse. Track whether an observation exists, and return
without nudging when a row exists but its timestamp is invalid; only treat
lastObsEpoch === 0 as maximally stale when no observation row exists.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e413b3cf-f54c-4a0f-97ba-48a175b83dcb
📒 Files selected for processing (3)
plugin/claude-code/scripts/user-prompt-submit.shplugin/codex/scripts/user-prompt-submit.shplugin/opencode/engram.ts
Comment on lines
+246
to
+262
| NOW_EPOCH=$(date "+%s") | ||
|
|
||
| # Parse last save timestamp and compare to now | ||
| LAST_EPOCH=$(parse_epoch "$LAST_SAVE_AT") | ||
| if [ -z "$LAST_EPOCH" ]; then | ||
| echo "$OUTPUT" | ||
| exit 0 | ||
| if [ -z "$LAST_SAVE_AT" ]; then | ||
| # No observations exist yet for this project. This is the "never saved" | ||
| # case, not "session just started" (session age was already gated to | ||
| # >= 5 minutes above) — treat it as maximally stale so the nudge can fire | ||
| # and break the cycle where a project with zero observations can never | ||
| # reach its first one. | ||
| ELAPSED=901 | ||
| else | ||
| # Parse last save timestamp and compare to now | ||
| LAST_EPOCH=$(parse_epoch "$LAST_SAVE_AT") | ||
| if [ -z "$LAST_EPOCH" ]; then | ||
| echo "$OUTPUT" | ||
| exit 0 | ||
| fi | ||
| ELAPSED=$(( NOW_EPOCH - LAST_EPOCH )) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move save-nudge eligibility into the core Go API/tool.
The adapters independently decide first-save staleness and reminder eligibility. Put this policy in the core so all integrations consume one result and cannot drift.
plugin/claude-code/scripts/user-prompt-submit.sh#L246-L262: replace the local sentinel/elapsed decision with a core eligibility call.plugin/codex/scripts/user-prompt-submit.sh#L177-L193: replace the duplicate local eligibility decision with the same core call.plugin/opencode/engram.ts#L481-L488: consume the core eligibility result instead of evaluating observation age locally.
As per path instructions, adapters stay thin: parse input, call the core Go API/tool, return.
📍 Affects 3 files
plugin/claude-code/scripts/user-prompt-submit.sh#L246-L262(this comment)plugin/codex/scripts/user-prompt-submit.sh#L177-L193plugin/opencode/engram.ts#L481-L488
🤖 Prompt for 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.
In `@plugin/claude-code/scripts/user-prompt-submit.sh` around lines 246 - 262,
Move save-nudge eligibility into the core Go API/tool so adapters no longer
calculate first-save staleness or observation age locally. In
plugin/claude-code/scripts/user-prompt-submit.sh lines 246-262 and
plugin/codex/scripts/user-prompt-submit.sh lines 177-193, replace the local
sentinel and elapsed-time decisions with the shared core eligibility call; in
plugin/opencode/engram.ts lines 481-488, consume that same core result instead
of evaluating age locally. Keep each adapter limited to parsing input, invoking
the core API/tool, and returning the result.
Source: Path instructions
Comment on lines
+481
to
+488
| // No observations exist yet for this project. This is the "never | ||
| // saved" case, not "session just started" (session age was already | ||
| // gated to >= 5 minutes above) — treat it as maximally stale so the | ||
| // nudge can fire and break the cycle where a project with zero | ||
| // observations can never reach its first one. | ||
| // | ||
| // Only nudge if last save was more than 15 minutes ago (or never). | ||
| if (lastObsEpoch !== 0 && nowSecs - lastObsEpoch < 900) return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Distinguish an invalid observation timestamp from no observations.
lastObsEpoch === 0 also means created_at failed to parse. This change treats that as maximally stale and can nudge immediately after a recent but malformed save. Track whether an observation row exists, and skip the nudge when its timestamp is invalid.
🤖 Prompt for 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.
In `@plugin/opencode/engram.ts` around lines 481 - 488, Update the
observation-staleness logic around lastObsEpoch to distinguish “no observation
row” from an existing row whose created_at failed to parse. Track whether an
observation exists, and return without nudging when a row exists but its
timestamp is invalid; only treat lastObsEpoch === 0 as maximally stale when no
observation row exists.
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
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.
🔗 Linked Issue
Closes #668
🏷️ PR Type
type:bug— Bug fix📝 Summary
date -uon every naive-timestamp parse path inparse_epoch()(claude-code and codexuser-prompt-submit.sh), since sqlite'sdatetime('now')timestamps are always UTC with no zone marker — parsing them as local time skewed the save-nudge's elapsed-time math by the host's UTC offset (nudge never fires on negative-offset hosts, fires too eagerly on positive-offset hosts).📂 Changes
plugin/claude-code/scripts/user-prompt-submit.sh-uto the naive-timestamp fallback parses inparse_epoch(); restructure the no-prior-observation branch to setELAPSED=901instead of exiting without a nudgeplugin/codex/scripts/user-prompt-submit.shplugin/opencode/engram.tslastObsEpoch === 0) to nudge instead of skip; this implementation's timestamp normalization was already UTC-correct🧪 Test Plan
go test ./...— not run; no Go toolchain available in the environment I built this fix ingo test -tags e2e ./internal/server/...— not run, same reasonManual verification against a live
engram serveinstance, using real sessions/observations created and torn down through the HTTP API (no residual data left behind — verified viaengram statsbefore/after):TZ=America/<UTC-6>and underTZ=Europe/Madrid(UTC+1/+2) — confirmed the epoch comes out skewed in opposite directions for each, matching the reported symptom (never nudges on the UTC-6 host; would over-nudge on the Europe host).plugin/opencode/engram.tsin isolation withtsc --noEmit— no syntax errors introduced (remaining errors are pre-existing missing ambient types forBun/@opencode-ai/plugin, unrelated to this change and present before it too).I did not have write access to add the
type:buglabel myself — happy to have a maintainer apply it.Summary by CodeRabbit