diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 021fcb9..20b1e99 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -18,6 +18,7 @@ jobs: - run: python3 scripts/check-marker-block.py - run: python3 scripts/check-marker-semantics.py - run: python3 scripts/validate-examples.py + - run: python3 scripts/check-output-contract.py - run: python3 scripts/validate-repo.py - run: python3 scripts/check-savepoint-renderer.py - run: python3 scripts/check-install-helper.py diff --git a/AGENTS.md b/AGENTS.md index 79baa2e..2ad083b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,6 +31,7 @@ python3 scripts/check-frontmatter.py python3 scripts/check-marker-block.py python3 scripts/check-marker-semantics.py python3 scripts/validate-examples.py +python3 scripts/check-output-contract.py python3 scripts/validate-repo.py python3 scripts/check-savepoint-renderer.py python3 scripts/check-install-helper.py diff --git a/README.ko.md b/README.ko.md index 0246138..8413c03 100644 --- a/README.ko.md +++ b/README.ko.md @@ -62,6 +62,52 @@ portable skill entrypoint는 `skills/savepoint/scripts/savepoint.py`입니다. r `inspect --json`은 파일과 marker가 valid이면 `0`, savepoint-like 파일을 읽었지만 invalid이면 `1`, 파일을 읽을 수 없거나 savepoint artifact가 아니면 `2`로 종료합니다. +최소 파일 흐름: + +```bash +python3 scripts/savepoint.py init-input --output .savepoint/input.json +$EDITOR .savepoint/input.json +python3 scripts/savepoint.py save --input .savepoint/input.json --output .savepoint/SAVEPOINT.md --assert-no-active-commands --scan-redaction --validate +python3 scripts/savepoint.py inspect .savepoint/SAVEPOINT.md --json +``` + +짧은 savepoint를 JSON 편집 없이 만들 때: + +```bash +python3 scripts/savepoint.py save \ + --output .savepoint/SAVEPOINT.md \ + --assert-no-active-commands --scan-redaction --validate \ + --goal "focused fix 마무리" \ + --current-state "구현은 끝난 상태" \ + --next-action "최종 검증 suite 실행" \ + --project-status passed \ + --validation-command "python3 scripts/check-savepoint-renderer.py" \ + --validation-result passed \ + --validation-summary "focused renderer checks passed" +``` + +`--scan-redaction`을 쓰면 입력 JSON을 렌더 전에 먼저 스캔합니다. `.savepoint/input.json`에 raw secret을 넣지 마세요. `--delete-input-on-success`를 추가하면 resume-ready save가 성공했을 때만 `.savepoint/input.json`을 삭제합니다. + +기존 자동화도 `scripts/savepoint.py`를 호출하게 바꿉니다. + +| 이전 호출 | 현재 호출 | +|---|---| +| `scripts/render_savepoint.py --input ...` | `scripts/savepoint.py save --input ...` | +| `scripts/validate_savepoint.py ...` | `scripts/savepoint.py validate ...` | + +프로젝트 검증 입력은 `validation.project`를 사용합니다. + +| 이전 key | 현재 field | +|---|---| +| `project_validation` | `validation.project.commands`와 `validation.project.status` | +| `skipped_checks_next_validation` | `validation.project.next_validation` | +| `smallest_next_step` | `next_action` | +| `blockers` | `unresolved_blockers` | + +`failed-expected` 또는 `not-run-justified`를 쓰면 사유와 다음 검증 명령을 함께 기록합니다. + +`validation.project.status`는 문서에 나열된 영어 값을 사용합니다. 검증 명령 `result`는 `passed` 또는 `failed`처럼 canonical English 값을 쓰고, summary와 reason은 한국어도 괜찮습니다. + ## 설치 추천 명령: @@ -76,6 +122,8 @@ python3 scripts/install.py --target codex --scope repo --apply --add-gitignore helper는 기본 dry-run입니다. 실제로 쓰려면 `--apply`가 필요합니다. repo-scope install에서 `--add-gitignore`를 주면 `.savepoint/`를 추가합니다. +Windows에서는 install helper나 일반 Git clone/worktree를 권장합니다. 일부 archive extraction 도구는 symlink를 제대로 처리하지 못할 수 있습니다. + ## Runtime boundary 일반 create/load에서는 다음만 사용합니다. @@ -107,6 +155,7 @@ python3 scripts/check-frontmatter.py python3 scripts/check-marker-block.py python3 scripts/check-marker-semantics.py python3 scripts/validate-examples.py +python3 scripts/check-output-contract.py python3 scripts/validate-repo.py python3 scripts/check-savepoint-renderer.py python3 scripts/check-install-helper.py diff --git a/README.md b/README.md index b531790..7c7037d 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,52 @@ The portable skill entrypoint is `skills/savepoint/scripts/savepoint.py`; reposi `inspect --json` exits `0` when the file and marker are valid, `1` when a savepoint-like file is parsed but invalid, and `2` when the file cannot be read or is not a savepoint artifact. +Minimal file workflow: + +```bash +python3 scripts/savepoint.py init-input --output .savepoint/input.json +$EDITOR .savepoint/input.json +python3 scripts/savepoint.py save --input .savepoint/input.json --output .savepoint/SAVEPOINT.md --assert-no-active-commands --scan-redaction --validate +python3 scripts/savepoint.py inspect .savepoint/SAVEPOINT.md --json +``` + +For a short savepoint without editing JSON: + +```bash +python3 scripts/savepoint.py save \ + --output .savepoint/SAVEPOINT.md \ + --assert-no-active-commands --scan-redaction --validate \ + --goal "finish the focused fix" \ + --current-state "implementation is done" \ + --next-action "run the final validation suite" \ + --project-status passed \ + --validation-command "python3 scripts/check-savepoint-renderer.py" \ + --validation-result passed \ + --validation-summary "focused renderer checks passed" +``` + +With `--scan-redaction`, the input JSON is scanned before rendering. Do not put raw secrets in `.savepoint/input.json`. Add `--delete-input-on-success` to remove `.savepoint/input.json` only after a resume-ready save succeeds. + +Existing automation should call `scripts/savepoint.py`. Update old root-wrapper calls as: + +| Old call | Current call | +|---|---| +| `scripts/render_savepoint.py --input ...` | `scripts/savepoint.py save --input ...` | +| `scripts/validate_savepoint.py ...` | `scripts/savepoint.py validate ...` | + +Use `validation.project` for project validation input. Replace old top-level input keys as: + +| Old key | Current field | +|---|---| +| `project_validation` | `validation.project.commands` plus `validation.project.status` | +| `skipped_checks_next_validation` | `validation.project.next_validation` | +| `smallest_next_step` | `next_action` | +| `blockers` | `unresolved_blockers` | + +For `failed-expected` or `not-run-justified`, include the reason and next validation command. + +Use the listed English `validation.project.status` values. Validation command `result` should use canonical English values such as `passed` or `failed`; summaries and reasons may be any language. + ## Install Recommended commands: @@ -76,6 +122,8 @@ python3 scripts/install.py --target codex --scope repo --apply --add-gitignore The helper defaults to dry-run. It writes files only with `--apply`. With repo-scope install, `--add-gitignore` appends `.savepoint/`. +On Windows, prefer the install helper or a normal Git clone/worktree. Archive extraction tools can mishandle symlinks. + Typical skill locations: - Codex user skill: `$HOME/.agents/skills/savepoint/` @@ -114,6 +162,7 @@ python3 scripts/check-frontmatter.py python3 scripts/check-marker-block.py python3 scripts/check-marker-semantics.py python3 scripts/validate-examples.py +python3 scripts/check-output-contract.py python3 scripts/validate-repo.py python3 scripts/check-savepoint-renderer.py python3 scripts/check-install-helper.py diff --git a/scripts/check-output-contract.py b/scripts/check-output-contract.py new file mode 100644 index 0000000..4914692 --- /dev/null +++ b/scripts/check-output-contract.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Validate evals/output-contract.json.""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + + +ROOT = Path(__file__).resolve().parents[1] +DEFAULT_PATH = ROOT / "evals" / "output-contract.json" +REQUIRED_CATEGORIES = { + "artifact-contract", + "security-redaction", + "resume-ready-semantics", + "token-budget", + "no-unwanted-files", + "least-permission", +} +REQUIRED_CASE_IDS = { + "artifact-file-mode-01", + "text-mode-no-recovery-01", + "redaction-secret-01", + "resume-ready-not-run-justified-01", + "resume-ready-failed-expected-01", + "resume-ready-failed-blocking-01", + "least-permission-01", +} + + +def validate_contract(path: Path) -> list[str]: + errors: list[str] = [] + try: + data = json.loads(path.read_text(encoding="utf-8-sig")) + except OSError as exc: + return [f"{path}: failed to read file: {exc}"] + except json.JSONDecodeError as exc: + return [f"{path}: invalid JSON: {exc}"] + + if data.get("skill_name") != "savepoint": + errors.append(f"{path}: skill_name must be savepoint") + if data.get("version") != 1: + errors.append(f"{path}: version must be 1") + + cases = data.get("cases") + if not isinstance(cases, list) or not cases: + errors.append(f"{path}: cases must be a non-empty list") + return errors + + seen_ids: set[str] = set() + categories: set[str] = set() + for index, case in enumerate(cases): + if not isinstance(case, dict): + errors.append(f"{path}: case #{index} must be an object") + continue + errors.extend(validate_case(path, index, case)) + case_id = case.get("id") + category = case.get("category") + if isinstance(case_id, str): + if case_id in seen_ids: + errors.append(f"{path}: duplicate case id: {case_id}") + seen_ids.add(case_id) + if isinstance(category, str): + categories.add(category) + + for category in sorted(REQUIRED_CATEGORIES - categories): + errors.append(f"{path}: missing category: {category}") + for case_id in sorted(REQUIRED_CASE_IDS - seen_ids): + errors.append(f"{path}: missing case id: {case_id}") + return errors + + +def validate_case(path: Path, index: int, case: dict[str, Any]) -> list[str]: + errors: list[str] = [] + for field in ["id", "category", "scenario"]: + value = case.get(field) + if not isinstance(value, str) or not value.strip(): + errors.append(f"{path}: case #{index} has invalid {field}") + must = case.get("must") + if not isinstance(must, list) or not must: + errors.append(f"{path}: case #{index} must be a non-empty list") + elif any(not isinstance(item, str) or not item.strip() for item in must): + errors.append(f"{path}: case #{index} must entries must be non-empty strings") + return errors + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--path", type=Path, default=DEFAULT_PATH) + args = parser.parse_args(argv) + + errors = validate_contract(args.path) + if errors: + for error in errors: + print(f"error: {error}", file=sys.stderr) + return 1 + print("ok: output contract") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/check-savepoint-renderer.py b/scripts/check-savepoint-renderer.py index aa53884..21ad637 100644 --- a/scripts/check-savepoint-renderer.py +++ b/scripts/check-savepoint-renderer.py @@ -19,6 +19,7 @@ SAVEPOINT_CLI = ROOT / "skills" / "savepoint" / "scripts" / "savepoint.py" ROOT_SAVEPOINT_CLI = ROOT / "scripts" / "savepoint.py" VALIDATOR = ROOT / "skills" / "savepoint" / "scripts" / "validate_savepoint.py" +OUTPUT_CONTRACT_CHECKER = ROOT / "scripts" / "check-output-contract.py" HELPER_SCRIPT_DIR = RENDER_HELPER.parent if str(HELPER_SCRIPT_DIR) not in sys.path: sys.path.insert(0, str(HELPER_SCRIPT_DIR)) @@ -65,6 +66,14 @@ def load_validator_helper(): return module +def load_savepoint_helper(): + spec = importlib.util.spec_from_file_location("savepoint_under_test", SAVEPOINT_CLI) + require(spec is not None and spec.loader is not None, "could not load savepoint CLI module") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + def git(repo: Path, *args: str) -> None: result = run(["git", *args], repo) require(result.returncode == 0, result.stderr or result.stdout) @@ -1302,11 +1311,40 @@ def test_renderer_secret_scan_blocks_resume_ready() -> None: with tempfile.TemporaryDirectory() as tmp: repo = make_repo_with_modified_app(Path(tmp)) input_path = semantic_input(repo) + secret_name = "sk-abcdefghijklmnopqrstuvwxyz123456" + (repo / f"{secret_name}.txt").write_text("do not include raw secret-like paths\n", encoding="utf-8") + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(RENDER_HELPER), + "--input", + str(input_path), + "--assert-no-active-commands", + "--scan-redaction", + "--run-savepoint-validation", + ], + repo, + ) + require(result.returncode == 2, "secret-bearing render should return unsafe status") + text = output.read_text(encoding="utf-8") + require("REDACTION_CHECKED: no" in text, "secret scan failure should not mark redaction checked") + require("RESUME_READY: no" in text, "secret scan failure should block resume-ready") + require("BLOCKERS: redaction-check-failed" in text, "secret scan blocker missing") + require(secret_name not in text, "raw secret-like path leaked into rendered output") + require("" in text, "secret-like path should be redacted in rendered output") + + +def test_renderer_rejects_secret_bearing_input_before_render() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + input_path = semantic_input(repo) + secret_value = "sk-abcdefghijklmnopqrstuvwxyz123456" payload = input_path.read_text(encoding="utf-8") input_path.write_text( payload.replace( "disk state can drift after the snapshot is captured", - "token='sk-abcdefghijklmnopqrstuvwxyz123456'", + f"token='{secret_value}'", ), encoding="utf-8", ) @@ -1323,11 +1361,11 @@ def test_renderer_secret_scan_blocks_resume_ready() -> None: ], repo, ) - require(result.returncode == 2, "secret-bearing render should return unsafe status") - text = output.read_text(encoding="utf-8") - require("REDACTION_CHECKED: no" in text, "secret scan failure should not mark redaction checked") - require("RESUME_READY: no" in text, "secret scan failure should block resume-ready") - require("BLOCKERS: redaction-check-failed" in text, "secret scan blocker missing") + combined = f"{result.stdout}\n{result.stderr}" + require(result.returncode == 1, "secret-bearing input should fail before render") + require(not output.exists(), "secret-bearing input should not write SAVEPOINT.md") + require("input JSON failed redaction scan" in result.stderr, "input redaction scan error missing") + require(secret_value not in combined, "input redaction scan must not print raw secret values") def test_renderer_redacts_secret_even_when_scan_flag_is_omitted() -> None: @@ -1397,6 +1435,305 @@ def test_savepoint_cli_save_validate_and_inspect() -> None: require("next_validation" in parsed["validation"]["project"], "inspect JSON should include project validation next command") +def test_savepoint_cli_deletes_input_on_success_only_under_savepoint_dir() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + original_input = semantic_input(repo) + input_path = repo / ".savepoint" / "input.json" + input_path.parent.mkdir(parents=True, exist_ok=True) + input_path.write_text(original_input.read_text(encoding="utf-8"), encoding="utf-8") + original_input.unlink() + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--input", + str(input_path), + "--output", + str(output), + "--assert-no-active-commands", + "--scan-redaction", + "--validate", + "--delete-input-on-success", + ], + repo, + ) + require(result.returncode == 0, result.stderr or result.stdout) + require(output.exists(), "savepoint CLI did not write SAVEPOINT.md") + require(not input_path.exists(), "successful save should delete .savepoint input when requested") + + +def test_savepoint_cli_does_not_delete_input_outside_savepoint_dir() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + input_path = semantic_input(repo) + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--input", + str(input_path), + "--output", + str(output), + "--assert-no-active-commands", + "--scan-redaction", + "--validate", + "--delete-input-on-success", + ], + repo, + ) + require(result.returncode == 0, result.stderr or result.stdout) + require(output.exists(), "savepoint CLI did not write SAVEPOINT.md") + require(input_path.exists(), "input outside .savepoint should not be deleted") + + +def test_savepoint_cli_does_not_delete_outside_symlink_to_savepoint_input() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + original_input = semantic_input(repo) + target = repo / ".savepoint" / "input.json" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(original_input.read_text(encoding="utf-8"), encoding="utf-8") + original_input.unlink() + outside = repo / "outside-input.json" + try: + outside.symlink_to(target) + except OSError as exc: + print(f"skip: test_savepoint_cli_does_not_delete_outside_symlink_to_savepoint_input ({exc})") + return + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--input", + str(outside), + "--output", + str(output), + "--assert-no-active-commands", + "--scan-redaction", + "--validate", + "--delete-input-on-success", + ], + repo, + ) + require(result.returncode == 0, result.stderr or result.stdout) + require(output.exists(), "savepoint CLI did not write SAVEPOINT.md") + require(outside.exists() or outside.is_symlink(), "input symlink outside .savepoint should not be deleted") + + +def test_delete_input_on_success_requires_path_itself_under_savepoint_dir() -> None: + helper = load_savepoint_helper() + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo(Path(tmp)) + (repo / ".savepoint").mkdir() + original_cwd = Path.cwd() + + class OutsideParent: + def resolve(self): + return repo.resolve() + + class OutsideAlias: + parent = OutsideParent() + + def is_absolute(self): + return True + + def resolve(self): + return (repo / ".savepoint" / "input.json").resolve() + + try: + os.chdir(repo) + allowed = helper.is_under_savepoint_dir(OutsideAlias()) + finally: + os.chdir(original_cwd) + require(allowed is False, "path itself must be under .savepoint before deletion is allowed") + + +def test_delete_input_on_success_handles_path_resolution_error() -> None: + helper = load_savepoint_helper() + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo(Path(tmp)) + (repo / ".savepoint").mkdir() + original_cwd = Path.cwd() + + class RaisingParent: + def resolve(self): + raise OSError("blocked") + + class RaisingPath: + parent = RaisingParent() + + def is_absolute(self): + return True + + def resolve(self): + raise OSError("blocked") + + try: + os.chdir(repo) + allowed = helper.is_under_savepoint_dir(RaisingPath()) + finally: + os.chdir(original_cwd) + require(allowed is False, "path resolution errors should deny deletion without traceback") + + +def test_savepoint_cli_keeps_input_when_save_is_not_resume_ready() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + original_input = semantic_input(repo) + input_path = repo / ".savepoint" / "input.json" + input_path.parent.mkdir(parents=True, exist_ok=True) + input_path.write_text(original_input.read_text(encoding="utf-8"), encoding="utf-8") + original_input.unlink() + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--input", + str(input_path), + "--output", + str(output), + "--scan-redaction", + "--validate", + "--delete-input-on-success", + ], + repo, + ) + require(result.returncode == 2, "unsafe savepoint should return not-ready status") + require(output.exists(), "unsafe save should still write SAVEPOINT.md") + require(input_path.exists(), "input should remain when save is not resume-ready") + + +def test_savepoint_cli_direct_flags_can_render_passed_savepoint() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--output", + str(output), + "--assert-no-active-commands", + "--scan-redaction", + "--validate", + "--goal", + "finish direct flag savepoint", + "--current-state", + "simple savepoint fields are supplied directly on the CLI", + "--next-action", + "inspect the generated savepoint", + "--project-status", + "passed", + "--validation-command", + "python scripts/check-savepoint-renderer.py", + "--validation-result", + "passed", + "--validation-summary", + "focused renderer check passed", + "--files-to-inspect-first", + "app.py", + ], + repo, + ) + require(result.returncode == 0, result.stderr or result.stdout) + text = output.read_text(encoding="utf-8") + require("RESUME_READY: yes" in text, "direct passed savepoint should be resume-ready") + require("finish direct flag savepoint" in text, "direct goal missing from savepoint") + require("app.py" in text, "direct files-to-inspect-first missing from savepoint") + + +def test_savepoint_cli_direct_flags_can_render_not_run_justified_savepoint() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--output", + str(output), + "--assert-no-active-commands", + "--scan-redaction", + "--validate", + "--goal", + "finish direct flag savepoint", + "--current-state", + "only documentation changed since the last validated state", + "--next-action", + "rerun the focused documentation validation", + "--project-status", + "not-run-justified", + "--reason", + "current change only updates savepoint documentation", + "--next-validation", + "python scripts/check-savepoint-renderer.py", + ], + repo, + ) + require(result.returncode == 0, result.stderr or result.stdout) + text = output.read_text(encoding="utf-8") + require("RESUME_READY: yes" in text, "direct not-run-justified savepoint should be resume-ready") + require("current change only updates savepoint documentation" in text, "direct reason missing from savepoint") + + +def test_savepoint_cli_rejects_input_and_direct_flags_together() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + input_path = semantic_input(repo) + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--input", + str(input_path), + "--output", + str(output), + "--goal", + "do not mix input modes", + ], + repo, + ) + require(result.returncode == 1, "CLI should reject mixed input and direct flag modes") + require("cannot combine --input with direct save flags" in result.stderr, "mixed mode error missing") + require(not output.exists(), "mixed input mode should not write SAVEPOINT.md") + + +def test_savepoint_cli_rejects_incomplete_direct_flags() -> None: + with tempfile.TemporaryDirectory() as tmp: + repo = make_repo_with_modified_app(Path(tmp)) + output = repo / ".savepoint" / "SAVEPOINT.md" + result = run( + [ + sys.executable, + str(SAVEPOINT_CLI), + "save", + "--output", + str(output), + "--goal", + "missing required direct fields", + "--current-state", + "not enough direct fields were supplied", + ], + repo, + ) + require(result.returncode == 1, "CLI should reject incomplete direct flag mode") + require("missing required direct save field" in result.stderr, "direct flag required-field error missing") + require(not output.exists(), "incomplete direct input should not write SAVEPOINT.md") + + def test_savepoint_cli_init_input_defaults_to_unknown_validation() -> None: with tempfile.TemporaryDirectory() as tmp: repo = make_repo_with_modified_app(Path(tmp)) @@ -1919,6 +2256,17 @@ def test_find_git_root_handles_empty_output() -> None: require(helper.find_git_root(ROOT) is None, "empty git root output should not crash") +def test_output_contract_checker_rejects_directory_path_without_traceback() -> None: + with tempfile.TemporaryDirectory() as tmp: + directory = Path(tmp) / "contract-dir" + directory.mkdir() + result = run([sys.executable, str(OUTPUT_CONTRACT_CHECKER), "--path", str(directory)], ROOT) + combined = f"{result.stdout}\n{result.stderr}" + require(result.returncode == 1, "output contract checker should reject directory path") + require("failed to read file" in result.stderr, "directory path should produce clean read error") + require("Traceback" not in combined, "directory path should not produce traceback") + + def test_refuses_directory_output() -> None: with tempfile.TemporaryDirectory() as tmp: repo = make_repo(Path(tmp)) @@ -2025,8 +2373,19 @@ def main() -> int: test_renderer_rejects_removed_blockers_alias, test_renderer_keeps_savepoint_unsafe_without_active_command_assertion, test_renderer_secret_scan_blocks_resume_ready, + test_renderer_rejects_secret_bearing_input_before_render, test_renderer_redacts_secret_even_when_scan_flag_is_omitted, test_savepoint_cli_save_validate_and_inspect, + test_savepoint_cli_deletes_input_on_success_only_under_savepoint_dir, + test_savepoint_cli_does_not_delete_input_outside_savepoint_dir, + test_savepoint_cli_does_not_delete_outside_symlink_to_savepoint_input, + test_delete_input_on_success_requires_path_itself_under_savepoint_dir, + test_delete_input_on_success_handles_path_resolution_error, + test_savepoint_cli_keeps_input_when_save_is_not_resume_ready, + test_savepoint_cli_direct_flags_can_render_passed_savepoint, + test_savepoint_cli_direct_flags_can_render_not_run_justified_savepoint, + test_savepoint_cli_rejects_input_and_direct_flags_together, + test_savepoint_cli_rejects_incomplete_direct_flags, test_savepoint_cli_init_input_defaults_to_unknown_validation, test_savepoint_cli_inspect_json_reports_invalid_marker, test_savepoint_cli_inspect_json_requires_valid_savepoint_for_resume_ready, @@ -2054,6 +2413,7 @@ def main() -> int: test_renderer_revalidates_final_rewrite_before_success, test_run_command_handles_oserror, test_find_git_root_handles_empty_output, + test_output_contract_checker_rejects_directory_path_without_traceback, test_refuses_directory_output, test_refuses_savepoint_named_directory_output, test_refuses_non_savepoint_output_name, diff --git a/scripts/validate-repo.py b/scripts/validate-repo.py index a308539..e8c9168 100644 --- a/scripts/validate-repo.py +++ b/scripts/validate-repo.py @@ -383,6 +383,7 @@ def validate_readme_format(self) -> None: "Savepoint", ".savepoint/SAVEPOINT.md", "scripts/savepoint.py", + "scripts/check-output-contract.py", "scripts/validate-repo.py", "python3 -m compileall", ]: @@ -398,6 +399,7 @@ def validate_readme_format(self) -> None: "Savepoint", ".savepoint/SAVEPOINT.md", "scripts/savepoint.py", + "scripts/check-output-contract.py", "scripts/validate-repo.py", "python3 -m compileall", ]: @@ -627,6 +629,7 @@ def validate_manual_eval_cases(self) -> None: if phrase not in text: self.fail(f"{path.relative_to(ROOT)} missing eval phrase: {phrase}") output_contract = ROOT / "evals" / "output-contract.json" + self.require_exists(ROOT / "scripts" / "check-output-contract.py") self.require_exists(output_contract) if not output_contract.exists(): return diff --git a/skills/savepoint/SKILL.md b/skills/savepoint/SKILL.md index bed058f..1ab7dcd 100644 --- a/skills/savepoint/SKILL.md +++ b/skills/savepoint/SKILL.md @@ -27,7 +27,7 @@ Native slash-command support depends on the client. If slash prompts are not pas - Prefer current files, Git state, and durable state files over chat memory. - Do not paste transcripts, full diffs, long logs, shell history, PRDs, ADRs, issues, or commits. - Reference existing artifacts by path, URL, branch, or commit. -- Redact API keys, tokens, cookies, credentials, private keys, passwords, `.env` values, and PII as ``. +- Redact API keys, tokens, cookies, credentials, private keys, passwords, `.env` values, and PII as ``; do not place raw secrets in semantic input files. - File savepoints must end with exactly one `SAVEPOINT_V1` marker block. - Keep top-level `SAVEPOINT.md` compact. Use generated `details/*.md` only when needed for recovery. @@ -44,6 +44,8 @@ python3 /scripts/savepoint.py save --input .savepoint/input Inside this repository, `python3 scripts/savepoint.py save ...` also works. +For simple savepoints, direct flags such as `--goal`, `--current-state`, `--next-action`, and `--project-status` may replace `--input`; do not combine direct flags with `--input`. Add `--delete-input-on-success` only when `.savepoint/input.json` should be removed after a resume-ready save. + 5. Inspect only the generated `.savepoint/SAVEPOINT.md`. 6. Report exact path, `RESUME_READY`, blockers if any, and the first next action. diff --git a/skills/savepoint/references/contract.md b/skills/savepoint/references/contract.md index 5fd13ea..4858136 100644 --- a/skills/savepoint/references/contract.md +++ b/skills/savepoint/references/contract.md @@ -33,6 +33,8 @@ Project validation statuses: - `failed-blocking`: resume-ready is not allowed. - `not-run-unknown`: resume-ready is not allowed. +Use these exact English status values in `validation.project.status`. Command `result` values should be canonical English values such as `passed` or `failed`; command summaries, reasons, and next-validation notes may be any language. + `VALIDATION_RECORDED: yes` records both savepoint artifact validation and an honest project validation posture. It is not a claim that the project validation passed. ## Load Report diff --git a/skills/savepoint/references/safety.md b/skills/savepoint/references/safety.md index 3031006..2b10013 100644 --- a/skills/savepoint/references/safety.md +++ b/skills/savepoint/references/safety.md @@ -1,8 +1,8 @@ # Savepoint Safety -Scan generated savepoint artifacts by default, not the whole repository. +Scan semantic input files and generated savepoint artifacts by default, not the whole repository. -Never copy these values into `SAVEPOINT.md`, `details/*.md`, or text mode output: +Never copy these values into `.savepoint/input.json`, `SAVEPOINT.md`, `details/*.md`, or text mode output: - API keys, tokens, cookies, credentials, private keys, passwords - full `.env` values @@ -10,7 +10,7 @@ Never copy these values into `SAVEPOINT.md`, `details/*.md`, or text mode output - raw logs that may contain secrets - unnecessary PII -Use `` for required mentions. +Use `` for required mentions. With `--scan-redaction`, secret-like values in the input JSON fail before rendering so the raw value is not copied into `SAVEPOINT.md`. Secret-like paths such as `.env`, `id_rsa`, `id_ed25519`, `*.pem`, `*.p12`, `*.pfx`, `credentials.json`, or service-account files may be named by path when needed, but do not read or quote their contents. diff --git a/skills/savepoint/scripts/render_savepoint.py b/skills/savepoint/scripts/render_savepoint.py index 2001673..110db66 100644 --- a/skills/savepoint/scripts/render_savepoint.py +++ b/skills/savepoint/scripts/render_savepoint.py @@ -84,6 +84,18 @@ def read_input(path: Path) -> tuple[dict[str, Any] | None, str | None]: return data, None +def input_redaction_scan_error(path: Path) -> str | None: + try: + text = path.read_text(encoding="utf-8-sig") + except OSError as exc: + return f"failed to read input JSON: {exc}" + errors: list[str] = [] + scan_secret_patterns(path, text, errors) + if errors: + return "input JSON failed redaction scan; redact or remove secret-like values before rendering" + return None + + def clean_text(value: Any, *, fallback: str = "not recorded") -> str: if value is None: return fallback @@ -647,6 +659,12 @@ def main(argv: list[str] | None = None) -> int: print(f"error: {preflight_error}", file=sys.stderr) return 1 + if args.scan_redaction: + input_scan_error = input_redaction_scan_error(args.input) + if input_scan_error: + print(f"error: {input_scan_error}", file=sys.stderr) + return 1 + data, input_error = read_input(args.input) if input_error or data is None: print(f"error: {input_error}", file=sys.stderr) diff --git a/skills/savepoint/scripts/savepoint.py b/skills/savepoint/scripts/savepoint.py index 61a50df..f49f04a 100644 --- a/skills/savepoint/scripts/savepoint.py +++ b/skills/savepoint/scripts/savepoint.py @@ -6,6 +6,7 @@ import argparse import json import sys +import tempfile from pathlib import Path import render_savepoint @@ -28,7 +29,7 @@ def parse_args(argv: list[str]) -> argparse.Namespace: subcommands = parser.add_subparsers(dest="command", required=True) save = subcommands.add_parser("save", help="Create or refresh a file savepoint.") - save.add_argument("--input", required=True, type=Path, help="JSON file with semantic savepoint input.") + save.add_argument("--input", type=Path, help="JSON file with semantic savepoint input.") save.add_argument("--output", type=Path, help="Savepoint path to write.") save.add_argument("--force", action="store_true", help="Overwrite an existing output file.") save.add_argument( @@ -38,6 +39,30 @@ def parse_args(argv: list[str]) -> argparse.Namespace: ) save.add_argument("--scan-redaction", action="store_true", help="Scan generated text for secret patterns.") save.add_argument("--validate", action="store_true", help="Run bundled savepoint validation after writing.") + save.add_argument( + "--delete-input-on-success", + action="store_true", + help="After a resume-ready save, delete --input only when it is under .savepoint/.", + ) + save.add_argument("--goal", help="Direct save input: current goal.") + save.add_argument("--current-state", help="Direct save input: current state.") + save.add_argument("--next-action", help="Direct save input: next action.") + save.add_argument( + "--project-status", + choices=sorted(render_savepoint.PROJECT_VALIDATION_STATUSES), + help="Direct save input: validation.project.status.", + ) + save.add_argument("--reason", help="Direct save input: project validation reason.") + save.add_argument("--next-validation", help="Direct save input: next validation command.") + save.add_argument("--validation-command", help="Direct save input: recorded validation command.") + save.add_argument("--validation-result", help="Direct save input: recorded validation result.") + save.add_argument("--validation-summary", help="Direct save input: recorded validation summary.") + save.add_argument( + "--files-to-inspect-first", + nargs="*", + default=None, + help="Direct save input: focused paths for the next agent to inspect first.", + ) save.add_argument( "--run-savepoint-validation", action="store_true", @@ -71,8 +96,8 @@ def parse_args(argv: list[str]) -> argparse.Namespace: return parser.parse_args(argv) -def render_save_argv(args: argparse.Namespace) -> list[str]: - argv = ["--input", str(args.input)] +def render_save_argv(args: argparse.Namespace, input_path: Path) -> list[str]: + argv = ["--input", str(input_path)] if args.output is not None: argv.extend(["--output", str(args.output)]) if args.force: @@ -86,6 +111,163 @@ def render_save_argv(args: argparse.Namespace) -> list[str]: return argv +DIRECT_SAVE_FLAG_ATTRS = [ + ("goal", "--goal"), + ("current_state", "--current-state"), + ("next_action", "--next-action"), + ("project_status", "--project-status"), + ("reason", "--reason"), + ("next_validation", "--next-validation"), + ("validation_command", "--validation-command"), + ("validation_result", "--validation-result"), + ("validation_summary", "--validation-summary"), + ("files_to_inspect_first", "--files-to-inspect-first"), +] + + +def direct_save_flags_present(args: argparse.Namespace) -> list[str]: + present: list[str] = [] + for attr, flag in DIRECT_SAVE_FLAG_ATTRS: + if getattr(args, attr) is not None: + present.append(flag) + return present + + +def direct_save_validation_error(args: argparse.Namespace) -> str | None: + required = [ + ("goal", "--goal"), + ("current_state", "--current-state"), + ("next_action", "--next-action"), + ("project_status", "--project-status"), + ] + missing = [flag for attr, flag in required if not clean_text(getattr(args, attr), fallback="").strip()] + if missing: + return f"missing required direct save field(s): {', '.join(missing)}" + + validation_fields = [ + ("validation_command", "--validation-command"), + ("validation_result", "--validation-result"), + ("validation_summary", "--validation-summary"), + ] + validation_present = any(clean_text(getattr(args, attr), fallback="").strip() for attr, _flag in validation_fields) + validation_missing = [flag for attr, flag in validation_fields if not clean_text(getattr(args, attr), fallback="").strip()] + status = args.project_status + if status in {"passed", "failed-expected"} or validation_present: + if validation_missing: + return f"missing required direct save field(s): {', '.join(validation_missing)}" + if status in {"failed-expected", "not-run-justified"}: + missing_reason = [] + if not clean_text(args.reason, fallback="").strip(): + missing_reason.append("--reason") + if not clean_text(args.next_validation, fallback="").strip(): + missing_reason.append("--next-validation") + if missing_reason: + return f"missing required direct save field(s): {', '.join(missing_reason)}" + if status == "failed-blocking" and not validation_present and not clean_text(args.reason, fallback="").strip(): + return "missing required direct save field(s): --reason or validation command fields" + return None + + +def direct_save_data(args: argparse.Namespace) -> dict[str, object]: + project: dict[str, object] = { + "status": args.project_status, + "reason": args.reason or "", + "commands": [], + "next_validation": args.next_validation or "", + } + if args.validation_command or args.validation_result or args.validation_summary: + project["commands"] = [ + { + "command": args.validation_command or "", + "result": args.validation_result or "", + "summary": args.validation_summary or "", + } + ] + return { + "goal": args.goal, + "current_state": args.current_state, + "next_action": args.next_action, + "unresolved_blockers": "none", + "files_to_inspect_first": args.files_to_inspect_first or [], + "validation": {"project": project}, + } + + +def write_direct_input(args: argparse.Namespace) -> Path: + handle = tempfile.NamedTemporaryFile( + "w", + encoding="utf-8", + newline="\n", + suffix=".json", + prefix="savepoint-direct-", + delete=False, + ) + with handle: + json.dump(direct_save_data(args), handle, ensure_ascii=True, indent=2) + handle.write("\n") + return Path(handle.name) + + +def is_under_savepoint_dir(path: Path) -> bool: + try: + base = (Path.cwd() / ".savepoint").resolve() + absolute = path if path.is_absolute() else Path.cwd() / path + absolute.parent.resolve().relative_to(base) + resolved = absolute.resolve() + resolved.relative_to(base) + return resolved.is_file() + except (ValueError, OSError, RuntimeError): + return False + + +def delete_input_on_success(args: argparse.Namespace, exit_code: int) -> None: + if not args.delete_input_on_success or args.input is None or exit_code != 0: + return + if not is_under_savepoint_dir(args.input): + return + input_path = args.input if args.input.is_absolute() else Path.cwd() / args.input + try: + input_path.unlink() + except OSError as exc: + print(f"warning: failed to delete input JSON: {exc}", file=sys.stderr) + + +def run_save(args: argparse.Namespace) -> int: + direct_flags = direct_save_flags_present(args) + if args.input is not None and direct_flags: + print(f"error: cannot combine --input with direct save flags: {', '.join(direct_flags)}", file=sys.stderr) + return 1 + if args.input is None and not direct_flags: + print("error: save requires --input or direct save flags", file=sys.stderr) + return 1 + + temporary_input: Path | None = None + input_path = args.input + if input_path is None: + direct_error = direct_save_validation_error(args) + if direct_error: + print(f"error: {direct_error}", file=sys.stderr) + return 1 + try: + input_path = write_direct_input(args) + except OSError as exc: + print(f"error: failed to write direct save input: {exc}", file=sys.stderr) + return 1 + temporary_input = input_path + + try: + exit_code = render_savepoint.main(render_save_argv(args, input_path)) + finally: + if temporary_input is not None: + try: + temporary_input.unlink() + except OSError: + pass + + delete_input_on_success(args, exit_code) + return exit_code + + def run_validate(args: argparse.Namespace) -> int: argv: list[str] = [] if args.allow_example_paths: @@ -233,7 +415,7 @@ def run_text(args: argparse.Namespace) -> int: def main(argv: list[str] | None = None) -> int: args = parse_args(sys.argv[1:] if argv is None else argv) if args.command == "save": - return render_savepoint.main(render_save_argv(args)) + return run_save(args) if args.command == "init-input": return run_init_input(args) if args.command == "validate":