PKG-13: the no-op half of the pre-deploy check has never run as documented
checks / unit (pull_request) Successful in 6m19s

⚠️ THE INVOCATION ON PKG-09's CARD AND IN THE TOOL'S OWN DOCSTRING RUNS ONLY
HALF THE CHECK. The container runs as uid 0 against a checkout owned by the
operator, and git refuses that:

    fatal: detected dubious ownership in repository at '/w/goodwe-addon'

`_git` returned non-zero, `last_version_change()` returned None, and the
silent-no-op check - THE REASON PKG-09 EXISTS - reported SKIPPED. Measured, on
the documented command against release/1.0:

    ⚠️  SKIPPED: could not ask git what changed since the last bump;
                 the no-op check is INCONCLUSIVE, not clean
    NOT CLEARED

and with `--user $(id -u):$(id -g)`, unchanged otherwise:

    no runtime change since the last bump - nothing to deploy
    CLEARED for deploy.

⚠️ THE TWO HALVES WERE MUTUALLY EXCLUSIVE ACROSS THE TWO DOCUMENTED
ENVIRONMENTS. In the container the validator ran and git skipped; on a bare host
git ran and the validator skipped on aiohttp. NEITHER DOCUMENTED COMMAND RAN
BOTH. The pre-0.9.0 clearance was sound only because it happened to be driven as
two separate invocations.

⚠️ AND IT WAS INVISIBLE IN THE ONLY CASE ANYONE HAD RUN. `main()` consults the
git result only when `shipped == deployed`; every real run so far compared 0.7.0
against 0.9.0 and printed its verdict without ever needing git.

FIXED IN `_git()` WITH `safe.directory`, NOT BY DOCUMENTING `--user`. The tool
should be correct however it is invoked rather than requiring every future
caller to remember a flag; the read is read-only, on a repository the operator
is about to deploy from.

⚠️ WHY THE TESTS COULD NOT SEE IT, WHICH IS THE FINDING BEHIND THE FINDING.
`test_version_gate.py` invokes the gate with `sys.executable` against fixtures
it creates as its own uid, so no fixture could ever meet an ownership mismatch.
MUTATION.md habit 4 in a fourth shape, inside the file written to prevent it -
after the pickaxe bug, after the one-bump history, after the filter-only
survivor. Two fixtures added:

  - a directory that is NOT a git repository, so the lookup genuinely fails:
    the gate must exit 2 saying INCONCLUSIVE and must NOT say CLEARED.
  - ⚠️ THE EXIT CODES PINNED BY VALUE, NOT BY TRUTHINESS. `returncode != 0` is
    satisfied by collapsing NOT-CLEARED into REFUSED, which review found the
    earlier draft could not tell apart. 1 says "this deploy is wrong"; 2 says
    "nobody checked". Driven: collapsing 2 into 1 now goes red on exactly that
    check.

Suite: 26 files, all pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
glenn schrooyen
2026-09-07 17:02:01 +02:00
co-authored by Claude Opus 5
parent d0633665c9
commit 0b653c35a6
2 changed files with 75 additions and 2 deletions
+38
View File
@@ -231,6 +231,44 @@ check("...and because config.yaml IS a runtime path, that cosmetic commit is "
r8.returncode == 1 and "COSMETIC" in r8.stdout)
print()
print("⭐ A GIT LOOKUP THAT GENUINELY FAILS - THE SHAPE NO FIXTURE COULD REACH")
# ⚠️ EVERY FIXTURE ABOVE CREATES ITS OWN REPOSITORY AS ITS OWN UID, so none of
# them could ever meet the failure that actually happened: the documented
# invocation runs the container as uid 0 against a checkout owned by the
# operator, git refuses with "detected dubious ownership", `_git` returns
# non-zero, and the no-op half reports SKIPPED. It degraded honestly - exit 2,
# never a false clearance - but the check this file exists for had never once
# run in its documented form, and nothing here could see that.
#
# `_git` now sets `safe.directory`, so ownership no longer breaks it. This
# section grades the REMAINING failure mode: git answering not at all.
root5 = tempfile.mkdtemp(prefix="test_version_gate.nogit.")
tmpdirs.append(root5)
addon5 = os.path.join(root5, "goodwe_controller")
os.makedirs(addon5)
write(addon5, "config.yaml", 'name: t\nversion: "0.7.0"\n') # NOT a git repo
check("a repository git cannot answer for reports no last-bump commit",
last_version_change(root5) is None)
r9 = run_gate(addon5, root5, "0.7.0", opts_file(VALID))
check("⭐ AND THE GATE DOES NOT CLEAR ON IT. An unanswerable git lookup is "
"INCONCLUSIVE, not clean - the one thing it must never do is call a "
"tree it could not inspect deployable",
r9.returncode != 0 and "CLEARED for deploy" not in r9.stdout)
check("...it says INCONCLUSIVE in those words", "INCONCLUSIVE" in r9.stdout)
# ⚠️ AND THE EXIT CODES ARE PINNED BY VALUE, NOT BY TRUTHINESS. `returncode != 0`
# is satisfied by collapsing NOT-CLEARED into REFUSED, which review found the
# earlier draft could not tell apart. 1 and 2 mean different things to an
# operator: 1 says "this deploy is wrong", 2 says "nobody checked".
check("NOT CLEARED is exactly 2, not merely non-zero", r9.returncode == 2)
r10 = run_gate(addon3, root3, "0.7.0", opts_file(dict(VALID, slew_w=1000)))
check("REFUSED is exactly 1, not merely non-zero",
r10.returncode == 1 and "REFUSED" in r10.stdout)
print("\nthe shipped tree's own version is readable")
check("shipped_version() finds a version", bool(shipped_version(HERE)))
+37 -2
View File
@@ -84,8 +84,43 @@ def is_runtime(path):
def _git(*args, repo=None):
return subprocess.run(["git", "-C", repo or os.path.dirname(ADDON), *args],
capture_output=True, text=True)
"""git, run so it answers regardless of who owns the checkout.
⚠️ WITHOUT `safe.directory` THIS TOOL SILENTLY DID HALF ITS JOB. The
invocation this file's own docstring documents runs the container as uid 0
against a checkout owned by the operator, and git refuses that:
fatal: detected dubious ownership in repository at '/w/goodwe-addon'
`_git` then returned non-zero, `last_version_change()` returned None, and
the no-op check reported SKIPPED - so **the silent-no-op half, which is the
reason this file exists, had never once run in its documented form.**
It degraded honestly: exit 2, INCONCLUSIVE, never a false clearance. But a
check that cannot fire is not a check, and the two documented environments
were mutually exclusive - in the container the validator ran and git
skipped; on a bare host git ran and the validator skipped on aiohttp.
Neither documented command ran both halves.
⚠️ AND IT WAS INVISIBLE IN THE ONLY CASE ANYONE HAD RUN. `main()` consults
the git result only when `shipped == deployed`; every real run so far
compared 0.7.0 against 0.9.0 and printed its verdict without needing git.
`test_version_gate.py` could not see it either: it invokes the gate with
`sys.executable` against fixtures it creates as its own uid, so no fixture
ever met the ownership mismatch. MUTATION.md habit 4, in a fourth shape,
inside the file written to prevent it.
`safe.directory` is the right fix rather than `--user`: it makes the tool
correct however it is invoked, instead of making every future caller
remember a flag. The read is read-only, on a repository the operator is
about to deploy from.
"""
where = repo or os.path.dirname(ADDON)
return subprocess.run(
["git", "-c", f"safe.directory={where}", "-c", "safe.directory=*",
"-C", where, *args],
capture_output=True, text=True)
def _version_at(sha, repo=None, path=CONFIG_PATH):