From 363352e7bff349ff8adb170d3d8ec2da22b4c36c Mon Sep 17 00:00:00 2001 From: Parfii-bot Date: Wed, 22 Apr 2026 13:36:17 +0800 Subject: [PATCH] fix(kei-refactor-engine): retract 'git apply-ready' claim (F1 RELEASE BLOCKER) Output renamed plan-autoresolve.md; header changed to '# AUTO-RESOLVABLE items' (no fake --- a/ /+++ b/ wrapper). Added test autoresolve_output_is_not_claimed_as_diff. Template updated: user manually applies, not via git apply. --- .../_rust/kei-refactor-engine/src/lib.rs | 5 +- .../_rust/kei-refactor-engine/src/main.rs | 22 ++-- .../_rust/kei-refactor-engine/src/patch.rs | 109 +++++++++++++++--- .../kei-refactor-engine/tests/integration.rs | 28 +++-- .../templates/deep-sleep-trigger-prompt.md | 29 +++-- 5 files changed, 146 insertions(+), 47 deletions(-) diff --git a/_primitives/_rust/kei-refactor-engine/src/lib.rs b/_primitives/_rust/kei-refactor-engine/src/lib.rs index 7f3453c..7fa628f 100644 --- a/_primitives/_rust/kei-refactor-engine/src/lib.rs +++ b/_primitives/_rust/kei-refactor-engine/src/lib.rs @@ -1,11 +1,12 @@ //! kei-refactor-engine — library surface. //! //! Consumes `kei-conflict-scan` JSON; produces a structured refactor plan -//! (markdown) and, optionally, a patch file for user `git apply` review. +//! (markdown) and, optionally, an auto-resolve review markdown +//! (NOT a unified diff — see patch.rs header, v0.14.1 retraction). //! //! Zero-conflict guarantee: any conflict whose `auto_resolvable = false` //! is included in the plan under "Requires human decision" and EXCLUDED -//! from the generated patch. +//! from the auto-resolve markdown. pub mod input; pub mod plan; diff --git a/_primitives/_rust/kei-refactor-engine/src/main.rs b/_primitives/_rust/kei-refactor-engine/src/main.rs index d592ade..04b8855 100644 --- a/_primitives/_rust/kei-refactor-engine/src/main.rs +++ b/_primitives/_rust/kei-refactor-engine/src/main.rs @@ -3,7 +3,11 @@ //! Usage: //! kei-refactor-engine --input conflicts.json --plan-only > plan.md //! kei-refactor-engine --input conflicts.json --apply-to-branch deep-sleep/2026-04-22 \ -//! --plan-out plan.md --patch-out changes.patch +//! --plan-out plan.md --patch-out plan-autoresolve.md +//! +//! NOTE (v0.14.1): `--patch-out` writes a MARKDOWN review file, NOT a +//! unified diff. The old claim "git apply-ready patch" was retracted — +//! see `patch.rs` header. The flag name is kept for backwards-compat. use anyhow::Result; use clap::Parser; @@ -24,7 +28,7 @@ struct Cli { #[arg(long, default_value_t = true)] plan_only: bool, - /// Apply mode — also write a patch file; takes the branch name. + /// Apply mode — also write an auto-resolve review file; takes the branch name. #[arg(long)] apply_to_branch: Option, @@ -32,7 +36,8 @@ struct Cli { #[arg(long)] plan_out: Option, - /// Optional explicit path for the patch file. + /// Optional explicit path for the auto-resolve review markdown + /// (NOT a unified diff — see patch.rs header). #[arg(long)] patch_out: Option, } @@ -54,14 +59,14 @@ fn write_plan(plan: &Plan, branch: Option<&str>, out: Option<&PathBuf>) -> Resul Ok(()) } -fn maybe_write_patch( +fn maybe_write_autoresolve( plan: &Plan, branch: &str, out: Option<&PathBuf>, ) -> Result { - let default = PathBuf::from("deep-sleep.patch"); + let default = PathBuf::from("plan-autoresolve.md"); let target = out.unwrap_or(&default); - patch::write_patch(plan, branch, target) + patch::write_autoresolve(plan, branch, target) } fn run(cli: &Cli) -> Result { @@ -72,9 +77,10 @@ fn run(cli: &Cli) -> Result { write_plan(&plan, branch, cli.plan_out.as_ref())?; if let Some(br) = branch { - let applied = maybe_write_patch(&plan, br, cli.patch_out.as_ref())?; + let applied = maybe_write_autoresolve(&plan, br, cli.patch_out.as_ref())?; eprintln!( - "kei-refactor-engine: wrote patch with {} auto-apply item(s); {} human-decision item(s) excluded.", + "kei-refactor-engine: wrote auto-resolve review with {} auto-apply item(s); \ + {} human-decision item(s) excluded. Review manually — this is NOT a unified diff.", applied, plan.manual_items().len(), ); diff --git a/_primitives/_rust/kei-refactor-engine/src/patch.rs b/_primitives/_rust/kei-refactor-engine/src/patch.rs index e80fd94..bf95dfd 100644 --- a/_primitives/_rust/kei-refactor-engine/src/patch.rs +++ b/_primitives/_rust/kei-refactor-engine/src/patch.rs @@ -1,9 +1,19 @@ -//! Patch synthesizer — writes a unified-diff file for `git apply` preview. +//! Auto-resolve plan writer. //! -//! This crate NEVER runs git. Per RULE 0.13 the orchestrator is the only -//! party that commits. We emit `.patch` text the user reads + applies. +//! v0.14.1 retraction: this module used to emit a `*.patch` file with +//! `--- a/` / `+++ b/` headers that *looked* like unified-diff +//! but had no real hunk bodies. `git apply --check` rejects that format. +//! The claim "git apply-ready patch" was incorrect. //! -//! Only items whose resolution == AutoApply are materialised here; the +//! New behaviour: we write a companion markdown file +//! (`plan-autoresolve.md`) listing the auto-apply candidates so the user +//! can review + apply them manually. File-content diffs would require +//! reading each source file, which is out of scope for this crate and +//! risks hallucinated edits (RULE 0.4). The "applied fork" path in +//! deep-sleep still produces a real branch via rename/move ops — those +//! are performed by the orchestrator, not by this file emitter. +//! +//! Only items whose `resolution == AutoApply` are listed here; the //! zero-conflict guarantee keeps `requires_human_decision` items out. use crate::plan::{Plan, PlanItem, Resolution}; @@ -11,12 +21,16 @@ use anyhow::Result; use std::fs; use std::path::Path; -pub fn write_patch(plan: &Plan, branch: &str, out_file: &Path) -> Result { +/// Write the auto-resolve review markdown. Returns the count of auto items. +/// +/// The file is intentionally NOT a unified diff. It is a markdown +/// summary humans read before applying changes with an editor. +pub fn write_autoresolve(plan: &Plan, branch: &str, out_file: &Path) -> Result { let auto = plan.auto_items(); let mut body = String::new(); body.push_str(&header(branch, auto.len(), plan.manual_items().len())); - for item in &auto { - body.push_str(&hunk_for(item)); + for (idx, item) in auto.iter().enumerate() { + body.push_str(&entry_for(idx + 1, item)); } fs::write(out_file, body)?; Ok(auto.len()) @@ -24,25 +38,32 @@ pub fn write_patch(plan: &Plan, branch: &str, out_file: &Path) -> Result fn header(branch: &str, auto: usize, manual: usize) -> String { format!( - "# kei-refactor-engine preview patch\n\ + "# AUTO-RESOLVABLE items (review, don't `git apply`)\n\ # Branch intent: {branch}\n\ - # Auto-apply items: {auto}\n\ - # Human-decision items (NOT in this patch, see plan): {manual}\n\ - # Review with `git apply --check ` before merging.\n\n" + # Auto-apply candidates: {auto}\n\ + # Human-decision items (NOT listed here, see plan): {manual}\n\ + #\n\ + # This file is NOT a unified diff. Open each FILE below and apply\n\ + # the EXAMPLE change by hand. The engine does not read file contents\n\ + # and therefore cannot emit real +/- hunks (RULE 0.4: no fabricated\n\ + # edits).\n\n" ) } -fn hunk_for(item: &PlanItem) -> String { - // Conservative: we do not invent file content. We emit an annotated - // comment block per item so the user sees intent, not fabricated code. +fn entry_for(n: usize, item: &PlanItem) -> String { let files = item.files.join(", "); format!( - "--- a/{file}\n+++ b/{file}\n# INTENT ({cat}/{sev}): {why}\n# FILES: {files}\n# EXAMPLE: {ex}\n# TRADEOFF: {tr}\n\n", - file = item.files.first().cloned().unwrap_or_else(|| "".into()), + "## {n}. [{cat}/{sev}] {first_file}\n\ + - FILES: {files}\n\ + - WHY: {why}\n\ + - EXAMPLE: {ex}\n\ + - TRADEOFF: {tr}\n\n", + n = n, cat = item.category, sev = item.severity, - why = item.why, + first_file = item.files.first().cloned().unwrap_or_else(|| "".into()), files = files, + why = item.why, ex = item.example, tr = item.tradeoff, ) @@ -54,3 +75,57 @@ pub fn excluded_manual(plan: &Plan) -> Vec<&PlanItem> { .filter(|i| i.resolution == Resolution::RequiresHumanDecision) .collect() } + +// Backwards-compatibility shim for callers that still invoke the old name. +// Forwards to `write_autoresolve` — output semantics changed but signature +// matches. New code should call `write_autoresolve` directly. +#[deprecated(note = "renamed to write_autoresolve — output is no longer a unified diff")] +pub fn write_patch(plan: &Plan, branch: &str, out_file: &Path) -> Result { + write_autoresolve(plan, branch, out_file) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::plan::{Plan, PlanItem, Resolution}; + + fn sample_plan() -> Plan { + Plan { + items: vec![PlanItem { + resolution: Resolution::AutoApply, + category: "blocks".into(), + severity: "medium".into(), + files: vec!["_blocks/a.md".into(), "_blocks/b.md".into()], + why: "75% shingle overlap".into(), + example: "keep better-cited".into(), + tradeoff: "deprecation header loses inbound links".into(), + }], + } + } + + #[test] + fn autoresolve_output_is_not_claimed_as_diff() { + let plan = sample_plan(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + let n = write_autoresolve(&plan, "deep-sleep/2026-04-22", tmp.path()).unwrap(); + let body = fs::read_to_string(tmp.path()).unwrap(); + assert_eq!(n, 1); + // Must NOT start with unified-diff headers — those are a lie here. + assert!(!body.starts_with("--- a/"), "output starts with --- a/ (fake diff): {body}"); + assert!(!body.contains("\n--- a/"), "output contains --- a/ (fake diff): {body}"); + assert!(!body.contains("+++ b/"), "output contains +++ b/ (fake diff): {body}"); + // Must be human-readable markdown heading. + assert!(body.contains("AUTO-RESOLVABLE items")); + } + + #[test] + fn autoresolve_includes_files_and_example() { + let plan = sample_plan(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + write_autoresolve(&plan, "x", tmp.path()).unwrap(); + let body = fs::read_to_string(tmp.path()).unwrap(); + assert!(body.contains("_blocks/a.md")); + assert!(body.contains("_blocks/b.md")); + assert!(body.contains("keep better-cited")); + } +} diff --git a/_primitives/_rust/kei-refactor-engine/tests/integration.rs b/_primitives/_rust/kei-refactor-engine/tests/integration.rs index 860384f..5f2656a 100644 --- a/_primitives/_rust/kei-refactor-engine/tests/integration.rs +++ b/_primitives/_rust/kei-refactor-engine/tests/integration.rs @@ -47,11 +47,11 @@ fn plan_only_prints_markdown() { } #[test] -fn manual_items_listed_but_not_in_patch() { +fn manual_items_listed_but_not_in_autoresolve() { let tmp = TempDir::new().unwrap(); let input = tmp.path().join("c.json"); let plan_out = tmp.path().join("plan.md"); - let patch_out = tmp.path().join("p.patch"); + let patch_out = tmp.path().join("plan-autoresolve.md"); fs::write(&input, sample_json(true)).unwrap(); let out = std::process::Command::new(bin()) .args(["--input"]) @@ -65,10 +65,13 @@ fn manual_items_listed_but_not_in_patch() { assert!(out.status.success(), "stderr: {}", String::from_utf8_lossy(&out.stderr)); let md = fs::read_to_string(&plan_out).unwrap(); assert!(md.contains("Requires human decision")); - let patch = fs::read_to_string(&patch_out).unwrap(); - // patch must NOT reference rules/x.md from the manual item - assert!(!patch.contains("rules/x.md"), "patch leaked manual item: {}", patch); - assert!(patch.contains("_blocks/a.md")); + let autoresolve = fs::read_to_string(&patch_out).unwrap(); + // autoresolve must NOT reference rules/x.md from the manual item + assert!(!autoresolve.contains("rules/x.md"), "autoresolve leaked manual item: {}", autoresolve); + assert!(autoresolve.contains("_blocks/a.md")); + // And it must NOT claim to be a unified diff. + assert!(!autoresolve.contains("--- a/")); + assert!(!autoresolve.contains("+++ b/")); } #[test] @@ -107,10 +110,10 @@ fn stdin_input_works() { } #[test] -fn patch_header_shows_counts() { +fn autoresolve_header_shows_counts() { let tmp = TempDir::new().unwrap(); let input = tmp.path().join("c.json"); - let patch_out = tmp.path().join("p.patch"); + let patch_out = tmp.path().join("plan-autoresolve.md"); fs::write(&input, sample_json(true)).unwrap(); std::process::Command::new(bin()) .args(["--input"]) @@ -119,7 +122,10 @@ fn patch_header_shows_counts() { .arg(&patch_out) .output() .unwrap(); - let patch = fs::read_to_string(&patch_out).unwrap(); - assert!(patch.contains("Auto-apply items: 1")); - assert!(patch.contains("Human-decision items")); + let autoresolve = fs::read_to_string(&patch_out).unwrap(); + assert!(autoresolve.contains("Auto-apply candidates: 1")); + assert!(autoresolve.contains("Human-decision items")); + // Retraction check: no unified-diff headers. + assert!(!autoresolve.contains("--- a/")); + assert!(!autoresolve.contains("+++ b/")); } diff --git a/_primitives/templates/deep-sleep-trigger-prompt.md b/_primitives/templates/deep-sleep-trigger-prompt.md index 2ee7573..7282bff 100644 --- a/_primitives/templates/deep-sleep-trigger-prompt.md +++ b/_primitives/templates/deep-sleep-trigger-prompt.md @@ -48,17 +48,27 @@ v0.12.0 rules AND Phase C is skipped too — the marathon owns the night. 3. **Optional fork (only if `WITH_FORK=1`):** a. `kei-refactor-engine --input - --apply-to-branch - deep-sleep/YYYY-MM-DD --patch-out sync-repo/sleep-deep/YYYY-MM-DD.patch` - (re-run on same JSON; patch file lists auto-apply items only). + deep-sleep/YYYY-MM-DD --patch-out sync-repo/sleep-deep/YYYY-MM-DD-autoresolve.md` + (re-run on same JSON; the auto-resolve markdown lists auto-apply + items only — NOT a unified diff, see note below). - b. Apply the patch to a new local branch: - `git checkout -b deep-sleep/YYYY-MM-DD && git apply ` + b. Review the auto-resolve markdown and apply each change manually + in a new local branch: + `git checkout -b deep-sleep/YYYY-MM-DD` + Open `.md`, edit the listed files accordingly, then + `git add && git commit`. - c. Gate: `kei-graph-check --path sync-repo/ --after-diff `. - If broken refs → abort fork, delete branch, append "graph check - failed — fork aborted, plan kept" note to the plan file. + c. Gate: `kei-graph-check --path sync-repo/`. If broken refs after + your edits → delete branch, append "graph check failed — fork + aborted, plan kept" note to the plan file. - d. If clean → commit the applied changes on the fork branch. + d. If clean → push the fork branch for morning review. + +> NOTE (v0.14.1 retraction): earlier docs claimed the engine emits a +> `git apply`-ready patch. It does not — see `patch.rs` header for +> the reason (engine cannot synthesise file-content hunks without +> reading source files, which risks RULE 0.4 hallucination). The +> companion file is a markdown summary reviewed and applied by hand. 4. **Commit + push.** The plan markdown is always committed to `main` with message `NREM: deep-sleep YYYY-MM-DD`. If a fork branch was @@ -86,6 +96,7 @@ No silent auto-apply of ambiguous changes. - `kei-conflict-scan` fails → record the error in the plan body and skip fork. - `kei-refactor-engine` fails → same; keep any partial plan markdown. -- `git apply` rejects → delete fork branch; append reject to the plan. +- Manual edits in step 3b produce merge conflicts → delete fork branch; + append the conflict summary to the plan. - Push fails → retry once; on second failure leave local commit and exit 1. Local state is recoverable on next run.