feat(skill): port debrief to a Claude Code Skill (zero-install)#12
feat(skill): port debrief to a Claude Code Skill (zero-install)#12cloudprobe wants to merge 3 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds documentation and test infrastructure for a Claude Code "debrief" skill that generates daily standups from local git commits and session logs. It includes a decision record with archival criteria, sample output specifications, skill behavior definition, and improves git test robustness through environment variable sanitization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
ecd0837 to
4f522f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/debrief/SKILL.md`:
- Around line 49-58: Add an explicit normalization step for comparing the
resolved local [start, end] range with JSONL record timestamps: convert the
resolved local [start, end] to UTC (or convert both sides to Unix epoch seconds)
before comparing against the JSONL "timestamp" (ISO-8601 Z) so comparisons are
timezone-agnostic; update the "Resolve the time range" section or prepend the
"Compare assistant records" section with one line stating to perform this
conversion and that comparisons must use epoch seconds rather than string
equality.
- Line 242: The current clipboard step uses the literal command pattern printf
'%s' "<output>" | pbcopy (and the xclip variant) which interpolates special
chars and is unsafe; change the implementation to avoid shell interpolation by
writing the rendered output to a safe temporary file or using a here-doc with
the payload provided via an environment variable (e.g., bind output to OUTPUT)
and then feed that file/stream to pbcopy or xclip, and keep the one-line
stderr-style confirmation message "Copied to clipboard." unchanged; update any
script invocation that references printf '%s' "<output>" | pbcopy or xclip to
the tempfile/here-doc approach to ensure characters like ", $, `, and \ are not
reinterpreted by the shell.
- Line 4: Update the frontmatter allowed-tools to include Bash(env *),
Bash(xclip *), and Bash(wl-copy *) so the procedure's env/git invocation and
Linux/Wayland clipboard commands are permitted; change the timestamp handling in
the procedure (references: the Step 1 local [start,end] conversion and Step 4
JSONL filtering against timestamp fields) to convert both the user [start,end]
and each JSONL record's ISO-8601 "timestamp" to UTC Unix epoch seconds before
doing comparisons; and improve the clipboard output escaping (reference the
printf '| pbcopy' usage around Line 242) by explicitly escaping or serializing
the output (e.g., use a robust shell-escaping path such as producing a
JSON/string-escaped value via jq `@json` or using printf '%q' on the output) so
embedded quotes, dollar signs, and backticks cannot break the constructed shell
commands.
🪄 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: 39075673-ba61-44ca-9269-f237b0a2d27d
📒 Files selected for processing (4)
docs/decision-2026-04-21.mdexamples/sample-output.mdinternal/collector/git_test.goskills/debrief/SKILL.md
Three pre-existing issues in git_test.go surfaced when running the tests under a global gitconfig (init.defaultBranch=main, a global core.hooksPath) and under a pre-commit hook context: 1. checkout -b main fails because git init already created main. Use -B (create-or-reset) so the test works both with and without init.defaultBranch=main. 2. Temp repo inherits core.hooksPath from ~/.gitconfig, so developer-machine pre-commit/commit-msg hooks run during the test. A global min-length commit-msg hook rejected "add tests" (9 chars). Set core.hooksPath=/dev/null inside the temp repo to suppress all inherited hooks. 3. When the test runs inside a git hook (pre-commit), git sets GIT_DIR / GIT_WORK_TREE / GIT_INDEX_FILE in the environment. Go subprocesses inherit them, so both "git -C <tempdir>" in the test helper and the production collector's git calls silently operate on the outer worktree instead of the temp repo. Strip GIT_* from the helper's subprocess env (cleanGitEnv), and unset the process-level hook vars with restore-on-cleanup (unsetGitHookVars) so Collect() also sees a clean env. Same fixes applied to TestGitCollector_CollectFromTempRepo and TestGitCollector_MultiDayCommitsSplitByDay.
Ships skills/debrief/SKILL.md as the primary distribution for debrief. Zero-install for Claude Code users (drop the dir under ~/.claude/skills or install via /plugin). Preserves the wedge from the Go CLI verbatim: local git commits + ~/.claude/projects/*.jsonl session logs, classified into four buckets (Decided / Shipped / Investigated / Watch). No network calls. Rules ported directly from the Go source so behavior matches: - Commit classifier rules from internal/synthesizer (feat/fix/perf/ refactor → Shipped; PR-squash chore/test with substantive body → Shipped; etc.) - Session-note filter heuristics from internal/collector/claude.go (planning-prefix rejection, conversational rejection, <40 char drop, action-verb requirement) - Output ordering + Quiet-day fallback from internal/synthesizer Also includes examples/sample-output.md — five reference outputs (four-bucket day, chore-only quiet day, empty day, Slack format, week range) plus a tuning rubric for dogfooding. The Go CLI remains on main for now. Phase 4 (archive at v1.1.0-cli-final, rewrite README, strip Go source from main) is gated on Phase 2 dogfood validating that the skill replaces the CLI.
4f522f3 to
1785736
Compare
Three fixes from the PR #12 review: 1. allowed-tools frontmatter now uses the documented "Bash(cmd:*)" glob syntax (colon, not space). Previously "Bash(git *)" wouldn't match under Claude Code's permission pattern parser. Also adds env and xclip entries since the skill references them in §3 and §8. 2. Spell out timezone normalization between §1 (local [start, end]) and §4 (JSONL timestamp, UTC). Without the rule, a "today" invocation near midnight pulls in the wrong day. Mandate Unix epoch seconds comparison on both sides, never string-compare ISO forms. 3. Harden the clipboard step against commit subjects containing ", \$, backtick, or \\. The "printf '%s' \"<output>\" | pbcopy" form interpolates these through the shell — realistic in commit messages like "fix: handle \$HOME expansion". Switch to tempfile + pbcopy/ xclip stdin redirect so the payload never passes through argv.
Summary
skills/debrief/SKILL.mdas the primary distribution — zero-install for Claude Code users. Preserves the wedge from the Go CLI (local git +~/.claude/projects/*.jsonl+ 4-bucket Decided/Shipped/Investigated/Watch classification). Rules ported directly frominternal/synthesizerandinternal/collector/claude.goso behavior matches.examples/sample-output.md(five reference outputs + dogfood tuning rubric).git_test.gobugs that only surfaced under a global gitconfig + pre-commit hook context:checkout -b mainvsinit.defaultBranch=main, inheritedcore.hooksPathtriggering global commit-msg hook, and hook-injectedGIT_DIR/GIT_WORK_TREEredirecting subprocess git calls to the outer worktree.The Go CLI stays on
mainfor now. Phase 4 (archive atv1.1.0-cli-final, rewrite README, strip Go source) is gated on Phase 2 dogfooding showing the skill replaces the CLI.Test plan
go test -race -count=1 ./...— passes cleanlyGIT_DIR=/tmp/bogus GIT_WORK_TREE=/tmp/bogus go test -race -run TestGitCollector ./internal/collector/...— simulated hook env, passesln -s \$(pwd)/skills/debrief ~/.claude/skills/debrief, invoke in a real Claude session on a real repo + Claude history. Verify four buckets populate, empty sections skip, and chore-only days produce the "Quiet day" fallback (not "No activity").🤖 Generated with Claude Code