Repository navigation
Tighten protected path coverage and relax npm auto-approval - #1
Closed
Chachamaru127 wants to merge 1 commit into
Closed
Chachamaru127 wants to merge 1 commit into
Chachamaru127 wants to merge 1 commit into
Conversation
ウォークスルーJS/TS テストコマンド認識の正規表現パターンを拡張し、許可判定の受け入れ条件を広げました。また、.git ディレクトリ、.gitignore、.gitattributes を保護対象パスとして追加し、git 関連ファイルの処理制御を強化しました。 変更内容
コード レビュー予想所要時間🎯 2 (シンプル) | ⏱️ ~10 分
詩
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (2)
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 |
Chachamaru127
pushed a commit
that referenced
this pull request
Apr 10, 2026
The harness sync command was silently dropping the "skills": ["./"] field from plugin.json on every sync, because the pluginJSON struct in go/cmd/harness/sync.go had no Skills field. This caused a constant auto-revert loop: Phase 38.2.1 added the field manually, harness sync removed it, we restored it with git checkout, harness sync removed it again, and so on. Root cause analysis (from yesterday's review #1): - CC 2.1.94+: "skills": ["./"] tells CC to use each SKILL.md frontmatter `name` field as the invocation name (enabling HAR:* short-name aliases while keeping directory names unchanged) - Phase 38.2.1 added the field to plugin.json manually - generatePluginJSON() regenerates plugin.json from scratch using only fields defined in pluginJSON struct - pluginJSON struct never had Skills → always dropped on sync - Result: HAR:* invocation worked *sometimes* (when the field was present) but was silently broken whenever harness sync ran Fix (3 lines): - Add `Skills []string` field to pluginJSON struct with json tag "skills,omitempty" - Hardcode []string{"./"} in generatePluginJSON (no need for harness.toml config field — this is always the correct value for this plugin layout) - Add assertion in TestSync_GeneratesPluginJSON to prevent regression Verification: - go test ./cmd/harness/ -run TestSync_GeneratesPluginJSON → PASS - go test ./... → all 12 packages PASS - ./bin/harness-darwin-arm64 sync . → plugin.json contains "skills": ["./"] (verified via jq) - 3 platform binaries rebuilt and re-verified Closes review finding #1 (Claude Sonnet 4.6 review, Apr 11 2026). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7 tasks
kimseunghyun-kr
added a commit
to kimseunghyun-kr/claude_team_harness
that referenced
this pull request
May 15, 2026
kimseunghyun-kr
added a commit
to kimseunghyun-kr/claude_team_harness
that referenced
this pull request
May 15, 2026
…g additions Foundation for the per-persona reasoning branch model (Fix Chachamaru127#1). No behavior change yet for users — phases 3b-3i wire this in next. lib/state.py: - worktree_path() / reasoning_branch_name() / worktree_create() / worktree_remove() / list_persona_worktrees() — per-spawn worktree lifecycle - audit_orphan_worktrees() — removes prior-epoch or closed-epoch worktrees, logs each removal to gc.log (crash recovery without manual intervention) - reasoning_commit_message() — fixed structured commit message format 'reason(deliberation): <persona> epoch-N slot-S <bid|write>' - capture_refs() — ref snapshot helper for isolation check lib/postcheck.py: - branch_isolation_check() — given pre/post ref snapshots and persona id, flags modifications to refs outside refs/heads/persona/<id>/ as 'branch-isolation-violated' lib/config.py: - DeliberationConfig extended with extraction_model, extraction_temperature, spawn_mode, eavesdrop_enabled, eavesdrop_probability, gc_keep_epochs. Defaults preserve current behavior. harness.toml [deliberation]: - 6 new keys with safe defaults (eavesdrop off, extraction_temp=0.0, spawn_mode=subagent, gc_keep_epochs=3) tests/test-deliberation-worktrees.sh (new, 7 assertions): - worktree create/remove on expected branch - orphan audit removes prior-epoch worktrees + logs to gc.log - orphan audit removes closed-epoch worktrees - branch_isolation_check accepts own branch / rejects main / rejects foreign - reasoning_commit_message format pin
Chachamaru127
pushed a commit
that referenced
this pull request
May 15, 2026
Address Codex review feedback and the actionlint job failure. actionlint job - Drop `docker://rhysd/actionlint:1.7.12` (mutable Docker tag) and install via `go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.12` instead. Go module proxy resolves @Version to a content-addressed cache entry, providing the same supply-chain immutability as a SHA-pinned action without depending on a Docker registry tag (Codex P2 #2). Reuses the existing setup-go-harness composite. smoke-install doctor - Add `${GITHUB_WORKSPACE}/bin` to GITHUB_PATH after building the harness binary so `harness` resolves bare. This makes doctor's "bin/harness in PATH" check pass on its own merit instead of relying on a fallback that swallowed every doctor exit (Codex P2 #1). Removed the `|| { warning; exit 0 }` block so a real health regression now blocks the smoke gate.
Chachamaru127
pushed a commit
that referenced
this pull request
May 15, 2026
Address Codex review feedback and the actionlint job failure. actionlint job - Drop `docker://rhysd/actionlint:1.7.12` (mutable Docker tag) and install via `go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.12` instead. Go module proxy resolves @Version to a content-addressed cache entry, providing the same supply-chain immutability as a SHA-pinned action without depending on a Docker registry tag (Codex P2 #2). Reuses the existing setup-go-harness composite. smoke-install doctor - Add `${GITHUB_WORKSPACE}/bin` to GITHUB_PATH after building the harness binary so `harness` resolves bare. This makes doctor's "bin/harness in PATH" check pass on its own merit instead of relying on a fallback that swallowed every doctor exit (Codex P2 #1). Removed the `|| { warning; exit 0 }` block so a real health regression now blocks the smoke gate.
Chachamaru127
added a commit
that referenced
this pull request
May 31, 2026
…iew (Phase 85.1.4) The Phase 85 Workflow adversarial review surfaced multiple gaps in the initial lease implementation (e3395915). The verify pipeline could not be trusted (32/33 sub-agents failed schema), so Lead manually triaged the three perspective summaries (security/concurrency/skeptic) and confirmed each substantive finding against the on-disk code. Confirmed and fixed 1. Slow-path race (CRITICAL, security+concurrency): two concurrent reclaimers could both pass isStale and both rename, with the second silently renaming the first's freshly-acquired lock away. POSIX rename cannot serialize this because it takes whatever is currently at the path. Fix: per-store mkdir-based reclaim mutex (the cross-platform POSIX atomic primitive) + re-validate staleness inside the mutex. Pinned by TestLeaseReclaim_ConcurrentSlowPath (16 goroutines under -race, count=20, all green). 2. writeLockAtomic visibility race (CRITICAL, follow-on to #1): under O_CREAT|O_EXCL + Write, a peer reading mid-create observed an empty file -> "corrupted" -> reclaim recovery -> rename in-flight lock away -> peer wins. Switch to CreateTemp + Write + os.Link (POSIX link(2) atomic create-only). The visible path now only ever observes complete data. 3. Empty active.json downgrades safety silently (MAJOR, skeptic+security): {} parsed as a non-nil empty map made every session id appear "dead", which combined with TTL expiry would falsely reclaim a healthy peer's lock. LoadLiveSessionsFromActiveJSON returns nil for empty input now, which makes isStale fall back to documented TTL-only semantics. Pinned by TestLeaseStaleness_EmptyActiveJsonFallsBack. 4. World-readable mode (MAJOR, security): 0o644 lock files leaked "user X is editing file Y" to other unix users on shared hosts. Lock files now 0o600, leases directory now 0o700. Pinned by TestLeaseLock_FileMode. 5. Default TTL too short for heavy edit cycles (MAJOR, skeptic): doubled defaultLeaseTTL from 30 min to 60 min to cover the realistic floor of breezing Worker test+lint+build cycles on large files. 6. HolderPID dead code (MINOR, concurrency): documented as debug-only metadata. Session id is the single identity authority; crash-restart with same session id is legitimate refresh. Corrupted-path holder reporting: when a peer completes a valid write in the window between our outer readLock and our mutex acquire, surface peer's identity so the caller returns HeldByOther instead of the less- specific Unavailable. Out of scope (spec.md gates, not fixed here) - O_CREAT|O_EXCL on NFS/SMB/FUSE (Local FS only is the contract) - fsync parent directory (single-user dev, fail-open recovery is OK) Test coverage: 20 iterations x 15 tests x -race = 300 runs, all PASS in 5.4s. Three new tests added; all 12 previous tests still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Testing
Codex Task
Summary by CodeRabbit
リリースノート
✏️ Tip: You can customize this high-level summary in your review settings.