Skip to content

Commit 4bf785a

Browse files
setup: Merge git hooks into a bounded managed block instead of overwriting them
Every setup write — config assets and required git hooks alike — unlinked the destination file before renaming the staged replacement over it. The staging file already lives in the destination's directory, so the rename alone is atomic; the unlink only opened a window where the file did not exist, and on a rename failure the error path deleted the staging copy too, leaving neither. This mattered once `.claude/settings.json` and `.opencode/opencode.json` became merge targets holding user-owned keys that setup cannot reconstruct. Both pre-deletes are dropped in favor of atomic rename. `sce setup --hooks` also replaced `pre-commit`, `commit-msg`, and `post-commit` wholesale, destroying any hook a repository already ran (husky, lefthook, hand-written). Each canonical hook payload is now delimited by an SCE managed block marker pair. Install creates the full script when no hook exists, replaces the block in place when one already carries it, recognizes and replaces a legacy pre-marker SCE payload wholesale, and otherwise appends the block after a foreign hook's content so SCE always runs last. A last-effective-line heuristic reports an advisory when a foreign hook's trailing zero-indent `exec`/`exit` would make the appended block unreachable. `sce doctor` now classifies hook content by managed-block currency instead of byte-exact comparison, so foreign content around a current block does not read as drift. Co-authored-by: SCE <sce@crocoder.dev>
1 parent 8db465d commit 4bf785a

21 files changed

Lines changed: 1348 additions & 152 deletions

cli/assets/hooks/commit-msg

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#!/bin/sh
22
set -eu
33

4+
# >>> sce managed block (do not edit) >>>
45
if ! command -v sce >/dev/null 2>&1; then
56
# sce brand colors — only emit ANSI when stderr is a real terminal
67
if [ -t 2 ]; then
@@ -25,4 +26,7 @@ if ! command -v sce >/dev/null 2>&1; then
2526
exit 0
2627
fi
2728

28-
exec sce hooks commit-msg "$@"
29+
sce hooks commit-msg "$@"
30+
status=$?
31+
exit "$status"
32+
# <<< sce managed block <<<

cli/assets/hooks/post-commit

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
#!/bin/sh
22
set -eu
33

4-
remote_url="$(git remote get-url origin 2>/dev/null || true)"
5-
4+
# >>> sce managed block (do not edit) >>>
65
if ! command -v sce >/dev/null 2>&1; then
76
# sce brand colors — only emit ANSI when stderr is a real terminal
87
if [ -t 2 ]; then
@@ -27,8 +26,13 @@ if ! command -v sce >/dev/null 2>&1; then
2726
exit 0
2827
fi
2928

29+
remote_url="$(git remote get-url origin 2>/dev/null || true)"
30+
3031
if [ -n "$remote_url" ]; then
31-
exec sce hooks post-commit --vcs git --remote-url "$remote_url" "$@"
32+
sce hooks post-commit --vcs git --remote-url "$remote_url" "$@"
33+
else
34+
sce hooks post-commit --vcs git "$@"
3235
fi
33-
34-
exec sce hooks post-commit --vcs git "$@"
36+
status=$?
37+
exit "$status"
38+
# <<< sce managed block <<<

cli/assets/hooks/pre-commit

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#!/bin/sh
22
set -eu
33

4+
# >>> sce managed block (do not edit) >>>
45
if ! command -v sce >/dev/null 2>&1; then
56
# sce brand colors — only emit ANSI when stderr is a real terminal
67
if [ -t 2 ]; then
@@ -25,4 +26,7 @@ if ! command -v sce >/dev/null 2>&1; then
2526
exit 0
2627
fi
2728

28-
exec sce hooks pre-commit "$@"
29+
sce hooks pre-commit "$@"
30+
status=$?
31+
exit "$status"
32+
# <<< sce managed block <<<

cli/src/services/doctor/inspect.rs

Lines changed: 171 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ use crate::services::repository_identity::resolve::{
1515
resolve_repository_identity, RepositoryIdentitySource,
1616
};
1717
use crate::services::setup::{
18-
config_merge, iter_embedded_assets_for_setup_target_with_selection, iter_required_hook_assets,
19-
persisted_optional_workflows, repair_merge_target_asset, EmbeddedAsset, SetupTarget,
18+
config_merge, hook_merge, iter_embedded_assets_for_setup_target_with_selection,
19+
iter_required_hook_assets, persisted_optional_workflows, repair_merge_target_asset,
20+
EmbeddedAsset, SetupTarget,
2021
};
2122

2223
use super::types::{
@@ -318,17 +319,28 @@ fn inspect_hook_content_state_without_problem(
318319
};
319320

320321
match fs::read(hook_path) {
321-
Ok(bytes) => {
322-
if bytes == expected_hook.bytes {
323-
HookContentState::Current
324-
} else {
325-
HookContentState::Stale
326-
}
327-
}
322+
Ok(bytes) => hook_managed_block_content_state(hook_name, &bytes, expected_hook.bytes),
328323
Err(_) => HookContentState::Unknown,
329324
}
330325
}
331326

327+
/// Classifies a hook's on-disk bytes against the canonical template by SCE
328+
/// managed-block currency (merging the canonical block into `bytes` is a
329+
/// no-op) rather than whole-file equality, so foreign content a repository
330+
/// has appended around the block does not read as drift. An unbalanced or
331+
/// partial managed block is also reported `Stale`, since it needs the same
332+
/// `--fix` repair as a drifted one.
333+
fn hook_managed_block_content_state(
334+
hook_name: &str,
335+
bytes: &[u8],
336+
canonical: &[u8],
337+
) -> HookContentState {
338+
match hook_merge::merge_or_create_hook(Some(bytes), canonical, hook_name) {
339+
Ok(merge) if merge.bytes == bytes => HookContentState::Current,
340+
Ok(_) | Err(_) => HookContentState::Stale,
341+
}
342+
}
343+
332344
#[allow(dead_code)]
333345
fn inspect_repository_hooks(
334346
repository_root: &Path,
@@ -1568,13 +1580,7 @@ fn inspect_hook_content_state(
15681580
};
15691581

15701582
match fs::read(hook_path) {
1571-
Ok(bytes) => {
1572-
if bytes == expected_hook.bytes {
1573-
HookContentState::Current
1574-
} else {
1575-
HookContentState::Stale
1576-
}
1577-
}
1583+
Ok(bytes) => hook_managed_block_content_state(hook_name, &bytes, expected_hook.bytes),
15781584
Err(error) => {
15791585
problems.push(DoctorProblem {
15801586
kind: ProblemKind::HookReadFailed,
@@ -1602,8 +1608,9 @@ mod tests {
16021608
use std::path::PathBuf;
16031609

16041610
use super::{
1605-
collect_claude_integration_groups, collect_opencode_integration_groups,
1606-
collect_pi_integration_groups, inspect_claude_integration_health, IntegrationContentState,
1611+
collect_claude_integration_groups, collect_hook_file_health,
1612+
collect_opencode_integration_groups, collect_pi_integration_groups,
1613+
inspect_claude_integration_health, HookContentState, IntegrationContentState,
16071614
IntegrationGroupHealth,
16081615
};
16091616
use crate::services::setup::OPTIONAL_WORKFLOWS;
@@ -1942,4 +1949,150 @@ mod tests {
19421949

19431950
std::fs::remove_dir_all(&root).ok();
19441951
}
1952+
1953+
fn canonical_pre_commit_bytes() -> &'static [u8] {
1954+
crate::services::setup::iter_required_hook_assets()
1955+
.find(|asset| asset.relative_path == "pre-commit")
1956+
.expect("embedded catalog carries pre-commit")
1957+
.bytes
1958+
}
1959+
1960+
#[cfg(unix)]
1961+
fn mark_executable(path: &std::path::Path) {
1962+
use std::os::unix::fs::PermissionsExt;
1963+
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755))
1964+
.expect("mark hook executable");
1965+
}
1966+
1967+
#[test]
1968+
fn hook_with_foreign_content_and_current_block_reports_current() {
1969+
let dir = unique_temp_repository_root("hook-foreign-current");
1970+
let foreign_prefix = b"#!/bin/sh\necho husky-style-guard\n".to_vec();
1971+
let merge = crate::services::setup::hook_merge::merge_or_create_hook(
1972+
Some(&foreign_prefix),
1973+
canonical_pre_commit_bytes(),
1974+
"pre-commit",
1975+
)
1976+
.expect("merge over foreign hook should succeed");
1977+
1978+
let hook_path = dir.join("pre-commit");
1979+
std::fs::write(&hook_path, &merge.bytes).expect("write foreign-plus-block hook");
1980+
#[cfg(unix)]
1981+
mark_executable(&hook_path);
1982+
1983+
let health = collect_hook_file_health(&dir);
1984+
let pre_commit = health
1985+
.iter()
1986+
.find(|hook| hook.name == "pre-commit")
1987+
.expect("pre-commit health present");
1988+
assert_eq!(pre_commit.content_state, HookContentState::Current);
1989+
1990+
std::fs::remove_dir_all(&dir).ok();
1991+
}
1992+
1993+
#[test]
1994+
fn hook_with_drifted_managed_block_reports_stale() {
1995+
let dir = unique_temp_repository_root("hook-drifted-stale");
1996+
let canonical_text =
1997+
String::from_utf8(canonical_pre_commit_bytes().to_vec()).expect("hook is utf8");
1998+
let drifted_text = canonical_text.replace(
1999+
"sce hooks pre-commit \"$@\"",
2000+
"sce hooks pre-commit \"$@\" # drifted",
2001+
);
2002+
assert_ne!(
2003+
drifted_text, canonical_text,
2004+
"drift fixture should actually differ from canonical"
2005+
);
2006+
2007+
let hook_path = dir.join("pre-commit");
2008+
std::fs::write(&hook_path, drifted_text.as_bytes()).expect("write drifted hook");
2009+
#[cfg(unix)]
2010+
mark_executable(&hook_path);
2011+
2012+
let health = collect_hook_file_health(&dir);
2013+
let pre_commit = health
2014+
.iter()
2015+
.find(|hook| hook.name == "pre-commit")
2016+
.expect("pre-commit health present");
2017+
assert_eq!(pre_commit.content_state, HookContentState::Stale);
2018+
2019+
std::fs::remove_dir_all(&dir).ok();
2020+
}
2021+
2022+
fn init_git_repo(label: &str) -> PathBuf {
2023+
let repo = unique_temp_repository_root(label);
2024+
let output = std::process::Command::new("git")
2025+
.args(["init", "-q"])
2026+
.current_dir(&repo)
2027+
.output()
2028+
.expect("git init should spawn");
2029+
assert!(
2030+
output.status.success(),
2031+
"git init failed: {}",
2032+
String::from_utf8_lossy(&output.stderr)
2033+
);
2034+
repo
2035+
}
2036+
2037+
#[test]
2038+
fn fix_repairs_drifted_hook_content_while_preserving_foreign_content() {
2039+
let repo = init_git_repo("hook-fix-repair");
2040+
2041+
let initial_outcome = crate::services::setup::install_required_git_hooks(&repo)
2042+
.expect("initial hook install should succeed");
2043+
let pre_commit_path = initial_outcome
2044+
.hook_results
2045+
.iter()
2046+
.find(|result| result.hook_name == "pre-commit")
2047+
.expect("pre-commit hook installed")
2048+
.hook_path
2049+
.clone();
2050+
let hooks_directory = pre_commit_path
2051+
.parent()
2052+
.expect("hook path has a parent directory")
2053+
.to_path_buf();
2054+
2055+
let foreign_prefix = b"#!/bin/sh\necho husky-style-guard\n".to_vec();
2056+
let foreign_plus_block = crate::services::setup::hook_merge::merge_or_create_hook(
2057+
Some(&foreign_prefix),
2058+
canonical_pre_commit_bytes(),
2059+
"pre-commit",
2060+
)
2061+
.expect("merge over foreign hook should succeed");
2062+
let drifted_text = String::from_utf8(foreign_plus_block.bytes.clone())
2063+
.expect("hook is utf8")
2064+
.replace(
2065+
"sce hooks pre-commit \"$@\"",
2066+
"sce hooks pre-commit \"$@\" # drifted",
2067+
);
2068+
std::fs::write(&pre_commit_path, drifted_text.as_bytes())
2069+
.expect("seed foreign-plus-drifted-block hook");
2070+
#[cfg(unix)]
2071+
mark_executable(&pre_commit_path);
2072+
2073+
let health_before = collect_hook_file_health(&hooks_directory);
2074+
let pre_commit_before = health_before
2075+
.iter()
2076+
.find(|hook| hook.name == "pre-commit")
2077+
.expect("pre-commit health present");
2078+
assert_eq!(pre_commit_before.content_state, HookContentState::Stale);
2079+
2080+
crate::services::setup::install_required_git_hooks(&repo)
2081+
.expect("'--fix' repair reuses the canonical setup hook installation");
2082+
2083+
let repaired_bytes = std::fs::read(&pre_commit_path).expect("read repaired hook");
2084+
assert!(
2085+
repaired_bytes.starts_with(&foreign_prefix),
2086+
"foreign content should survive the repair"
2087+
);
2088+
2089+
let health_after = collect_hook_file_health(&hooks_directory);
2090+
let pre_commit_after = health_after
2091+
.iter()
2092+
.find(|hook| hook.name == "pre-commit")
2093+
.expect("pre-commit health present");
2094+
assert_eq!(pre_commit_after.content_state, HookContentState::Current);
2095+
2096+
std::fs::remove_dir_all(&repo).ok();
2097+
}
19452098
}

cli/src/services/hooks/lifecycle.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::services::lifecycle::{
1111
RequiredHooksInstallOutcome, ServiceLifecycle, SetupOutcome,
1212
};
1313
use crate::services::setup::{
14-
install_required_git_hooks, iter_required_hook_assets,
14+
hook_merge, install_required_git_hooks, iter_required_hook_assets,
1515
RequiredHookInstallStatus as SetupRequiredHookInstallStatus,
1616
RequiredHooksInstallOutcome as SetupRequiredHooksInstallOutcome,
1717
};
@@ -298,10 +298,9 @@ fn inspect_hook_content_state(
298298

299299
match fs::read(hook_path) {
300300
Ok(bytes) => {
301-
if bytes == expected_hook.bytes {
302-
HookContentState::Current
303-
} else {
304-
HookContentState::Stale
301+
match hook_merge::merge_or_create_hook(Some(&bytes), expected_hook.bytes, hook_name) {
302+
Ok(merge) if merge.bytes == bytes => HookContentState::Current,
303+
Ok(_) | Err(_) => HookContentState::Stale,
305304
}
306305
}
307306
Err(error) => {
@@ -375,6 +374,7 @@ fn required_hooks_outcome_from_setup(
375374
RequiredHookInstallStatus::Skipped
376375
}
377376
},
377+
unreachable_block_advisory: result.unreachable_block_advisory,
378378
},
379379
)
380380
.collect(),

cli/src/services/lifecycle.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ pub struct RequiredHookInstallResult {
103103
pub hook_name: String,
104104
pub hook_path: PathBuf,
105105
pub status: RequiredHookInstallStatus,
106+
pub unreachable_block_advisory: bool,
106107
}
107108

108109
#[derive(Clone, Debug, Eq, PartialEq)]

cli/src/services/setup/command.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ fn setup_required_hooks_outcome_from_lifecycle(
123123
RequiredHookInstallStatus::Updated => setup::RequiredHookInstallStatus::Updated,
124124
RequiredHookInstallStatus::Skipped => setup::RequiredHookInstallStatus::Skipped,
125125
},
126+
unreachable_block_advisory: result.unreachable_block_advisory,
126127
})
127128
.collect(),
128129
}

0 commit comments

Comments
 (0)