diff --git a/config.sample.toml b/config.sample.toml index 1527dead3..b48cf5517 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -148,6 +148,17 @@ model_report_regex = "(?m)^model:[ \\t]*([^ \\t\\r\\n]+)[ \\t]*\\r?$" # ~/.local/share/opencode/auth.json ({"openrouter": {"type": "api", "key": "..."}}); # confirm the path with your installed CLI. # Per-task engine_args can set reasoning effort ("--variant", "low|high|max"). +# +# Because that wrapper confines writes, the lint warns when a task declares a +# deliverable OUTSIDE its task dir and the check does not export it there — +# otherwise the worker does the work and physically cannot deliver it, and the +# eval row blames the model. Ringer recognises the wrappers it ships by name. +# For a sandbox wrapper of your own, say so outright: +# confines_writes_to_taskdir = true # or false to silence the rule +# Omit it and ringer infers: codex's "workspace-write" sandbox_args, or one of +# this repo's own wrappers. An engine it cannot read stays silent rather than +# warning from a guess. +# # Uncomment and set an absolute path for `bin` to enable. # [engines.opencode] # bin = "/absolute/path/to/ringer/engines/opencode-sandboxed.sh" diff --git a/ringer.py b/ringer.py index 1c782912e..8d58ed010 100755 --- a/ringer.py +++ b/ringer.py @@ -759,6 +759,10 @@ class EngineConfig: # its own "model" — this is what makes a harness engine (OpenCode) model # agnostic instead of hard-coding one model into the command line. model_default: str = "" + # Whether this engine's sandbox confines worker writes to the task dir. + # None means "work it out" (see engine_confines_writes_to_taskdir); set it + # explicitly for a wrapper whose boundary ringer cannot read from here. + confines_writes_to_taskdir: bool | None = None @property def process_name(self) -> str: @@ -1620,6 +1624,17 @@ def load_engines(raw: Any) -> dict[str, EngineConfig]: model_default = str( section.get("model_default", base.model_default if base else "") ).strip() + raw_confines = section.get("confines_writes_to_taskdir") + if raw_confines is None: + confines_writes_to_taskdir = ( + base.confines_writes_to_taskdir if base is not None else None + ) + elif isinstance(raw_confines, bool): + confines_writes_to_taskdir = raw_confines + else: + raise ValueError( + f"engines.{clean_name}.confines_writes_to_taskdir must be true or false" + ) engines[clean_name] = EngineConfig( name=clean_name, bin=bin_path, @@ -1629,6 +1644,7 @@ def load_engines(raw: Any) -> dict[str, EngineConfig]: token_regex=token_regex, model_report_regex=model_report_regex, model_default=model_default, + confines_writes_to_taskdir=confines_writes_to_taskdir, ) return engines @@ -2184,15 +2200,72 @@ def unreachable_deliverable_findings(manifest: Manifest) -> list[str]: return findings +# Wrappers THIS repo ships whose sandbox confines worker writes to the task +# directory. Recognising them by name is not the guess the old docstring +# warned against: the Seatbelt profile lives in engines/ a few lines from +# here, and its writable surface is exactly TASKDIR + the per-run scratch. +SANDBOXING_ENGINE_BINS = frozenset( + {"opencode-sandboxed.sh", "opencode-sandboxed-linux.sh"} +) + + def engine_confines_writes_to_taskdir(engine: "EngineConfig") -> bool: """True when the engine's sandbox limits worker writes to the task dir. - Keyed on the codex CLI's "workspace-write" vocabulary because that is the - only sandbox whose write boundary is legible from config alone. opencode's - confinement lives inside its wrapper script, invisible from here — better - to stay silent than to warn from a guess. + Three sources, most explicit first: + + 1. The engine block says so outright (`confines_writes_to_taskdir`), which + is how a wrapper ringer cannot read declares its own boundary. + 2. The codex CLI's "workspace-write" vocabulary in sandbox_args. + 3. One of the sandboxing wrappers this repo ships, by bin name. + + (3) closes a real gap: opencode's confinement was called "invisible from + here" and the lint stayed silent, so on 2026-08-16 a site-build run + declared its deliverables inside a session scratchpad the Seatbelt profile + denies, and two lanes did the work, could not deliver it, and were logged + as model failures. The boundary was never invisible — it ships in this + repo. """ - return any("workspace-write" in arg for arg in engine.sandbox_args) + if engine.confines_writes_to_taskdir is not None: + return engine.confines_writes_to_taskdir + if any("workspace-write" in arg for arg in engine.sandbox_args): + return True + return Path(engine.bin).name in SANDBOXING_ENGINE_BINS + + +# Commands that put a file where they are pointed. A redirect is handled +# separately; these are the copy-shaped verbs a check uses to export. +CHECK_WRITE_VERBS = ("cp", "mv", "install", "tee", "rsync", "ln") + + +def check_appears_to_write(check: str, forms: Iterable[str]) -> bool: + """True when the check plausibly CREATES the path, not merely names it. + + Naming is not writing. `python3 verify.py /abs/out.html` passes the path as + an ARGUMENT — it reads the file the worker was supposed to produce, which + is precisely the failure this lint exists to catch. A substring test cannot + tell the two apart, and treating any mention as the check-exports design is + what let the 2026-08-16 site-build run through: its check was a bare script + call carrying the deliverable path, exactly the "opaque check" the lint's + own docstring said should still warn. + + Evidence of writing is a redirect onto the path, or a copy-shaped verb + somewhere in the same command segment. Anything else warns and asks the + author to confirm. + """ + for form in forms: + if not form: + continue + quoted = re.escape(form) + # `> path`, `>> path`, with or without quoting. + if re.search(rf">>?\s*['\"]?{quoted}", check): + return True + # `cp ... path`, `tee path`, ... bounded to one command segment so a + # copy earlier in the check cannot vouch for an unrelated later path. + verbs = "|".join(CHECK_WRITE_VERBS) + if re.search(rf"\b(?:{verbs})\b[^;&|\n]*{quoted}", check): + return True + return False def sandbox_unreachable_deliverable_findings( @@ -2233,15 +2306,17 @@ def sandbox_unreachable_deliverable_findings( resolved = resolve_for_compare(written) if resolved == taskdir or taskdir in resolved.parents: continue - # The check names the path — the check-exports pattern. - if any(form in task.check for form in (rel, str(written), str(resolved))): + # The check WRITES the path — the check-exports pattern. Merely + # naming it is not enough; see check_appears_to_write. + if check_appears_to_write(task.check, (rel, str(written), str(resolved))): continue findings.append( f"{task.key}: deliverable {rel} is outside the task directory, and " f"{task.engine}'s sandbox confines worker writes to the task directory — " "the worker cannot create it there. Have the CHECK export it (checks run " "unsandboxed), or the task fails with the work complete and the retry " - "fails identically." + "fails identically. If your check already creates this file by some means " + "this lint cannot see, that is what this warning is asking you to confirm." ) return findings diff --git a/tests/test_lint.py b/tests/test_lint.py index f27ebaa5a..44cca6257 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -11,6 +11,8 @@ ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(ROOT)) +from dataclasses import replace as dataclass_replace # noqa: E402 + from ringer import ( # noqa: E402 AppConfig, ArtifactConfig, @@ -729,13 +731,19 @@ def test_w12_full_access_task_is_quiet(self) -> None: self.assertEqual([], findings) def test_w12_unconfined_engine_is_quiet(self) -> None: - # opencode's confinement lives in its wrapper script, invisible from - # config. Warning on a guess trains authors to ignore the warning. + # REVERSED 2026-08-16. This asserted that opencode must stay silent + # because its confinement was "invisible from config" — but the + # Seatbelt profile ships in engines/, so the boundary was always + # knowable, and the silence cost two lanes on the ohalloran-demonstrator + # run. The surviving principle is narrower and still holds: never warn + # for an engine whose boundary is genuinely unknown (see + # test_an_unknown_engine_bin_stays_silent). The shipped wrapper now + # warns, as it should have all along. findings = self.w12_findings( {"engine": "opencode", "check": "bash /work/check.sh"}, ["/exports/cttc/review-r1.md"], ) - self.assertEqual([], findings) + self.assertEqual(1, len(findings), findings) def test_w12_no_config_is_silent(self) -> None: # Plain `ringer.py lint` may run with no loadable config; the rule @@ -868,6 +876,109 @@ def test_w13_is_a_warning_not_a_blocking_error(self) -> None: findings = self.w13_findings({"engine": "codex"}) self.assertFalse(findings[0].startswith("ERROR:"), findings[0]) + # --- sandbox-unreachable deliverables ----------------------------------- + + DELIVERABLE = "/tmp/elsewhere/candidate.html" + + def unreachable_findings( + self, + *, + check: str, + engine: str = "opencode", + full_access: bool = False, + ) -> list[str]: + extra: dict[str, object] = {"engine": engine, "check": check} + if full_access: + extra["full_access"] = True + return self.w12_findings(extra, [self.DELIVERABLE]) + + def test_opencode_wrapper_sandbox_is_visible_to_the_lint(self) -> None: + # Regression for the 2026-08-16 ohalloran-demonstrator run: deliverables + # declared in a session scratchpad the Seatbelt profile denies. Both + # opencode lanes did the work, could not deliver it, and were logged + # failure_class=model. The lint stayed silent because opencode's + # boundary was treated as unknowable. + findings = self.unreachable_findings( + check="python3 '/tmp/assets/check-candidate.py' '/tmp/elsewhere/candidate.html'" + ) + self.assertEqual(1, len(findings), findings) + self.assertIn("candidate.html", findings[0]) + + def test_a_check_that_only_names_the_path_still_warns(self) -> None: + # The half that actually let the run through. A bare script call + # carrying the deliverable as an ARGUMENT reads it; it does not create + # it. The old substring test read that as the check-exports design. + findings = self.unreachable_findings( + check="verify.sh /tmp/elsewhere/candidate.html || exit 1" + ) + self.assertEqual(1, len(findings), findings) + + def test_a_check_that_exports_the_deliverable_stays_quiet(self) -> None: + # Checks run unsandboxed, so a deliverable outside the taskdir is + # legitimate exactly when the check puts it there. + for check in ( + "cat out.html > /tmp/elsewhere/candidate.html", + "cp out.html /tmp/elsewhere/candidate.html", + "tee /tmp/elsewhere/candidate.html < out.html", + ): + with self.subTest(check=check): + self.assertEqual([], self.unreachable_findings(check=check)) + + def test_a_copy_elsewhere_does_not_vouch_for_a_later_path(self) -> None: + # Segment-bounded: an unrelated cp earlier in the check must not + # exempt a deliverable named later by a read-only verifier. + findings = self.unreachable_findings( + check="cp a.txt b.txt; verify.sh /tmp/elsewhere/candidate.html" + ) + self.assertEqual(1, len(findings), findings) + + def findings_with_engine(self, **engine_overrides: object) -> list[str]: + """Lint the same task against a variant of the opencode engine block.""" + config = self.w12_config() + engine = dataclass_replace(config.engines["opencode"], **engine_overrides) + config = dataclass_replace( + config, engines={**config.engines, "opencode": engine} + ) + task = self.task(check="verify.sh x", expect_files=[self.DELIVERABLE]) + task["engine"] = "opencode" + return [ + item + for item in lint_manifest(self.manifest([task]), config=config) + if "sandbox confines worker writes" in item + ] + + def test_engine_may_declare_its_own_boundary(self) -> None: + # A wrapper ringer cannot read declares it outright... + self.assertEqual( + 1, + len( + self.findings_with_engine( + bin="/opt/my-wrapper.sh", confines_writes_to_taskdir=True + ) + ), + ) + # ...and an explicit false wins over the shipped-wrapper name. + self.assertEqual([], self.findings_with_engine(confines_writes_to_taskdir=False)) + + def test_an_unknown_engine_bin_stays_silent(self) -> None: + # The original principle holds: never warn from a guess. + self.assertEqual([], self.findings_with_engine(bin="/usr/local/bin/some-cli")) + + def test_full_access_tasks_are_exempt(self) -> None: + # --no-sandbox is wired as full_access_args, so there is no boundary. + self.assertEqual( + [], + self.unreachable_findings( + check="verify.sh /tmp/elsewhere/candidate.html", full_access=True + ), + ) + + def test_sandbox_unreachable_is_a_warning_not_a_blocking_error(self) -> None: + findings = self.unreachable_findings( + check="verify.sh /tmp/elsewhere/candidate.html" + ) + self.assertFalse(findings[0].startswith("ERROR:"), findings[0]) + def test_templates_are_clean(self) -> None: # Every kit ships one or more manifest skeletons (manifest.json plus # optional manifest-round*.json for multi-round kits).