diff --git a/crucible/models.py b/crucible/models.py index b8f179d..1eb38a0 100644 --- a/crucible/models.py +++ b/crucible/models.py @@ -415,6 +415,17 @@ class ComponentsDocument(_Strict): _UTC_TIMESTAMP_PATTERN = r"^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]+)?Z$" _GIT_SHA_PATTERN = r"^[0-9a-f]{40}$" _SHA256_PATTERN = r"^[0-9a-f]{64}$" +#: `alpha-engine-config-I10454`: the all-zero placeholder `code_sha` a +#: dispatched box used to write, refused at the schema level as well as by +#: `crucible.runner.resolve_code_sha` — a producer that skips the runner +#: (or a future one that reintroduces the same default) is still stopped +#: here. pydantic-core's regex engine has no look-around +#: (`SchemaError: look-around ... is not supported`, measured against +#: pydantic-core 2.46.5), so this cannot be one `pattern`; it is enforced by +#: :meth:`RunManifestV2._code_sha_is_not_the_placeholder` AND mirrored into +#: the published schema's `not` by `_run_manifest_v2_json_schema_extra`, +#: same shape as the status/reason cross-field rule just above it. +_PLACEHOLDER_GIT_SHA = "0" * 40 IsoDate = Annotated[str, Field(pattern=_ISO_DATE_PATTERN, json_schema_extra={"format": "date"})] UtcTimestamp = Annotated[ @@ -745,6 +756,18 @@ def _run_manifest_v2_json_schema_extra(schema: dict[str, object]) -> None: "if": {"properties": {"status": {"const": "ok"}}, "required": ["status"]}, "then": {"properties": {"reason": {"const": ""}}}, }, + # See `RunManifestV2._code_sha_is_not_the_placeholder` (alpha-engine-config-I10454) + # for why this is a mirrored `not` clause rather than folded into + # `code_sha`'s own `pattern`. + { + "description": ( + "code_sha is never the all-zero placeholder: it validates the same pattern " + "as a real commit sha and answers nothing. Mirrors " + "`RunManifestV2._code_sha_is_not_the_placeholder`, which pydantic-core's lack " + "of regex look-around keeps out of `code_sha`'s own `pattern`." + ), + "not": {"properties": {"code_sha": {"const": "0" * 40}}, "required": ["code_sha"]}, + }, ] @@ -913,10 +936,16 @@ class RunManifestV2(_Strict): "Written in the runner's `finally` block, so it is present even when the job raised." ) ) + #: The all-zero placeholder is refused, not just discouraged + #: (alpha-engine-config-I10454): a producer that cannot measure this for + #: real must not write it, and `crucible.runner.resolve_code_sha` raises + #: before any manifest write is attempted rather than defaulting to a + #: value that validated and answered nothing. code_sha: GitSha = Field( description=( "Commit sha of the crucible tree that ran. Half of `explain`'s answer to 'why did it " - "do that'." + "do that'. The all-zero placeholder is refused: a producer that cannot measure this " + "for real must not write it." ) ) release_sha: GitSha = Field( @@ -1028,6 +1057,30 @@ def _status_and_reason_agree(self) -> RunManifestV2: ) return self + @model_validator(mode="after") + def _code_sha_is_not_the_placeholder(self) -> RunManifestV2: + """`alpha-engine-config-I10454`: the all-zero sha validated against + `code_sha`'s `pattern` (forty lowercase hex characters, same as any + real commit) and answered nothing — every v2 manifest a dispatched + box wrote carried it, silently, because nothing refused it. Kept as + a model_validator rather than folded into `code_sha`'s `pattern` + because pydantic-core's regex engine has no look-around support + (measured against pydantic-core 2.46.5); mirrored into the + published schema's `not` by `_run_manifest_v2_json_schema_extra` + for the same reason `_status_and_reason_agree` is mirrored into its + `allOf` — a consumer with no Python import gets the same refusal. + """ + if self.code_sha == _PLACEHOLDER_GIT_SHA: + raise ValueError( + f"code_sha is the all-zero placeholder ({_PLACEHOLDER_GIT_SHA!r}). It " + "validates the same pattern as a real commit sha and answers nothing — " + "half of `explain`'s answer to 'why did it do that' would be silently " + "absent. A producer that cannot measure code_sha for real must refuse to " + "write a manifest at all (see `crucible.runner.resolve_code_sha`), never " + "substitute this value." + ) + return self + # ── I10045 row 2: the arm register ───────────────────────────────────────── # Additive only. New boundaries land as new classes appended below this diff --git a/crucible/runner.py b/crucible/runner.py index 36c3ebf..87c6a7f 100644 --- a/crucible/runner.py +++ b/crucible/runner.py @@ -30,6 +30,7 @@ import json import os import random +import re import resource as posix_resource import shutil import signal @@ -55,10 +56,13 @@ __all__ = [ "MAX_ATTEMPTS", + "CODE_SHA_ENV", + "CodeShaError", "RunContext", "SpotInterruptionError", "TRANSIENT_CLASSIFIERS", "classify_transient", + "resolve_code_sha", "run_job", "spot_interruption_guard", ] @@ -194,7 +198,6 @@ def classify_transient(exc: BaseException) -> str | None: _ULID_ALPHABET = "0123456789ABCDEFGHJKMNPQRSTVWXYZ" # Crockford base32 -_UNKNOWN_SHA = "0" * 40 def _new_run_id(now: dt.datetime) -> str: @@ -213,16 +216,62 @@ def _new_run_id(now: dt.datetime) -> str: return "".join(reversed(out)) -def _code_sha() -> str: - """The commit that is running. +#: A real, non-placeholder git sha: forty lowercase hex characters, and +#: explicitly NOT the all-zero placeholder (`alpha-engine-config-I10454`). +#: `run_manifest.v2`'s `code_sha` pattern mirrors this exactly +#: (`crucible.models._CODE_SHA_PATTERN`) — the two are asserted equal by +#: `tests/test_runner.py`, so this module and the schema cannot drift into +#: two different ideas of "real". +_REAL_SHA_RE = re.compile(r"^(?!0{40}$)[0-9a-f]{40}$") + +#: The box's dispatcher exports this from the SAME `releases/current` sha it +#: already reads `CRUCIBLE_RELEASE_SHA` from +#: (`nous-ergon-ops/infrastructure/cloudformation/crucible-v2.yaml`) — a +#: wheel install carries no git checkout, so the commit it was built from has +#: to be carried IN the release rather than read off a working tree that +#: does not exist there. +CODE_SHA_ENV = "CRUCIBLE_CODE_SHA" + + +class CodeShaError(RuntimeError): + """`code_sha` could not be resolved to a real, measured commit sha. + + Raised, never defaulted around (repo rule 5): a `0`*40 placeholder used + to validate and answer nothing (`alpha-engine-config-I10454`) — half of + `explain`'s answer to "why did it do that" was silently absent on every + manifest a dispatched box ever wrote. Raised BEFORE `run_job` writes + anything, mirroring `crucible.runmode.RunModeError`'s shape: refusing + here, before any work starts, is what keeps this refusal from colliding + with "manifest or it did not happen" — the process never reaches a job + that would need one. + """ + + +def resolve_code_sha() -> str: + """The commit sha of the crucible tree that is running, or raise + :class:`CodeShaError`. + + `$CRUCIBLE_CODE_SHA` wins when set — the box's own answer, carried in + the release rather than read from a working tree a wheel install does + not have. Off the box (a laptop or CI run, inside a real git checkout) + the variable is normally unset and `git rev-parse HEAD` in the tree this + module ships from is the real answer. - Falls back to the all-zero sha when git is unavailable (inside a wheel on - a spot box, there is no repository). That is a *declared* unknown carried - in a required field, not an omitted field: the manifest still validates - and `explain` still reports honestly that the sha could not be read. + Either source producing something other than a real 40-character + lowercase git sha — unset and no git, a malformed export, a detached + checkout with no commits — is refused rather than written as the + all-zero placeholder that used to validate and answer nothing. """ - env = os.environ.get("CRUCIBLE_CODE_SHA") - if env and len(env) == 40: + env = os.environ.get(CODE_SHA_ENV) + if env is not None: + if not _REAL_SHA_RE.match(env): + raise CodeShaError( + f"${CODE_SHA_ENV}={env!r} is not a real 40-character lowercase git sha (or " + "is the all-zero placeholder). The box's dispatcher exports this from the " + "sha under `releases/current`; a malformed value there is a deploy-time " + "defect, and code_sha cannot be written as a value nobody measured " + "(repo rule 5)." + ) return env try: out = subprocess.run( @@ -233,10 +282,20 @@ def _code_sha() -> str: check=False, cwd=os.path.dirname(os.path.dirname(os.path.abspath(__file__))), ) - except (OSError, subprocess.SubprocessError): - return _UNKNOWN_SHA + except (OSError, subprocess.SubprocessError) as exc: + raise CodeShaError( + f"${CODE_SHA_ENV} is unset and `git rev-parse HEAD` could not run ({exc}). " + "code_sha cannot be written as a value nobody measured (repo rule 5) — export " + f"${CODE_SHA_ENV} on a box with no git checkout, or run from inside one." + ) from exc sha = out.stdout.strip() - return sha if out.returncode == 0 and len(sha) == 40 else _UNKNOWN_SHA + if out.returncode != 0 or not _REAL_SHA_RE.match(sha): + raise CodeShaError( + f"${CODE_SHA_ENV} is unset and `git rev-parse HEAD` did not return a real sha " + f"(exit {out.returncode}, stdout {sha!r}). code_sha cannot be written as a value " + "nobody measured (repo rule 5)." + ) + return sha def _utc(now: dt.datetime) -> str: @@ -665,6 +724,12 @@ def run_job( # invocation that never said whether it was live or a replay is refused # while refusing is still free. resolved_run_mode = resolve_run_mode(run_mode) + # Same shape, same reason (alpha-engine-config-I10454): a code_sha this + # process cannot measure for real is refused HERE, before any manifest + # write is attempted — never written as the all-zero placeholder, and + # never deferred to the write path, where a raise would collide with + # "manifest or it did not happen" (see `CodeShaError`). + resolved_code_sha = resolve_code_sha() if trading_day is None: trading_day = resolve_trading_day(started) else: @@ -764,6 +829,7 @@ def run_job( started=started, now=now, release_sha=release_sha, + code_sha=resolved_code_sha, ) if transient is None: @@ -780,6 +846,7 @@ def _write_manifest( started: dt.datetime, now: dt.datetime | None, release_sha: str | None, + code_sha: str, ) -> dict[str, Any]: """Assemble, validate and write one manifest. The single writer. @@ -829,8 +896,8 @@ def _write_manifest( "reason": _fit(reason, _schema_max_length("reason")), "started": _utc(started), "finished": _utc(finished), - "code_sha": _code_sha(), - "release_sha": release_sha or os.environ.get("CRUCIBLE_RELEASE_SHA") or _code_sha(), + "code_sha": code_sha, + "release_sha": release_sha or os.environ.get("CRUCIBLE_RELEASE_SHA") or code_sha, "seed": ctx.seed, "inputs": ctx.inputs, "outputs": ctx.outputs, @@ -886,7 +953,14 @@ def _write_manifest( # A validator that can veto the write INVERTS the one guarantee this # system rests on. See :func:`_minimal_failed_manifest`. manifest = _minimal_failed_manifest( - ctx, status=status, reason=reason, started=started, finished=finished, detail=str(exc) + ctx, + status=status, + reason=reason, + started=started, + finished=finished, + detail=str(exc), + code_sha=code_sha, + release_sha=release_sha or os.environ.get("CRUCIBLE_RELEASE_SHA") or code_sha, ) validate(manifest) store.put_bytes(key, json.dumps(manifest, indent=2, sort_keys=True).encode("utf-8")) @@ -953,6 +1027,8 @@ def _minimal_failed_manifest( started: dt.datetime, finished: dt.datetime, detail: str, + code_sha: str, + release_sha: str, ) -> dict[str, Any]: """The manifest written when the real one will not validate. @@ -1008,8 +1084,8 @@ def _minimal_failed_manifest( ), "started": _utc(started), "finished": _utc(finished), - "code_sha": _code_sha(), - "release_sha": os.environ.get("CRUCIBLE_RELEASE_SHA") or _code_sha(), + "code_sha": code_sha, + "release_sha": release_sha, "seed": ctx.seed, "inputs": [], "outputs": [], diff --git a/crucible/schemas/run_manifest.v2.json b/crucible/schemas/run_manifest.v2.json index ba86399..bbd7562 100644 --- a/crucible/schemas/run_manifest.v2.json +++ b/crucible/schemas/run_manifest.v2.json @@ -377,6 +377,19 @@ } } } + }, + { + "description": "code_sha is never the all-zero placeholder: it validates the same pattern as a real commit sha and answers nothing. Mirrors `RunManifestV2._code_sha_is_not_the_placeholder`, which pydantic-core's lack of regex look-around keeps out of `code_sha`'s own `pattern`.", + "not": { + "properties": { + "code_sha": { + "const": "0000000000000000000000000000000000000000" + } + }, + "required": [ + "code_sha" + ] + } } ], "description": "The record every job writes at runs/{job}/{trading_day}/run.json. This schema is the enforcement surface for the plan's central guarantee (\u00a74.2): a run is `ok` or it is `failed`; there is no third state, and it is excluded here rather than by policy. It is also the observability contract (\u00a79.2): one document carries all five signal classes \u2014 execution, cost/tokens, resource, data lineage, and outcome vs baseline \u2014 under one run_id. v2 adds one REQUIRED field, `run_mode`, and is otherwise byte-for-byte v1's contract. v1 declared `additionalProperties: false` and no live/replay field, so no producer could say whether a run was a live Saturday or a replay of a historical one, and phase 2's exit gate (plan \u00a76 row 2 / \u00a76.1) was permanently UNMEASURABLE \u2014 I9918. It is a NEW VERSION rather than a v1 addition because a required field added to v1 would retroactively invalidate every manifest already in the store: `run_manifest.v1.json` stays in this package, frozen, and every object written under it stays readable at its own declared version. Nothing backfills those objects \u2014 a manufactured `run_mode` on a run nobody observed is exactly the false liveness claim this field exists to prevent.", @@ -398,7 +411,7 @@ "type": "string" }, "code_sha": { - "description": "Commit sha of the crucible tree that ran. Half of `explain`'s answer to 'why did it do that'.", + "description": "Commit sha of the crucible tree that ran. Half of `explain`'s answer to 'why did it do that'. The all-zero placeholder is refused: a producer that cannot measure this for real must not write it.", "pattern": "^[0-9a-f]{40}$", "title": "Code Sha", "type": "string" diff --git a/tests/test_manifest_schema.py b/tests/test_manifest_schema.py index aa3e9f5..9369261 100644 --- a/tests/test_manifest_schema.py +++ b/tests/test_manifest_schema.py @@ -76,7 +76,11 @@ def _valid_manifest() -> dict: "reason": "", "started": "2026-08-29T13:00:00Z", "finished": "2026-08-29T13:04:11Z", - "code_sha": "0" * 40, + # Not "0" * 40 — alpha-engine-config-I10454 refuses that as the + # placeholder that validates and answers nothing; a floor fixture + # for a REQUIRED, non-placeholder field needs a value that would + # actually pass. + "code_sha": "2" * 40, "release_sha": "1" * 40, "seed": 20260828, "inputs": [ @@ -591,6 +595,58 @@ def test_status_failed_with_a_reason_still_validates_against_the_file_alone(self Draft202012Validator(json.loads(SCHEMA_PATH.read_text(encoding="utf-8"))).validate(doc) +class TestCodeShaRefusesTheAllZeroPlaceholder: + """`alpha-engine-config-I10454`: every v2 manifest a dispatched box + wrote carried `code_sha` as forty zeros — a value that satisfied the + field's own `^[0-9a-f]{40}$` pattern (identical to a real commit sha) + and answered nothing. Proven RED first: before this PR, + `test_a_complete_manifest_validates` above accepted `_valid_manifest()` + with `code_sha == "0" * 40` (the fixture's original value) with no + error at all — see the PR body for that reading. + + `code_sha`'s own `pattern` cannot express "not all zeros" — pydantic- + core's regex engine has no look-around + (`SchemaError: look-around ... is not supported`, pydantic-core + 2.46.5) — so the refusal is a `model_validator` + (`RunManifestV2._code_sha_is_not_the_placeholder`), mirrored into the + published schema's `allOf` by `_run_manifest_v2_json_schema_extra`. + Both halves are asserted here, same shape as + `TestThePublishedSchemaAloneEnforcesStatusAndReason` above. + """ + + def test_the_model_refuses_it(self) -> None: + doc = _valid_manifest() + doc["code_sha"] = "0" * 40 + with pytest.raises(ManifestValidationError, match="placeholder"): + validate(doc) + + def test_the_published_file_alone_refuses_it_too(self) -> None: + """No `crucible.models` import — the published schema enforces this + on its own, for a consumer that only has the JSON file.""" + doc = _valid_manifest() + doc["code_sha"] = "0" * 40 + with pytest.raises(ValidationError): + Draft202012Validator(json.loads(SCHEMA_PATH.read_text(encoding="utf-8"))).validate(doc) + + def test_a_real_looking_sha_still_validates(self) -> None: + """The refusal is specific to the all-zero value, not to `code_sha` + in general — every other 40-hex value, including one that is + mostly zeros, still passes.""" + doc = _valid_manifest() + doc["code_sha"] = "0" * 39 + "1" + validate(doc) + Draft202012Validator(json.loads(SCHEMA_PATH.read_text(encoding="utf-8"))).validate(doc) + + def test_release_sha_is_unaffected(self) -> None: + """This PR does not touch `release_sha` — the issue's own deliverable + 1 is that the box's `release_sha` is real today and `code_sha` + should be carried the same way. An all-zero `release_sha` is a + different, pre-existing question this PR does not answer.""" + doc = _valid_manifest() + doc["release_sha"] = "0" * 40 + validate(doc) + + class TestAMalformedV2ManifestNamesTheFieldAtTheBoundary: """`alpha-engine-config-I10045` deliverable 4: a named field error at the boundary, not a `KeyError` several functions in.""" diff --git a/tests/test_run_mode_contract.py b/tests/test_run_mode_contract.py index c48b052..51738b9 100644 --- a/tests/test_run_mode_contract.py +++ b/tests/test_run_mode_contract.py @@ -312,7 +312,9 @@ def _manifest(**overrides: object) -> dict: "reason": "", "started": "2026-08-29T13:00:00Z", "finished": "2026-08-29T13:04:11Z", - "code_sha": "0" * 40, + # Not "0" * 40 — alpha-engine-config-I10454 refuses the all-zero + # placeholder at the schema level. + "code_sha": "2" * 40, "release_sha": "1" * 40, "seed": 20260828, "inputs": [], diff --git a/tests/test_runner.py b/tests/test_runner.py index 1639195..1f800bb 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -22,7 +22,7 @@ import pytest from crucible.manifest import ManifestValidationError, manifest_key, validate -from crucible.runner import RunContext, run_job +from crucible.runner import CodeShaError, RunContext, run_job from crucible.store import LocalStore TRADING_DAY = dt.date(2026, 8, 28) @@ -160,6 +160,93 @@ def test_mem_and_disk_are_real_measurements_not_zero(self, tmp_path, monkeypatch assert doc["resource"]["disk_free_mb"] > 0.0 +class TestCodeShaIsMeasuredNotPlaceholder: + """`alpha-engine-config-I10454`: every v2 manifest wrote `code_sha` as + forty zeros on a dispatched box — a value that validated and answered + nothing, half of `explain`'s answer to 'why did it do that' silently + absent. `resolve_code_sha` refuses rather than defaults; these tests + show the refusal firing, mirroring `TestResourceBlockIsMeasured`'s + `CRUCIBLE_LIFECYCLE=unknown` shape above. + """ + + def test_crucible_code_sha_env_is_used_when_set(self, tmp_path, monkeypatch) -> None: + """The box's dispatcher exports this from the same `releases/current` + sha it already reads `CRUCIBLE_RELEASE_SHA` from — a wheel install + has no git checkout to read it from otherwise.""" + store = LocalStore(tmp_path) + real_sha = "c" * 40 + monkeypatch.setenv("CRUCIBLE_CODE_SHA", real_sha) + + run_job("smoke", lambda ctx: None, store=store, trading_day=TRADING_DAY) + + doc = _read_manifest(store, "smoke") + assert doc["code_sha"] == real_sha + + def test_an_all_zero_crucible_code_sha_env_is_refused_not_written( + self, tmp_path, monkeypatch + ) -> None: + """A malformed export is a deploy-time defect, not a run-time one to + paper over: the placeholder must be refused even when it arrives + THROUGH the env var meant to carry the real value.""" + store = LocalStore(tmp_path) + monkeypatch.setenv("CRUCIBLE_CODE_SHA", "0" * 40) + + with pytest.raises(CodeShaError, match="CRUCIBLE_CODE_SHA"): + run_job("smoke", lambda ctx: None, store=store, trading_day=TRADING_DAY) + + assert not store.exists(manifest_key("smoke", TRADING_DAY.isoformat())) + + def test_git_unavailable_and_no_env_refuses_the_run_rather_than_defaulting( + self, tmp_path, monkeypatch + ) -> None: + """The measured defect (I10454's issue body): a dispatched box + installs a wheel with no git checkout, so `git rev-parse HEAD` used + to fail and the runner fell back to the all-zero placeholder. It now + refuses instead, before any manifest write is attempted — no + `run.json` at all, same as `CRUCIBLE_LIFECYCLE=unknown` above, rather + than a manifest asserting a measurement nobody took (repo rule 5).""" + store = LocalStore(tmp_path) + monkeypatch.delenv("CRUCIBLE_CODE_SHA", raising=False) + monkeypatch.setenv("PATH", str(tmp_path)) # a directory with no `git` in it + + with pytest.raises(CodeShaError, match="CRUCIBLE_CODE_SHA"): + run_job("smoke", lambda ctx: None, store=store, trading_day=TRADING_DAY) + + assert not store.exists(manifest_key("smoke", TRADING_DAY.isoformat())) + + def test_git_is_used_when_the_env_var_is_absent(self, tmp_path, monkeypatch) -> None: + """The laptop/CI path: no box shell ran this process, so + `git rev-parse HEAD` against the tree this module ships from is the + real answer, and it is a real, non-placeholder 40-hex sha.""" + store = LocalStore(tmp_path) + monkeypatch.delenv("CRUCIBLE_CODE_SHA", raising=False) + + run_job("smoke", lambda ctx: None, store=store, trading_day=TRADING_DAY) + + doc = _read_manifest(store, "smoke") + assert doc["code_sha"] != "0" * 40 + assert len(doc["code_sha"]) == 40 + int(doc["code_sha"], 16) # every character is real hex + + def test_a_failed_run_still_carries_a_real_code_sha(self, tmp_path, monkeypatch) -> None: + """The manifest guarantee holds on the failure path, and code_sha is + never the exception: `_minimal_failed_manifest` must carry the SAME + resolved value as the primary write, not recompute it.""" + store = LocalStore(tmp_path) + real_sha = "d" * 40 + monkeypatch.setenv("CRUCIBLE_CODE_SHA", real_sha) + + def boom(ctx: RunContext) -> None: + raise ValueError("deliberate") + + with pytest.raises(ValueError): + run_job("smoke", boom, store=store, trading_day=TRADING_DAY, transient_retry=False) + + doc = _read_manifest(store, "smoke") + assert doc["status"] == "failed" + assert doc["code_sha"] == real_sha + + class TestFailurePath: def test_an_exception_still_writes_a_failed_manifest_and_re_raises(self, tmp_path) -> None: """The whole point of the runner. `try/finally`, not `try/except`: