Repository navigation
fix(hooks): session-log の分割警告の上限を 600 行へ引き上げる - #304
Conversation
session-log.md の分割警告は 500 行で出るが、`/maintenance` が実際に 退避できるのは「直近 30 日より古いエントリ」だけ。全エントリが 30 日 以内に収まっていると、警告は出るのに移動対象が 0 件という状態になる。 当リポジトリは 520 行 / 全 20 エントリが 30 日以内で、まさにその状態 だった。行数だけを見て退避すると保持ルール違反になるため、警告に従う と規約を破ることになる。 上限は読みやすさの目安であって、保持期間 30 日のような守りの強さを 持つ値ではない。よって噛み合わない箇所は上限側で解消する。保持期間は 直近の作業履歴を本体に残す下限として 30 日のまま維持する。 定義は 4 箇所にあり、すべて同時に更新した: - go/internal/hookhandler/auto_cleanup_hook.go (defaultSessionLogMaxLines) - scripts/auto-cleanup-hook.sh - templates/hooks/auto-cleanup-hook.sh - skills/maintenance/references/cleanup.md (閾値表 + 判断根拠の注記) 稼働している hook は Go 実装 (bin/harness hook auto-cleanup) のため、 drift gate と同一条件で 4 プラットフォームのバイナリを再生成した。 skill mirror (codex / opencode) も同期済み。 検証 (hook に stdin で payload を渡して判定を直接観測): - 新バイナリ 520 行 -> 警告なし / 601 行 -> 警告あり (limit: 600 と表示) - 旧バイナリ 520 行 -> 警告あり (limit: 500) - bash scripts/ci/check-binary-source-drift.sh -> OK - go test ./internal/hookhandler/... -> ok - bash tests/validate-plugin.sh -> 134 合格 0 失敗 - bash scripts/ci/check-consistency.sh -> 24/24 合格 - mirror verify -> in-sync (0 drift) - VERSION / plugin.json / harness.toml は非接触 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ZBxNEtYJbtHkZcsAn8nsv
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
Changessession-log.md 分割閾値
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@skills/maintenance/references/cleanup.md`:
- Line 10: Update the session-log procedure’s Line 80 threshold from “over 500
lines” to “over 600 lines” in skills/maintenance/references/cleanup.md (the
canonical source), then synchronize the same content to
codex/.codex/skills/maintenance/references/cleanup.md and
opencode/skills/maintenance/references/cleanup.md.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c13a02d3-eb13-4eea-99d7-914565adf3a7
⛔ Files ignored due to path filters (1)
bin/harness-windows-amd64.exeis excluded by!**/*.exe
📒 Files selected for processing (10)
CHANGELOG.mdbin/harness-darwin-amd64bin/harness-darwin-arm64bin/harness-linux-amd64codex/.codex/skills/maintenance/references/cleanup.mdgo/internal/hookhandler/auto_cleanup_hook.goopencode/skills/maintenance/references/cleanup.mdscripts/auto-cleanup-hook.shskills/maintenance/references/cleanup.mdtemplates/hooks/auto-cleanup-hook.sh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e5ef82a76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ( | ||
| defaultPlansMaxLines = 200 | ||
| defaultSessionLogMaxLines = 500 | ||
| defaultSessionLogMaxLines = 600 |
There was a problem hiding this comment.
Raising this constant only hides the reported mismatch for the current 520-line file; a session log with 601 or more lines whose entries are all less than 30 days old still triggers checkSessionLog, while /maintenance is required to retain every entry. The same persistent, unactionable warning therefore returns under a sufficiently active 30-day period. Base the warning on the presence of at least one archivable entry, rather than moving the line-only cutoff.
Useful? React with 👍 / 👎.
| |------|---------|-------| | ||
| | `PLANS_MAX_LINES` | 200 | `scripts/auto-cleanup-hook.sh` | | ||
| | `SESSION_LOG_MAX_LINES` | 500 | 同上 | | ||
| | `SESSION_LOG_MAX_LINES` | 600 | 同上 | |
There was a problem hiding this comment.
The threshold table now says 600, but the session-log section in this same reference still says splitting is recommended above 500 lines (line 80). Since the maintenance skill explicitly loads this file for its thresholds and procedure, users handling a 501–600-line log receive contradictory guidance and can encounter the exact no-op cleanup scenario this change intends to eliminate. Update that prose, along with the Codex and OpenCode mirrors, to 600.
Useful? React with 👍 / 👎.
閾値表 (Line 10) だけを 600 に更新し、session-log 節の手順 (Line 80) が 「500行超で分割推奨」のまま残っていた。同じ資料の中で表と手順が異なる 値を示す状態だったため揃える。codex / opencode の mirror も同期。 あわせて、上限の引き上げでは同型の不一致が消えないことを既知の限界と して明記した。この警告は行数だけを見ており、退避条件 (直近 30 日より 古い) を満たすエントリが実在するかは判定しない。したがって 600 行超で 全エントリが 30 日以内なら、やはり移動対象 0 件で警告だけが出る。 恒久的な解消には警告の発火条件を「退避可能なエントリが 1 件以上ある」 へ変える必要があるが、本 PR では実施しない (承認された変更は上限の 引き上げであり、hook の判定ロジック変更は別途の判断を要する)。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ZBxNEtYJbtHkZcsAn8nsv
レビュー指摘への対応対応したもの手順側の閾値表記が 500 のまま残っていた — 有効な指摘です。閾値表 (Line 10) だけを更新し、session-log 節の手順 (Line 80) が「500行超で分割推奨」のままでした。同じ資料の中で表と手順が異なる値を示す状態だったため揃え、 指摘は正しいが、本 PR では実施しないもの「閾値を上げるだけでは同型の不一致が残る」 — そのとおりです。この警告は行数だけを見ており、退避条件 (直近 30 日より古いこと) を満たすエントリが実在するかは判定していません。したがって 601 行を超えていて全エントリが 30 日以内なら、やはり移動対象 0 件で警告だけが出ます。上限の引き上げは不一致が起きる位置をずらすだけで、種類としては残ります。 恒久的な解消には、警告の発火条件を「退避可能なエントリが 1 件以上ある」へ変える必要があります。これは hook の判定ロジックの変更で、session-log の日付ヘッダを解析する実装とそのテストが新たに必要になり、バイナリの再生成も伴います。本 PR で承認されている変更は上限の引き上げまでであり、判定ロジックの変更は別途の判断を要するため実施しません。 代わりに、この限界を 同じ構造は |
1 participant
/maintenanceの実行中に見つかった、警告と規約の不一致を解消します。何が起きていたか
session-log.mdの分割警告は 500 行で出ます。一方、/maintenanceが実際に退避できるのは「直近 30 日より古いエントリ」だけです。この 2 つが噛み合っていません。全エントリが 30 日以内に収まっていると、警告は出るのに移動対象が 1 件も無い状態になります。当リポジトリはまさにその状態でした。
session-log.mdの行数行数だけを見て退避すると保持ルールを破ることになるため、警告に従うと規約違反になります。逆に従わなければ、警告は毎セッション出続けます。
どちらを動かすか
上限は読みやすさの目安であり、保持期間 30 日のように守りの強さを持つ値ではありません。したがって不一致は上限側で解消します。保持期間は、直近の作業履歴を本体に残すための下限として 30 日のまま維持します。
判断の根拠は
skills/maintenance/references/cleanup.mdの閾値表に注記として残しました。同じ不一致を将来また踏んだとき、どちらを動かすべきかが表のそばで読めます。SESSION_LOG_MAX_LINESの既定値limit: 600)環境変数
SESSION_LOG_MAX_LINESによる上書きは従来どおり有効です。定義は 4 箇所にあります
閾値の定義は 1 箇所ではなく、すべて同時に更新しています。
go/internal/hookhandler/auto_cleanup_hook.go(defaultSessionLogMaxLines)scripts/auto-cleanup-hook.shtemplates/hooks/auto-cleanup-hook.shskills/maintenance/references/cleanup.md(閾値表 + 判断根拠の注記)実際に稼働している hook は Go 実装 (
bin/harness hook auto-cleanup) です。閾値は env のみで解決されるため、既定値の変更を実効化するには同梱バイナリの再生成が必要でした。drift gate と同一条件 (-trimpath/-buildvcs=false/GOTOOLCHAINpin) で 4 プラットフォームを再生成しています。skill mirror (codex / opencode) も同期済みです。検証
判定は hook に stdin で payload を渡し、出力を直接観測して確認しました。
limit: 600)origin/main)limit: 500)bash scripts/ci/check-binary-source-drift.sh→ OK (cherry-pick 後にも再確認)go test ./internal/hookhandler/...→ okbash tests/validate-plugin.sh→ 134 合格 0 失敗bash scripts/ci/check-consistency.sh→ 24/24 合格VERSION/.claude-plugin/plugin.json/harness.tomlは非接触なお既存テスト
TestAutoCleanupHandler_SessionLog_OverThresholdはSessionLogMaxLines: 500を構造体に明示指定しているため、既定値の変更では期待値が変わりません (テストの弱体化は発生していません)。🤖 Generated with Claude Code
https://claude.ai/code/session_012ZBxNEtYJbtHkZcsAn8nsv
Summary by CodeRabbit