From 6435e37b143a82f2d40bca23e38ff3e190860343 Mon Sep 17 00:00:00 2001 From: Nick Huo Date: Thu, 17 Sep 2026 18:17:44 -0700 Subject: [PATCH 1/2] fix: make unattended builds fail closed --- .claude/skills/porting-to-canyonos/SKILL.md | 5 + .../references/manifest.md | 6 +- .../references/validation-and-deploy.md | 9 +- cli/canyonos/build.py | 97 +++++++++++++++---- 4 files changed, 94 insertions(+), 23 deletions(-) diff --git a/.claude/skills/porting-to-canyonos/SKILL.md b/.claude/skills/porting-to-canyonos/SKILL.md index 42efab18..7d4a092b 100644 --- a/.claude/skills/porting-to-canyonos/SKILL.md +++ b/.claude/skills/porting-to-canyonos/SKILL.md @@ -8,6 +8,11 @@ description: Port existing Python agents—including LangChain, LangGraph, CrewA Requires Python, Docker, and the `canyonos` CLI. `prepare.py` uses the Python standard library; `validate.py` also requires `pyyaml`. +When invoked by an unattended `canyonos build -y`, never ask a question or wait +for approval. Use and report the documented defaults. If an action requires +approval or has no safe documented default, report it as a blocker and stop +without a question. Never deploy or ask whether to deploy from that flow. + ## Progress Copy this checklist into the response and update it while working: diff --git a/.claude/skills/porting-to-canyonos/references/manifest.md b/.claude/skills/porting-to-canyonos/references/manifest.md index 7953f2bc..47f4118e 100644 --- a/.claude/skills/porting-to-canyonos/references/manifest.md +++ b/.claude/skills/porting-to-canyonos/references/manifest.md @@ -108,9 +108,9 @@ Prefer running `canyonos config` when an interactive terminal is available; otherwise reproduce View/Change in conversation. Do not ask for derived values such as entrypoints or requirements. -An unattended `canyonos integrate` run must not block on this interaction. Use -and report the displayed defaults. Never invent EC2 infrastructure identifiers: -without them, keep the entry `local`. +An unattended `canyonos build -y` run must not block on this interaction or ask +questions. Use and report the displayed defaults. Never invent EC2 +infrastructure identifiers: without them, keep the entry `local`. ## Agent declarations diff --git a/.claude/skills/porting-to-canyonos/references/validation-and-deploy.md b/.claude/skills/porting-to-canyonos/references/validation-and-deploy.md index 630bb237..3f914b6e 100644 --- a/.claude/skills/porting-to-canyonos/references/validation-and-deploy.md +++ b/.claude/skills/porting-to-canyonos/references/validation-and-deploy.md @@ -60,14 +60,17 @@ Report: - unresolved runtime blockers; - intentionally omitted unreachable dependencies or source surfaces. -Then stop and ask exactly one direct approval question: +In an attended porting session, stop and ask exactly one direct approval +question: > Gap validation exits 0 -- static checks only; no image was built and no > request served. Run `canyonos deploy` now? This will build images and start > the deployment. -Do not treat silence, an unattended run, or the original request to “port” as -approval. +For an unattended `canyonos build -y`, instead report the validation result and +stop without asking this question. The build command only creates and validates +the port; it never deploys. Do not treat silence, an unattended run, or the +original request to “port” as approval. ## Deploy only after approval diff --git a/cli/canyonos/build.py b/cli/canyonos/build.py index fd50e094..b5469c6a 100644 --- a/cli/canyonos/build.py +++ b/cli/canyonos/build.py @@ -3,8 +3,9 @@ then launch that agent with a prompt to apply it to the current project. --agent/--scope/-y replace the two menus, so the command also runs where there -is no tty. The command's exit status is the agent's: what the port produced is -the skill's contract, not this command's. +is no tty. The command's exit status is the port's: the agent can end its +session having asked a question nobody answered, so what the port produced is +checked here with the skill's own validator rather than taken on trust. """ import os @@ -15,9 +16,10 @@ import tempfile import urllib.request -from canyonos import ui from utils.tui import select_menu +from canyonos import ui + SKILL_OWNER = "CanyonCodeCoreAI" SKILL_REPO = "canyoncodecore" SKILL_REF = "main" @@ -38,6 +40,19 @@ "modifications should be put into a new .car folder." ) +UNATTENDED_NOTE = ( + " This build is unattended: there is no terminal and nobody can answer you. " + "Do not ask questions or request approval. Use the skill's documented unattended " + "defaults and report the choices you made. Complete the whole porting checklist: " + "the port is done only when validation of .car exits 0. Stop after reporting the " + "validation result; canyonos build never deploys or asks whether to deploy. If a " + "required decision has no safe documented default, report it as a blocker and stop " + "without a question." +) + +CAR_DIR = ".car" +VALIDATOR = "validate.py" + # The leaf name of every install path must match the skill's own `name:` # frontmatter or the agent won't resolve it. @@ -113,6 +128,7 @@ def _fetch_with_git(dest): clone, ], capture_output=True, + check=False, ) if cloned.returncode != 0: return False @@ -120,6 +136,7 @@ def _fetch_with_git(dest): sparse = subprocess.run( ["git", "-C", clone, "sparse-checkout", "set", SKILL_PATH], capture_output=True, + check=False, ) skill = os.path.join(clone, SKILL_PATH) if sparse.returncode != 0 or not os.path.isdir(skill): @@ -138,9 +155,11 @@ def _fetch_with_tarball(dest): with tempfile.TemporaryDirectory() as tmp: archive = os.path.join(tmp, "repo.tar.gz") try: - with urllib.request.urlopen(TARBALL_URL, timeout=60) as response: - with open(archive, "wb") as out: - shutil.copyfileobj(response, out) + with ( + urllib.request.urlopen(TARBALL_URL, timeout=60) as response, + open(archive, "wb") as out, + ): + shutil.copyfileobj(response, out) except OSError: return False @@ -196,12 +215,13 @@ def install_skill(dest): return False -def launch_agent(agent, prompt): - """Run the agent over `prompt`. Returns its exit status, or None if there - was no agent to run. +def launch_agent(agent: str, prompt: str) -> int | None: + """Run the agent over `prompt`. Returns its exit status, or None if + there was no agent to run. - Attended, the agent owns the screen with its own TUI. Unattended it is - asked for a transcript instead, since nobody is watching one. + Attended, the agent owns the screen with its own TUI. Unattended it is asked + for a transcript instead, since nobody is watching one, and told as much in + the prompt so it stops posing questions into an empty room. """ spec = AGENTS[agent] if not shutil.which(spec["cli"]): @@ -211,16 +231,55 @@ def launch_agent(agent, prompt): # The agent's TUI wants stdin and stdout; anything less and it gets the # unattended flags instead. attended = sys.stdin.isatty() and sys.stdout.isatty() + if not attended: + prompt += UNATTENDED_NOTE argv = [spec["cli"], *([] if attended else spec["unattended"]), prompt] # No check=True: the agent exiting non-zero (including the user quitting it) # is an ordinary outcome, not something to raise a traceback over. - return subprocess.run(argv).returncode + return subprocess.run(argv, check=False).returncode -def run_build(agent=None, scope=None, yes=False): - """Install the skill and hand the port to a coding agent. +def report_port(skill_dir: str) -> bool: + """Say whether the port landed, and answer True only when it did. - True if the agent ran and exited clean. + The verdict is the skill's own step 4 -- its validator exiting 0 over the + `.car` in this directory -- so a session that stopped early fails here + instead of passing for having exited cleanly. Unverifiable is a failure: + build cannot call a port complete on evidence it never saw. + """ + validator = os.path.join(skill_dir, VALIDATOR) + if not os.path.isdir(CAR_DIR): + ui.fail(f"Port incomplete: no {CAR_DIR}/ was produced.") + return False + if not os.path.isfile(validator): + ui.fail(f"Port unverified: no {VALIDATOR} in {skill_dir}.") + return False + + check = subprocess.run( + [sys.executable, validator, CAR_DIR], + capture_output=True, + text=True, + check=False, + ) + output = "\n".join( + text.strip() for text in (check.stdout, check.stderr) if text.strip() + ) + if output: + ui.say(output) + if check.returncode == 0: + ui.ok(f"Port complete: {CAR_DIR}/ passed validation.") + return True + + ui.fail(f"Port incomplete: {CAR_DIR}/ did not pass validation.") + return False + + +def run_build( + agent: str | None = None, scope: str | None = None, yes: bool = False +) -> bool: + """Install the skill, hand the port to a coding agent, then check its work. + + True only if the port it produced validates. """ # The menus read keys off stdin and draw on stderr; without both, flags are # the only way in. @@ -252,5 +311,9 @@ def run_build(agent=None, scope=None, yes=False): return False ui.say(f"Launching {spec['label']}...") - # None (nothing on PATH) and any non-zero status are both failures. - return launch_agent(agent, BUILD_PROMPT) == 0 + status = launch_agent(agent, BUILD_PROMPT) + if status is None: + return False + if status != 0: + ui.warn(f"{spec['label']} exited with status {status}.") + return report_port(dest) From fea426a6c2b3a29ef4e0967cb82e8c5d4880ef17 Mon Sep 17 00:00:00 2001 From: Nick Huo Date: Thu, 17 Sep 2026 19:11:02 -0700 Subject: [PATCH 2/2] fix: treat -y as unattended and fail a dead build session closed -y is the documented unattended form, but launch_agent decided on TTY state alone, so `canyonos build -y` from a terminal still handed the agent its normal TUI prompt and let it pose approval questions. Pass the flag through instead. A non-zero agent status only warned, then report_port validated the relative .car regardless -- a crashed or cancelled session could pass on an artifact an earlier run left behind. A dead session is no evidence about that .car, so it now fails without consulting it. Clearing .car first is not an option: the skill's prepare.py guards it behind --refresh/--force precisely to preserve port edits. Adds the module's first tests: launch mode selection, the fail-closed paths, and report_port's verdict. Co-Authored-By: Claude Opus 5 (1M context) --- cli/canyonos/build.py | 24 ++++--- cli/cli.py | 2 +- tests/test_canyonos_build.py | 118 +++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 tests/test_canyonos_build.py diff --git a/cli/canyonos/build.py b/cli/canyonos/build.py index b5469c6a..ca02fb3d 100644 --- a/cli/canyonos/build.py +++ b/cli/canyonos/build.py @@ -3,9 +3,10 @@ then launch that agent with a prompt to apply it to the current project. --agent/--scope/-y replace the two menus, so the command also runs where there -is no tty. The command's exit status is the port's: the agent can end its -session having asked a question nobody answered, so what the port produced is -checked here with the skill's own validator rather than taken on trust. +is no tty, and -y is the unattended form whether or not one is attached. The +command's exit status is the port's: the agent can end its session having asked +a question nobody answered, so what the port produced is checked here with the +skill's own validator rather than taken on trust. """ import os @@ -215,7 +216,7 @@ def install_skill(dest): return False -def launch_agent(agent: str, prompt: str) -> int | None: +def launch_agent(agent: str, prompt: str, unattended: bool) -> int | None: """Run the agent over `prompt`. Returns its exit status, or None if there was no agent to run. @@ -228,9 +229,9 @@ def launch_agent(agent: str, prompt: str) -> int | None: ui.fail(f"`{spec['cli']}` not found on PATH; install {spec['label']} first.") return None - # The agent's TUI wants stdin and stdout; anything less and it gets the - # unattended flags instead. - attended = sys.stdin.isatty() and sys.stdout.isatty() + # The agent's TUI wants stdin and stdout; anything less -- or a caller who + # already said not to ask -- and it gets the unattended flags instead. + attended = not unattended and sys.stdin.isatty() and sys.stdout.isatty() if not attended: prompt += UNATTENDED_NOTE argv = [spec["cli"], *([] if attended else spec["unattended"]), prompt] @@ -279,7 +280,9 @@ def run_build( ) -> bool: """Install the skill, hand the port to a coding agent, then check its work. - True only if the port it produced validates. + True only if the agent ran to completion and the `.car` it left validates. + A session that died says nothing about a `.car` an earlier run may have left + in the directory, so it fails without consulting it. """ # The menus read keys off stdin and draw on stderr; without both, flags are # the only way in. @@ -311,9 +314,10 @@ def run_build( return False ui.say(f"Launching {spec['label']}...") - status = launch_agent(agent, BUILD_PROMPT) + status = launch_agent(agent, BUILD_PROMPT, unattended=yes) if status is None: return False if status != 0: - ui.warn(f"{spec['label']} exited with status {status}.") + ui.fail(f"Port incomplete: {spec['label']} exited with status {status}.") + return False return report_port(dest) diff --git a/cli/cli.py b/cli/cli.py index d79ffd3f..448e39cb 100644 --- a/cli/cli.py +++ b/cli/cli.py @@ -120,7 +120,7 @@ def add(name, run): action="store_true", help=( f"Never ask: take --agent {DEFAULT_AGENT} and --scope {DEFAULT_SCOPE} " - "for whichever of them was not given" + "for whichever of them was not given, and run the agent unattended" ), ) add("doctor", lambda args: sys.exit(0 if run_doctor() else 1)) diff --git a/tests/test_canyonos_build.py b/tests/test_canyonos_build.py new file mode 100644 index 00000000..aa7d4b11 --- /dev/null +++ b/tests/test_canyonos_build.py @@ -0,0 +1,118 @@ +import pytest + +from canyonos import build as build_cmd + + +@pytest.fixture +def buildable(monkeypatch): + """Everything run_build drives before the agent succeeds.""" + monkeypatch.setattr(build_cmd, "install_skill", lambda _dest: True) + monkeypatch.setattr(build_cmd, "report_port", lambda _skill_dir: True) + + +@pytest.fixture +def agent_on_path(monkeypatch): + """The agent CLI resolves; record the argv it would have been run with.""" + monkeypatch.setattr(build_cmd.shutil, "which", lambda _cli: f"/usr/bin/{_cli}") + calls = [] + + class Completed: + returncode = 0 + + def run(argv, **_kwargs): + calls.append(argv) + return Completed() + + monkeypatch.setattr(build_cmd.subprocess, "run", run) + return calls + + +def _set_tty(monkeypatch, attached): + monkeypatch.setattr(build_cmd.sys.stdin, "isatty", lambda: attached) + monkeypatch.setattr(build_cmd.sys.stdout, "isatty", lambda: attached) + + +def test_an_attended_launch_passes_no_unattended_flags(monkeypatch, agent_on_path): + _set_tty(monkeypatch, True) + + assert build_cmd.launch_agent("claude", "port it", unattended=False) == 0 + assert agent_on_path[0] == ["claude", "port it"] + + +def test_an_unattended_launch_keeps_its_flags_on_a_tty(monkeypatch, agent_on_path): + _set_tty(monkeypatch, True) + + build_cmd.launch_agent("claude", "port it", unattended=True) + + argv = agent_on_path[0] + assert argv[1:-1] == build_cmd.AGENTS["claude"]["unattended"] + assert argv[-1].endswith(build_cmd.UNATTENDED_NOTE) + + +def test_a_launch_without_a_tty_is_unattended(monkeypatch, agent_on_path): + _set_tty(monkeypatch, False) + + build_cmd.launch_agent("claude", "port it", unattended=False) + + argv = agent_on_path[0] + assert argv[1:-1] == build_cmd.AGENTS["claude"]["unattended"] + assert argv[-1].endswith(build_cmd.UNATTENDED_NOTE) + + +def test_a_missing_agent_cli_reports_no_status(monkeypatch): + monkeypatch.setattr(build_cmd.shutil, "which", lambda _cli: None) + + assert build_cmd.launch_agent("claude", "port it", unattended=True) is None + + +def test_yes_runs_the_agent_unattended(monkeypatch, buildable, agent_on_path): + _set_tty(monkeypatch, True) + + assert build_cmd.run_build(yes=True) is True + assert agent_on_path[0][1:-1] == build_cmd.AGENTS["claude"]["unattended"] + + +def test_a_failed_agent_never_consults_an_earlier_port(monkeypatch, buildable): + monkeypatch.setattr(build_cmd, "launch_agent", lambda *_a, **_k: 1) + monkeypatch.setattr( + build_cmd, + "report_port", + lambda _skill_dir: pytest.fail("a dead session is no evidence about .car"), + ) + + assert build_cmd.run_build(yes=True) is False + + +def test_a_missing_agent_cli_fails_the_build(monkeypatch, buildable): + monkeypatch.setattr(build_cmd, "launch_agent", lambda *_a, **_k: None) + + assert build_cmd.run_build(yes=True) is False + + +def test_a_port_with_no_car_fails(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + skill = tmp_path / "skill" + skill.mkdir() + (skill / build_cmd.VALIDATOR).write_text("") + + assert build_cmd.report_port(str(skill)) is False + + +def test_a_port_with_no_validator_fails(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + (tmp_path / build_cmd.CAR_DIR).mkdir() + + assert build_cmd.report_port(str(tmp_path / "skill")) is False + + +@pytest.mark.parametrize(("status", "passed"), [(0, True), (1, False)]) +def test_a_port_takes_its_verdict_from_the_validator( + monkeypatch, tmp_path, status, passed +): + monkeypatch.chdir(tmp_path) + (tmp_path / build_cmd.CAR_DIR).mkdir() + skill = tmp_path / "skill" + skill.mkdir() + (skill / build_cmd.VALIDATOR).write_text(f"raise SystemExit({status})") + + assert build_cmd.report_port(str(skill)) is passed