feat(log): per-line cwd + source-aware emit label#33
Merged
Conversation
Two annoyances surfaced when dogfooding v0.2.0 across multiple
projects:
1. Lines in the global `~/.context-revive/hook.log` did not carry
any project context. With revive wired in 5+ repos, the same
`post-compact: forcing emit` message could be from any of them
— un-debuggable.
2. The forcing-emit log line said `post-compact:` even when the
trigger was `/clear`, because both paths shared one signal file
and `cadence_gate` had no idea which command wrote it.
Fixes:
- `log()` now prepends `[<cwd>]` after the timestamp, with a
tilde substitution for paths under $HOME. A subtle bash gotcha:
`${VAR/old/\~}` leaks the backslash; bash does NOT tilde-expand
inside parameter-substitution replacements, so the literal `~`
is correct (and was wrong on my first attempt — caught in
smoke before commit).
- `cmd_mark_compact` and `cmd_mark_clear` write `compact` / `clear`
to the signal file (was: a unix timestamp, never read). The
signal filename stays `revive-compact.signal` to keep the
on-disk path stable across upgrades.
- `cadence_gate` reads the payload, sanitizes via
`tr -cd '[:lower:]'`, and emits either `post-compact:` or
`post-clear:` accordingly. Unknown / malformed content falls
back to a neutral `post-context-loss:` label.
4 new tests cover: cwd field present in log, compact label,
clear label, malformed payload → neutral label without crashing.
The inline `[[ "$output" =~ \[[^]]+\]\ \[[^]]+\]\ post-compact ]]` passed on macOS bats but failed on Ubuntu bats in CI — bash 3.2 vs 5.x differ on how literal `\[` inside the pattern is parsed. The canonical workaround is to bind the pattern to a local variable and reference it as `$re`. No content change to the assertion.
There was a problem hiding this comment.
Pull request overview
This PR improves revive hook logging ergonomics when used across multiple repositories by enriching each log line with a per-line working directory and by making the “forcing emit” label reflect whether the trigger was a compact or a /clear.
Changes:
- Add a per-line
[cwd]field to~/.context-revive/hook.logoutput, with$HOMEpaths shortened to~. - Encode the trigger source (
compactvsclear) in the signal file payload and use it to emit a source-awarepost-…: forcing emitlog label (fallback topost-context-loss). - Add Bats coverage for the new log format and source-aware labels, including malformed payload handling.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
bin/revive |
Enriches log() output with cwd, and makes cadence_gate() log label depend on sanitized signal payload (compact/clear/fallback). |
tests/revive.bats |
Adds tests validating the new [cwd] log field and the compact/clear/neutral forcing-emit labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| @test "refresh log message reflects compact source" { | ||
| printf 'compact\n' > .claude/revive-compact.signal 2>/dev/null || mkdir -p .claude && printf 'compact\n' > .claude/revive-compact.signal |
There was a problem hiding this comment.
This test uses printf ... || mkdir -p .claude && printf ... which is hard to reason about due to &&/|| precedence, and it can still print a redirection error when .claude/ is missing (the failure happens before 2>/dev/null is applied). Prefer an explicit mkdir -p .claude first, then a single printf write.
Suggested change
| printf 'compact\n' > .claude/revive-compact.signal 2>/dev/null || mkdir -p .claude && printf 'compact\n' > .claude/revive-compact.signal | |
| mkdir -p .claude | |
| printf 'compact\n' > .claude/revive-compact.signal |
`${PWD/#$HOME/~}` did a string-prefix rewrite without a path
boundary check, so a checkout in `$HOME-something/...` (e.g.
HOME=/Users/alice and PWD=/Users/alice-work/project) would log
as `~-work/project` — wrong project name in a feature that
exists specifically to disambiguate projects.
Replace with a case statement that fires only on the exact
$HOME match or the `$HOME/` prefix; falls through to the
absolute path otherwise.
New test pins this: stages a sibling worktree at `$WORKDIR-other`
with HOME=$WORKDIR, runs mark-compact, and asserts the log line
does NOT contain the `~-other` splice artefact.
Replaced the convoluted `printf > .claude/... 2>/dev/null || mkdir -p .claude && printf > .claude/...` with the same two-line pattern the sibling clear-source and garbage-payload tests already use. Bash redirect failures land on the original stderr before `2>/dev/null` is applied, so the old form would have leaked an error noise even when functionally correct.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
Two small annoyances surfaced while dogfooding v0.2.0 across multiple projects:
1. Per-line
[cwd]field in the log~/.context-revive/hook.logis global. With revive wired in 5+ repos, the samepost-compact: forcing emitline could be from any of them — un-debuggable. Now:$HOME-prefixed paths get tilde-shortened so the message body still has room.2. Source-aware forcing-emit label
The forcing-emit log line said
post-compact:even when triggered by/clear, because both paths shared one signal file andcadence_gatehad no idea which command wrote it.mark-compactandmark-clearnow writecompact/clearto the signal payload (was: a never-read unix timestamp).cadence_gatereads, sanitizes viatr -cd '[:lower:]', and emits the matching label. Unknown / malformed content falls back to a neutralpost-context-loss:label rather than crashing.The signal filename stays
revive-compact.signalto keep the on-disk path stable across upgrades.Bash gotcha worth noting
${VAR/old/\~}leaks the backslash — bash does NOT tilde-expand inside parameter-substitution replacements, so the literal~is correct (caught in smoke test before commit).Test plan
bats tests/— 143/143 passshellcheck bin/revive install.shclean[~/Projects/context-revive] post-clear: forcing emit (counter=74)aftermark-clear; refresh🤖 Generated with Claude Code