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.
This commit is contained in:
parent
ef95bf2a7c
commit
363352e7bf
5 changed files with 146 additions and 47 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<String>,
|
||||
|
||||
|
|
@ -32,7 +36,8 @@ struct Cli {
|
|||
#[arg(long)]
|
||||
plan_out: Option<PathBuf>,
|
||||
|
||||
/// 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<PathBuf>,
|
||||
}
|
||||
|
|
@ -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<usize> {
|
||||
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<ExitCode> {
|
||||
|
|
@ -72,9 +77,10 @@ fn run(cli: &Cli) -> Result<ExitCode> {
|
|||
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(),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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/<file>` / `+++ b/<file>` 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<usize> {
|
||||
/// 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<usize> {
|
||||
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<usize>
|
|||
|
||||
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 <file>` 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(|| "<unknown>".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(|| "<unknown>".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<usize> {
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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/"));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 <patch>`
|
||||
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 `<autoresolve>.md`, edit the listed files accordingly, then
|
||||
`git add <files> && git commit`.
|
||||
|
||||
c. Gate: `kei-graph-check --path sync-repo/ --after-diff <patch>`.
|
||||
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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue