feat(harness): 앱 관리 하네스 동기화 + 컴포넌트 토글#44
Conversation
~/.claude 하네스를 repo 소스 블록과 정렬하는 apply primitive의 순수 부분. fs/Tauri 의존 없이 마커 안전성·멱등성·사용자 콘텐츠 보존을 단위테스트로 봉쇄. - inject_block/remove_block: 마커 사이만 교체/제거 (install.sh inject_marker Rust 포팅) - block_version/needs_update: VERSION 게이트 - compute_claude_md: enable=주입·disable=제거, 멱등 - HarnessConfig: 컴포넌트별 on/off, ~/.config/pharos/harness.json (기본 전부 on) - 단위테스트 11개 (멱등·마커밖 보존·stale 교체·disable 제거·config 라운드트립) IO/Tauri command·런치훅·UI는 T2~T5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
순수 코어를 감싸 파일시스템/설정/커맨드를 붙인다. - repo_dir(PHAROS_DIR or ~/pharos)·claude_md_path·config_path - HarnessConfig 컴포넌트별 enabled 조회/설정 + ComponentStatus - apply_harness: 활성 블록 주입/비활성 제거(원자적 쓰기), 유닛 심링크 보장/제거(install.sh link() 시맨틱 재현 — 실제 파일/디렉터리는 백업) - harness_status/sync_harness/set_component_enabled 커맨드(spawn_blocking) - lib.rs generate_handler! 등록 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lib.rs .setup()에서 daemon ensure 직후 spawn_blocking로 자동 적용. - needs_sync: 활성 블록 버전 불일치/누락, 비활성 블록 잔존, 유닛 심링크 존재 여부 불일치 중 하나라도 있으면 동기화 필요로 판정 - auto_sync_on_launch: 드리프트 있을 때만 apply_harness 호출(정상이면 no-op), repo 없으면 조용히 skip, 시작을 막지 않음 - T2에서 needs_update에 달았던 임시 allow(dead_code) 제거(이제 사용됨) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
마운트 시 harness_status로 컴포넌트 상태 로드. - 컴포넌트별 행: 이름 + 버전(블록 applied→source) 또는 링크 상태(유닛) + on/off 토글, 토글 시 set_component_enabled 후 상태 갱신 - "지금 재동기화" 버튼 → sync_harness - 상단 고지문: 마커 안쪽은 자동 관리, 직접 편집 금지 - 기존 ShellIntegrationSection 스타일(settings-row/switch) 매칭 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
워커가 순수 코어만 테스트하고 IO 레이어(apply_harness/symlink/원자쓰기)는 미검증이었던 갭을 닫는다. temp HOME+repo로 실파일 동작 입증: - 블록 주입 시 마커 밖 사용자 콘텐츠 보존 - 심링크 backup 가드: 사용자 실디렉터리는 .pre-pharos-*로 백업, 삭제 안 함 - build_status 버전/링크 상태 정확 - needs_sync 적용 후 수렴(런치훅 no-op) - fable 비활성 → 재적용 시 해당 블록만 제거, 나머지·사용자 콘텐츠 유지 harness 테스트 11 → 12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
install.sh 5~6단계는 최초 부트스트랩으로 유지하되, 이후 동기화는 GUI 앱이 담당함을 명시(런치훅 version-gate 재적용 + Settings→Harness 토글). 하네스 업데이트에 install.sh 재실행 불필요 — 앱 재시작이면 됨. CLI-only는 여전히 스크립트 의존. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 38 minutes and 39 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughAdds a complete harness synchronization system to the Tauri app. A new ChangesHarness Sync System
Sequence Diagram(s)sequenceDiagram
participant HarnessSection
participant harness_status
participant set_component_enabled
participant sync_harness
participant apply_harness
HarnessSection->>harness_status: invoke on mount
harness_status-->>HarnessSection: Vec~ComponentStatus~
HarnessSection->>set_component_enabled: toggle component
set_component_enabled->>apply_harness: update config and apply
apply_harness-->>set_component_enabled: ok
set_component_enabled-->>HarnessSection: Vec~ComponentStatus~
HarnessSection->>sync_harness: manual resync
sync_harness->>apply_harness: apply harness
apply_harness-->>sync_harness: ok
sync_harness-->>HarnessSection: Vec~ComponentStatus~
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src-tauri/src/harness.rs (2)
96-109: 💤 Low valueVersion parsing truncates at hyphen, breaking prerelease versions.
The
take_whilecondition stops at-, which means semver prerelease versions like1.0.0-beta.1would be parsed as1.0.0. If prerelease versions are used in the source blocks, version comparison will be incorrect.If prerelease versions are not expected, this is fine. Otherwise, stop only at whitespace and let the
-->be handled by the subsequent whitespace:♻️ Fix if prerelease versions are needed
let v: String = text[start..] .chars() - .take_while(|c| !c.is_whitespace() && *c != '-') + .take_while(|c| !c.is_whitespace()) .collect();🤖 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 `@src-tauri/src/harness.rs` around lines 96 - 109, The block_version function truncates version strings at the hyphen character, which breaks semver prerelease versions like 1.0.0-beta.1. To fix this, modify the take_while condition in the block_version function to only check for whitespace characters and remove the check for the hyphen character. The closing comment marker --> will be naturally handled by the whitespace condition, so the character check for *c != '-' should be removed from the logical condition.
539-593: ⚖️ Poor tradeoffTest mutates global environment variables, risking parallel test interference.
Modifying
HOMEandPHAROS_DIRaffects the entire process. Rust tests run in parallel by default, so concurrent tests reading these env vars could observe incorrect values. While the comment notes "no other test reads those," this is fragile as the codebase grows.Consider using
#[serial]fromserial_testcrate, or restructuring helpers to accept paths as parameters for testability.🤖 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 `@src-tauri/src/harness.rs` around lines 539 - 593, The test is mutating global environment variables HOME and PHAROS_DIR via std::env::set_var, which can cause interference when tests run in parallel since Rust tests run concurrently by default. To fix this, add the #[serial] attribute from the serial_test crate to the test function containing this code (the function with the apply_harness() call and subsequent assertions). This will ensure the test runs serially and prevent other tests from observing incorrect environment variable values during execution. Additionally, make sure the serial_test crate is added to the dev-dependencies in Cargo.toml if not already present.
🤖 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 `@src-tauri/src/harness.rs`:
- Around line 193-195: The home_dir() function returns an empty PathBuf when the
HOME environment variable is unset, which causes subsequent path joins to
produce relative paths and write files to the current working directory instead
of the user's home directory. Replace the current implementation that uses
std::env::var("HOME").map(PathBuf::from).unwrap_or_default() with the dirs
crate's home_dir() function, which provides platform-specific fallbacks and
handles the HOME variable more robustly. If the dirs crate is not available, add
it as a dependency first.
- Around line 274-280: The is_our_symlink function compares symlink targets
directly without normalizing path representations, causing failures when
install.sh stores a relative symlink target (like ./pharos/claude/skills) while
harness.rs compares against an absolute path. Modify the comparison logic to
canonicalize both the symlink target returned by read_link and the src path
using a path normalization approach (either canonical paths or consistent
relative/absolute resolution) before comparing them with the equality check,
ensuring the comparison succeeds regardless of whether paths are represented as
relative or absolute.
In `@src-tauri/src/lib.rs`:
- Around line 242-245: The harness operations are susceptible to race conditions
because auto_sync_on_launch, sync_harness, and set_component_enabled all run
concurrently via spawn_blocking and can interleave access to shared config and
symlink state. Add a static Mutex at the module level to serialize all harness
operations. Wrap the auto_sync_on_launch function body with a lock acquisition,
and similarly protect the apply_harness() calls and read-modify-write sequences
in set_component_enabled to ensure only one harness operation executes at a
time. This prevents lost updates to component toggles and symlink conflicts.
---
Nitpick comments:
In `@src-tauri/src/harness.rs`:
- Around line 96-109: The block_version function truncates version strings at
the hyphen character, which breaks semver prerelease versions like 1.0.0-beta.1.
To fix this, modify the take_while condition in the block_version function to
only check for whitespace characters and remove the check for the hyphen
character. The closing comment marker --> will be naturally handled by the
whitespace condition, so the character check for *c != '-' should be removed
from the logical condition.
- Around line 539-593: The test is mutating global environment variables HOME
and PHAROS_DIR via std::env::set_var, which can cause interference when tests
run in parallel since Rust tests run concurrently by default. To fix this, add
the #[serial] attribute from the serial_test crate to the test function
containing this code (the function with the apply_harness() call and subsequent
assertions). This will ensure the test runs serially and prevent other tests
from observing incorrect environment variable values during execution.
Additionally, make sure the serial_test crate is added to the dev-dependencies
in Cargo.toml if not already present.
🪄 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: 06b319fd-bc41-4663-8fe0-db980bd89ec7
📒 Files selected for processing (5)
README.mdinstall.shsrc-tauri/src/harness.rssrc-tauri/src/lib.rssrc/components/SettingsPanel.tsx
| fn home_dir() -> PathBuf { | ||
| std::env::var("HOME").map(PathBuf::from).unwrap_or_default() | ||
| } |
There was a problem hiding this comment.
home_dir() returns empty path when HOME is unset, causing relative path writes.
If HOME is unset (rare but possible), PathBuf::default() returns an empty path. Subsequent joins produce relative paths like .claude/CLAUDE.md, potentially writing harness content into the current working directory rather than the user's home.
Consider returning Option<PathBuf> or using dirs::home_dir() crate which handles platform-specific fallbacks.
🛡️ Defensive fallback suggestion
fn home_dir() -> PathBuf {
- std::env::var("HOME").map(PathBuf::from).unwrap_or_default()
+ std::env::var("HOME")
+ .map(PathBuf::from)
+ .unwrap_or_else(|_| {
+ // Fallback to /tmp to avoid writing to CWD; auto_sync_on_launch
+ // will skip anyway since repo won't exist there.
+ eprintln!("harness: HOME 환경변수가 없어 기본 경로 사용 불가");
+ PathBuf::from("/tmp/pharos-no-home")
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn home_dir() -> PathBuf { | |
| std::env::var("HOME").map(PathBuf::from).unwrap_or_default() | |
| } | |
| fn home_dir() -> PathBuf { | |
| std::env::var("HOME") | |
| .map(PathBuf::from) | |
| .unwrap_or_else(|_| { | |
| // Fallback to /tmp to avoid writing to CWD; auto_sync_on_launch | |
| // will skip anyway since repo won't exist there. | |
| eprintln!("harness: HOME 환경변수가 없어 기본 경로 사용 불가"); | |
| PathBuf::from("/tmp/pharos-no-home") | |
| }) | |
| } |
🤖 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 `@src-tauri/src/harness.rs` around lines 193 - 195, The home_dir() function
returns an empty PathBuf when the HOME environment variable is unset, which
causes subsequent path joins to produce relative paths and write files to the
current working directory instead of the user's home directory. Replace the
current implementation that uses
std::env::var("HOME").map(PathBuf::from).unwrap_or_default() with the dirs
crate's home_dir() function, which provides platform-specific fallbacks and
handles the HOME variable more robustly. If the dirs crate is not available, add
it as a dependency first.
| /// True when `dst` is a symlink that points at exactly `src` (i.e. ours). | ||
| fn is_our_symlink(dst: &Path, src: &Path) -> bool { | ||
| std::fs::symlink_metadata(dst) | ||
| .map(|m| m.file_type().is_symlink()) | ||
| .unwrap_or(false) | ||
| && std::fs::read_link(dst).map(|c| c == *src).unwrap_or(false) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how install.sh creates symlinks to understand path representation
rg -n 'ln -s' --type=sh -A2 -B2Repository: dong-park/pharos
Length of output: 194
🏁 Script executed:
# Find where is_our_symlink is called and what src parameter is passed
rg -n 'is_our_symlink' src-tauri/src/harness.rs -B3 -A3Repository: dong-park/pharos
Length of output: 1415
🏁 Script executed:
# Check install.sh to see how it computes the src path for symlinks
rg -n 'src=' install.sh -B5 -A5 | head -100Repository: dong-park/pharos
Length of output: 313
🏁 Script executed:
# Look at the broader context of symlink creation in harness.rs (around line 353 mentioned in scratchpad)
sed -n '345,365p' src-tauri/src/harness.rsRepository: dong-park/pharos
Length of output: 785
🏁 Script executed:
# Find where apply_unit and remove_unit are called to see src parameter sources
rg -n 'apply_unit|remove_unit' src-tauri/src/harness.rs -B2 -A2Repository: dong-park/pharos
Length of output: 525
🏁 Script executed:
# Find the repo_dir() function to understand how base path is computed
rg -n 'fn repo_dir|repo_dir()' src-tauri/src/harness.rs -B2 -A5Repository: dong-park/pharos
Length of output: 1424
🏁 Script executed:
# Check install.sh to see how it determines the repo path
rg -n 'REPO|repo' install.sh | head -20Repository: dong-park/pharos
Length of output: 403
🏁 Script executed:
# Get the complete link() function from install.sh
sed -n '246,280p' install.shRepository: dong-park/pharos
Length of output: 1046
🏁 Script executed:
# Also check if there's any path normalization or expansion happening in install.sh
rg -n 'realpath|canonicalize|readlink|ln -s' install.sh -B3 -A3Repository: dong-park/pharos
Length of output: 383
Symlink target comparison may fail if paths differ in representation between install.sh and harness.rs.
read_link returns the raw symlink target as stored. If install.sh is run with PHAROS_DIR as a relative path (e.g., ./pharos) or an absolute path different from harness.rs's computed default, the equality check c == *src will fail even when both point to the same directory.
Example: If install.sh stores a symlink target as ./pharos/claude/skills but harness.rs compares against /home/user/pharos/claude/skills (absolute), the ownership check fails incorrectly.
Consider canonicalizing both paths before comparison, or ensure both use identical path resolution logic (same PHAROS_DIR env var or default).
🤖 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 `@src-tauri/src/harness.rs` around lines 274 - 280, The is_our_symlink function
compares symlink targets directly without normalizing path representations,
causing failures when install.sh stores a relative symlink target (like
./pharos/claude/skills) while harness.rs compares against an absolute path.
Modify the comparison logic to canonicalize both the symlink target returned by
read_link and the src path using a path normalization approach (either canonical
paths or consistent relative/absolute resolution) before comparing them with the
equality check, ensuring the comparison succeeds regardless of whether paths are
represented as relative or absolute.
| // Keep ~/.claude aligned with the repo's harness source blocks + | ||
| // unit symlinks. Off the UI thread, never blocks startup: re-applies | ||
| // only on drift, silently skips when the repo isn't present. | ||
| let _ = tauri::async_runtime::spawn_blocking(harness::auto_sync_on_launch); |
There was a problem hiding this comment.
Concurrent access to harness files creates race conditions.
The fire-and-forget auto_sync_on_launch can run concurrently with user-triggered sync_harness or set_component_enabled commands, since all use spawn_blocking. This creates multiple race conditions:
-
Config lost-update:
set_component_enableduses read-modify-write (load → modify → save). If two calls overlap, the second save will overwrite the first, losing component toggle changes. -
Concurrent
apply_harness()calls: All three code paths callapply_harness(), which writes CLAUDE.md and manages symlinks. Concurrent execution can cause interleaved writes or symlink conflicts.
The race window is small (startup task completes quickly and frontend calls are user-triggered), but the data corruption risk is real.
🔒 Add synchronization around harness operations
Add a static mutex to serialize harness operations:
// At top of harness.rs or lib.rs
use std::sync::Mutex;
use once_cell::sync::Lazy;
static HARNESS_LOCK: Lazy<Mutex<()>> = Lazy::new(|| Mutex::new(()));Then wrap harness operations:
pub fn auto_sync_on_launch() {
let _guard = HARNESS_LOCK.lock().unwrap();
// ... existing implementation
}
// Similarly for apply_harness() and config operations in set_component_enabledAlternatively, use filesystem-level file locking (e.g., flock on Unix) around config read-modify-write sequences.
🤖 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 `@src-tauri/src/lib.rs` around lines 242 - 245, The harness operations are
susceptible to race conditions because auto_sync_on_launch, sync_harness, and
set_component_enabled all run concurrently via spawn_blocking and can interleave
access to shared config and symlink state. Add a static Mutex at the module
level to serialize all harness operations. Wrap the auto_sync_on_launch function
body with a lock acquisition, and similarly protect the apply_harness() calls
and read-modify-write sequences in set_component_enabled to ensure only one
harness operation executes at a time. This prevents lost updates to component
toggles and symlink conflicts.
std::os::unix::fs::symlink은 Windows에 없어 GUI installer 빌드가 E0433로 실패. cfg(unix/windows)로 symlink_impl 분리, Windows는 symlink_dir/symlink_file 사용. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
무엇을
GUI 앱이 Claude 하네스(
~/.claude의 CLAUDE.md 블록 + skills/hooks/commands 심링크)를 자체적으로 동기화한다.install.sh수동 재실행 없이 앱 설치/재시작만으로 하네스가 repo 소스와 정렬되고, 컴포넌트를 개별로 켜고 끌 수 있다.왜
Claude 네이티브 플러그인은 always-on 지침(CLAUDE.md)을 못 실어준다. 앱은 fs 권한이 있어 그 갭을 메울 수 있다 — 앱이 곧 하네스 배포 채널.
변경 (T1~T5)
harness.rs순수 코어: 마커 안전 inject/remove, VERSION 게이트, config, compute_claude_md (단위테스트 11)harness_status/sync_harness/set_component_enabled), 전부 spawn_blocking안전장치
.pre-pharos-*로 백업,remove_dir_all안 씀검증
yarn check전체 그린 (vitest 588)fable:false로 런치훅이 FABLE만 제거(297→277), 복원 시 재주입(version-gate가 guidelines 1.2.0 최신소스 반영)비목표 (연기)
per-skill 토글, 원격 마켓플레이스, 크로스머신 번들 전파
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation