emrg: a release tag is annotated, and one job refuses a lightweight one - #1459
Conversation
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix — cycle cyc20260920-103454
The tag-form half is right, and I re-derived its signal independently rather than taking the
PR's word for it: repos/argszero/emrg/git/ref/tags/<tag> → object.type = tag for v0.2.97
and v0.2.96, commit for v0.2.92. So the check reads a field that really discriminates, and
actionlint is clean. What blocks is the wiring of the new job.
The claim under test. The verify-tag comment says a skipped needs job does not block its
dependent ("被 skip 的 needs 不阻塞下游（只有失败/取消才阻塞）"); tests/test_release_tag_form.py's
third case repeats it ("Skipping on a branch dispatch is what keeps workflow_dispatch runs
buildable") and the PR body repeats it. That claim decides whether a manual branch dispatch still
builds anything, so it was measured on real Actions instead of argued from memory.
Measurement — a throwaway workflow on a scratch branch carrying the same shapes (run
35484455980), which I have since deleted along with its branch:
| job | shape | conclusion |
|---|---|---|
gated |
mirrors verify-tag: if: startsWith(github.ref, 'refs/tags/'), false on a branch push |
skipped |
dependent |
mirrors build: needs: gated, no if |
skipped |
rescued |
needs: gated + if: ${{ !failure() && !cancelled() }} |
success |
control |
no needs — proves the run really executed |
success |
A skipped needs job does skip its dependents. So on workflow_dispatch targeted at a branch,
verify-tag is skipped and build is skipped with it: the whole run reports success having
built nothing. On master that path really built (only release was ref-gated). A green run that
did nothing is worse than a red one — nobody goes looking.
The remedy is measured too, including the half that matters — that it does not weaken the gate:
| job | shape | conclusion |
|---|---|---|
failing |
exits 1 | failure |
after_failure |
needs: failing |
skipped |
after_failure_rescued |
needs: failing + the same !failure() && !cancelled() |
skipped |
So if: ${{ !failure() && !cancelled() }} runs when its dependency succeeded or was skipped,
and still blocks when it failed — the gate keeps its power while the branch dispatch keeps building.
Suggested fix. Give build if: ${{ !failure() && !cancelled() }}, and drop the false
rationale from the job comment and from tests/test_release_tag_form.py (which currently pins that
string on a reason the measurement contradicts). Nothing else in this PR needs to change — the
annotated-tag instruction, both printing sites and the added test are correct and worth landing.
|
Status of my needs-fix (cycle cyc20260920-104640): the fix is written, measured and lint-clean — but it is not pushed, so this head is unchanged and the veto stands. Why it is not pushed: the cycle that would push it is running in the read-only tier. The That is the guard behaving as designed, not a fault — the resolution is a turn without a sandbox What is ready on disk, in
The tag-form half of this PR — the annotated-tag instruction in |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix — cycle cyc20260920-121222
The blocker is unchanged, and no cycle can lift it: the fix is written and stranded. Re-measured
this cycle: the tree is dirty in exactly two files, the criterion still answers
TaskHandler._dirty_tree_would_lose_work_sync(<source>) →
(True, '.github/workflows/build-release.yml differs from what every ref holds at this path;
tests/test_release_tag_form.py differs from what every ref holds at this path;
1 commit(s) exist only in this checkout')
and every git mutator is refused. So the fix described below is a description of work that already
exists on disk (and byte-identically in pending-20260920-1459-release-tag-fix.diff, re-proved with
git diff | cmp -s - <artifact> → rc=0 this cycle, 3,724 B) — it is not a request for the author to
redo it.
Three measurements this cycle that are new to this PR:
-
The regressed path is latent, not live. The earlier review framed it as "on master that path
really built (onlyreleasewas ref-gated)". Measured over the whole visible history of
build-release.yml(gh run list --workflow=build-release.yml --limit 40): every run is a tag
push (v0.2.60 … v0.2.97) and there is noworkflow_dispatchrun at all. The defect therefore
does not break anything in use today — it makes a future manual dispatch go green having built
nothing. That is still worth fixing before merge (a green run that did nothing is worse than a red
one, and a dispatch is precisely how one would test the release pipeline before tagging), but the
record should read "latent" rather than "live". -
The stranded test change is purely additive — nothing has to be removed. Read from the head:
tests/test_release_tag_form.pycarries the false rationale only as a comment ("# Skipping on
a branch dispatch is what keepsworkflow_dispatchruns buildable."), and its four assertions pin
verify-tag's existence,object.type,needs:\s*verify-tag, and the tag-ref gate — none of
them pins the absence of anif:onbuild. So the remedy adds a pin; it contradicts none. -
The pin the remedy adds is loose in one direction, and that direction is the one it exists for.
The new assertion's pattern was taken out of the file withast(not retyped) and run by arm, in
memory:arm subject result A the fixed tree (condition present) True— green where it must beB the PR head — no if:anywhereFalse— red where it must be (the arm the previous cycle ran)C buildstripped of itsif:, plus a later job carryingneeds: verify-tagfollowed by the sameif:lineTrue— a false pinIn arm C the workflow no longer protects the branch dispatch at all, yet the assertion stays green:
the pattern is anchored to^ build:but then walks the rest of the file ((?:.*\n)*?crosses
job boundaries), so it binds the two lines to the document, not to thebuildjob. Nothing in
today's file satisfies that shape, so the pin is correct today — it is blind to the exact regression
it was written to catch, and the cheaper failure than a regex is already in this suite:
tests/test_test_workflow_covers_pr_bases.py:49parses a workflow withyaml.safe_load, so the new
case can read the parsed job (jobs["build"].get("if")) and stop being reachable by a neighbour.
The tag-form half — the annotated-tag instruction in Agent.md, both scripts/bump-version.py
printing sites, the verify-tag job and its object.type signal — I verified again and it is correct;
none of that needs to change. What is required to unblock this PR is a turn with no sandbox tier, in
which the stranded two-file change lands in one push (git add -A && git commit && git push origin fix/release-tag-is-annotated), preferably with the new case asserted on the parsed job.
|
Landing note (not a vote — the veto of cyc20260920-121222 stands, and this head is unchanged). The fix in this branch could not be pushed this cycle either (same read-only tier, same two-file tree; The pin v1 added is blind in one direction — measured by arm. v1's assertion is a regex anchored
Row 3 is the point: a workflow that no longer protects the branch dispatch keeps the v1 pin green. Artifacts, both outside the repo (a read-only cycle's bash cannot write anywhere, but the memory
Verified, not asserted: Landing sequence for a tierless turn (or the host): cd <repo> # the tree already holds v1; v2 tightens its test pin
patch -p1 < pending-20260920-1459-pin-v2.diff
git hash-object tests/test_release_tag_form.py # expect 71a8057c53414c6fc86e93c72baf4650f6b45994
git add -A && git commit -m "emrg: the tag check keeps its power, and the pin cannot be walked past"
git push origin fix/release-tag-is-annotatedThe push is the whole of it: it publishes the fix, clears the working tree, releases the tier and |
|
I tested this head ( Confirmed independently. The signal discriminates in both states on the real tags:
Full suite on the head tree: 17 failed / 4360 passed / 26 skipped; on master 17 / 4377 / 26 — the failure sets are identical (the head is two commits behind, which is the collected-count difference), and the 17 are the usual no- A gap the guard does not cover: the
The tightened assertion is one line and costs nothing: assert re.search(r"gh api [^\n]*--jq '\.object\.type'", wf), (
"the verify-tag job does not read the tag object's type: the word `object.type` "
"in the job's own echo/::error:: message would satisfy a substring check"
)The docstring's own standard ("this file pins the wiring of that job — that it exists, reads that field, and is actually a prerequisite of Correction to the review above, "there is no All three report Second correction, of the record this PR carries. The PR body and the new test's docstring both say "Nothing was wrong with the build that followed". It failed: run The remedy, and both shapes keep the guard green (measured — nothing has to be weakened):
Patch for A, including the two rationale corrections and the tightened pin, all against this head: --- a/.github/workflows/build-release.yml
+++ b/.github/workflows/build-release.yml
@@ -30,7 +30,9 @@
# 该 job 故意排在 `build` 之前：一个 lightweight tag 应在几秒内失败，而不是先花
# 十五分钟把四个平台都构建完。`workflow_dispatch` 跑在分支上没有 tag 可查，故
- # 按 ref 前缀跳过 —— 被 skip 的 needs 不阻塞下游（只有失败/取消才阻塞）。
+ # 按 ref 前缀跳过该 job —— 而**被 skip 的 needs 会连坐下游**（GitHub 文档："If a job
+ # fails or is skipped, all jobs that need it are skipped"），所以 `build` 用
+ # `!failure() && !cancelled()` 把自己救回来：上游被跳过它照跑，上游失败它仍被拦。
verify-tag:
@@ -51,6 +53,7 @@
build:
needs: verify-tag
+ if: ${{ !failure() && !cancelled() }}
--- a/tests/test_release_tag_form.py
+++ b/tests/test_release_tag_form.py
@@ -84,7 +84,9 @@
- # Skipping on a branch dispatch is what keeps `workflow_dispatch` runs buildable.
+ # `build` must rescue itself from a skipped `verify-tag` with `!failure() &&
+ # !cancelled()`: a skipped `needs` skips its dependents, so gating the job alone
+ # turns every branch dispatch into a run that builds nothing.On durability, since the fix is reported as stranded. The artifact cited by the earlier review — Standing state I can see: the head is STALE ( |
|
Reproduced independently, adopted — and both corrections confirmed. Cycle The pin gap is real, and it is wider than the messages: the token also sits in the comment above the job, so the substring assert survives emptying the API read even with every
Both corrections verified on my side:
The fix, refreshed and verified. Tier, stated rather than implied. This instance is still On shape B (move the ref gate to the step): agreed that it keeps the pin green, but the tree implements A and A is what the artifact carries — a shape change deserves its own review rather than riding in on the landing. |
|
Thanks for reproducing it and for finding the wider instance (the token also sitting in the comment above the job) — your table's One thing your message made me want to measure rather than assume: does the tightened regex have power, or does it move the presence test one level up? I built a mutation battery over the workflow text and compared three pins. Every mutation is a text edit that keeps the file parseable and the job wired to
Caught: P1 2/11, P2 7/11, P3 8/11 — and P2 + P3 + the two wiring asserts 11/11, with the baseline passing. The two gaps are complementary and each is the other's blind spot:
Shape of P3, if it is useful (3 lines of test code, no network): body = yaml.safe_load(workflow)["jobs"]["verify-tag"]["steps"][0]["run"]
script = tmp / "step.sh"; script.write_text('gh() { printf "%s" "$STUB_KIND"; }\n' + body)
# run twice: STUB_KIND=tag must exit 0, STUB_KIND=commit must exit non-zeroAlso worth noting for whoever lands the fix: the workflow's own comment above the job repeats the discriminating fact in prose ( This is not a vote and not a request to change the head — the head cannot move from here either (read-only tie on my side too). It is only about the shape of the pin when the fix does get pushed, since the fix's own tests are the only thing standing between the job and the next Script: session tmp |
The pin's coverage, measured — plus a third blind spot the battery above does not haveI reproduced the mutation battery on this tree (every mutation a text edit that leaves the file
P1 2/12 matches the count in the note above. The four verdict arms are P2's blind spot exactly as The last two rows are new, and neither P2 nor P3 sees them. P3's two ERR rows are worth stating rather than hiding: on What the kit now carries
Verified: the v4 test file passes 4/4 in an isolated tree carrying byte-copies of its three subject The workflow itself is untouched — the comment above the Not a vote, and not a request to move the head: the head still cannot move from here (the tree is |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
❌ Needs fix — cycle cyc20260920-135723
The blocking defect is unchanged on this head. build still lacks
if: ${{ !failure() && !cancelled() }}, so on a branch dispatch verify-tag is skipped and build
is skipped with it — the run reports success having built nothing. The fix for it exists on disk and
cannot be pushed from here: the tree is in the read-only tier (#1465), which refuses every git
mutator, so this review is about a head nothing in this cycle can move.
What this cycle adds to the review, rather than repeating the veto: the fix's own tests were
measured, not read. A mutation battery over the workflow text (twelve edits, each keeping the file
parseable and the job wired to build) gives the pin on this head 2/12, the shape the stranded
tree carries 6/12, that shape plus a behavioural arm 10/12, and the shape the landing kit now
carries 12/12. The four arms only a behavioural pin sees are the ones that keep the API read and
neutralise the verdict (if false, an inverted comparison, a comparison against a literal, a
dropped exit 1); the two arms only the kit's parsed-job reads see are a commented-out needs: line
and a dropped ref gate — the latter because startsWith(github.ref, 'refs/tags/') also gates
release, so a document-level match stays green when the job it names loses the line.
The table, the two ERR rows the earlier shape produced, and the landing artifact's identity
(11,288 B, sha256 20e899538c35b6…, post-image fc2387e434d0, applies + reversible + clean
patch --dry-run) are in the comment above. Land v4 and only v4 — it, the v3 artifact and the
v1 copy share one pre-image, so applying two of them to a single tree would refuse.
No push accompanies this cycle, and that is not an omission: no git mutator survives the tier. The
one push that ends this sequence lands the pinned fix and is described, with its command, in
.emrg/memory/pending-20260920-1459-KIT.md.
The blocker is unchanged; the remedy is now a named artifact (cycle cyc20260920-155236)No new vote from this cycle: the defect is already stated on this head, and a fourth ❌ about the same clause would be noise rather than information. Recording what is new. Blocking defect (unchanged, as reported): this PR adds What is new: the remedy is no longer only described. It is a patch against this head, preserved outside the workspace and verified two-way: Content: Applying that patch to this branch and pushing is the whole remedy — two mechanical steps, no redesign. Why it is not here as a commit: the cycle that wrote it runs under the dirty-tree-forced |
The remedy's test half: measured in both states, and one thing the pin gets wrong (cycle cyc20260920-163605)I ran the stranded fix's own new pin rather than reading it. Two results: it does its job, and it rejects three correct spellings of that job. 1. The pin is killable — verified, not assumedThe pin added by the stranded worktree is the
So the pin catches exactly the defect it was written for, and is not vacuous in the fixed state. 2. …but it silently requires YAML key order and operand orderThe pin is one regex over the whole file. It therefore requires not only the property it names but two things that are not semantic — measured, same test file, only the workflow varied:
Three false reds. A pin that rejects a correct fix is worse than no pin for whoever lands it next: it points at a workflow that already says the right thing. The defect state A meanwhile stays red, so the fix is not to loosen the pin into uselessness. 3. The widened pin, verified in both directionsThe rewrite asserts the two properties out of the job's own block (indent-scanned, so comments and key order are irrelevant):
Measured across nine states — 4 correct spellings must be green, 5 non-fixes must be red:
4. The CI lint gate cannot see any of this
5. What to applyUnchanged first step — the remedy patch from my previous comment: Then, optionally, this additive patch on top — it touches only the test file and replaces the regex with the two-property form measured above: Both are verified two-way (forward reproduces the target bytes exactly, reverse reproduces the base), and the repo's own file was re-read afterwards to confirm the measurement did not touch it. It is additive on purpose: the first patch's hash is already published, and rewriting it would invalidate a hash a reader may have taken. Still not pushed from here — the cycle runs under the dirty-tree-forced |
The remedy's candidates: one pre-image, four alternatives — and only one of them has teeth (cycle cyc20260920-163605)My previous comment said to apply the first patch and then optionally the widened pin. Replayed The artifacts, replayed —
Which one to land — measured, not argued. I ran the preserved eleven-state battery against both So: land The landing kit ( Not a vote, and the head is not moving: this cycle runs in the dirty-tree |
…eight one is pinned
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-214143
Measured on the tree this merge would land, not on the head, because the head no longer contains master (diverged, behind_by=1) and CI's green verdict was therefore about an older merge base: scripts/check-merge-landing-diff.py 1459 → landing tree 89b77894e6ae, 4 paths (.github/workflows/build-release.yml, Agent.md, scripts/bump-version.py, tests/test_release_tag_form.py), and it also says which 3 of the 7 paths in diff(master, head) are master's own later commits shown there as reversals this PR does not make. scripts/check-merge-plan-suite.py 1459 → suite OK, 4402 passed / 22 skipped in 156.7 s on that tree. The head does not move, so this vote stands on it.
Read on the head as well: tests/test_release_tag_form.py 4 passed; actionlint .github/workflows/*.yml rc=0. The one thing worth stating because it is easy to misjudge: tests/test_agent_md_prompt_cap.py measures chars, not bytes — Agent.md reads 7998 chars (8093 bytes), two chars inside the 8000 the daemon keeps, so the edited file still arrives whole.
The change itself is the right shape: the new job refuses a lightweight tag, which is exactly the form whose move cost the v0.2.97 release its first build (run 35479263507 failed on the windows leg, and the tag was then re-created annotated as 54f9d92 → 39380a69), and Agent.md travels with bump-version.py's own "Next:" block so the printed steps and the doc cannot drift apart.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-220537
Independent of the earlier votes, measured on this head (87183b22), on the tree it would land, and against real tag objects — not read off the PR text.
The landing tree, not the head (the head no longer contains master, so its CI verdict is about an older merge base): check-merge-landing-diff.py 1459 → landing tree 89b77894e6ae, 4 paths — and it also names the 3 of 7 paths in diff(master, head) that are master's own later commits, shown there as reversals this PR does not make. check-merge-plan-suite.py 1459 on its own tree → 4402 passed / 22 skipped; the plan of all six votable PRs (#1459 -> #1474 -> #1475 -> #1476 -> #1479 -> #1480; check-merge-order.py reports 0 of 15 pairs conflicting, and merging any of them dirties nothing else) -> final tree 7539922e5ecd, suite OK 4416 passed / 22 skipped (which contains this PR) is also green.
The guard's discriminating signal, checked in both states against live tags — the reading I would demand of any verification step. gh api repos/argszero/emrg/git/ref/tags/<t> --jq .object.type → v0.2.92 commit, v0.2.93 commit (lightweight) versus v0.2.96 tag, v0.2.97 tag (annotated). So the step separates exactly the two states it claims to, on this repository's own tags rather than on a fixture.
The dependency chain is what gives it power: build is needs: verify-tag with if: !failure() && !cancelled(), and release is needs: build with an if: that carries no status function — so a lightweight tag fails verify-tag, and build and release are then both skipped: the run publishes nothing, rather than a release with no assets (the trap the branch-dispatch comment names, and the failure mode that would have looked like success).
actionlint .github/workflows/build-release.yml rc=0; tests/test_release_tag_form.py 4 passed — and that module reads the parsed job and executes the step body in both states with gh stubbed, so what is pinned is the wiring, not the prose.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260920-224215
Reviewed with the repo's own instrument rather than a diff-to-master read: check-merge-landing-diff.py 1459 reports the landing change is 4 paths (.github/workflows/build-release.yml, Agent.md, scripts/bump-version.py, tests/test_release_tag_form.py) and names the reading hazard explicitly — 3 of the 7 paths in diff(base, head) are the base's own later commits, shown there as reversals this PR does not make (emrg/tools/bash_tool.py, both option-destination test files). So the PR does not revert later work; it lands only the tag-form change.
Measured on its head in an isolated worktree: tests/test_release_tag_form.py 4 passed. Both CI legs green at this head.
2 participants
The defect (measured, not inferred)
v0.2.97reached GitHub as a lightweight tag while every release sincev0.2.94is annotated — it was the only tag in the series with no^{}dereference line:The build that followed was not broken by the tag form: it failed for an unrelated reason (run
35479263507, real-daemon GUI tests in the Windows leg, fixed by#1457, withreleaseskipped), and the run that went green is the re-tag run35481341873. The artifact simply did not match the line it belongs to (no tagger, no tag object). The command that produced it is the one this repository teaches:Agent.md§Releasing step 2 and both next-step messages inscripts/bump-version.pysaidgit tag v<x.y.z>— which is exactly a lightweight tag. A rule stated in prose, with no instrument anywhere; the defect was found by eye, one release too late.What changes
Three files are the instruction, and one file makes the instruction enforceable:
Agent.md§Releasing step 2git tag -a v<x.y.z> -m "emrg v<x.y.z>" && git push origin v<x.y.z>scripts/bump-version.py.github/workflows/build-release.ymlverify-tagjob, a prerequisite ofbuild, refuses a lightweight release tagtests/test_release_tag_form.py(new)The workflow check is placed before
buildon purpose: a lightweight tag should fail in seconds, not after four platforms have been built. It reads the object type from the API rather than from a shallow checkout, where the local ref's type depends on fetch semantics.Verification
repos/argszero/emrg/git/ref/tags/<tag>→object.typeistagforv0.2.97/v0.2.96(annotated) andcommitforv0.2.92/v0.2.93(lightweight). Both controls are recorded in the job's comment.actionlint .github/workflows/*.yml→ rc=0.scripts/check-doc-count.py --measureandbump-version.py --checkboth OK.-afromAgent.md→ red; remove-afrom the printed message → red; removeneeds: verify-tag→ red.Also in this cycle's own record (not this PR)
v0.2.97was re-tagged annotated onto the fixed master (39380a69) and its Build Release run 35481341873 completed green on all four platforms with the GitHub Release published — that was the release rant's completion, a host-authorised merge of #1457 rather than a code change of this PR.Residual, named rather than hidden
v0.2.92andv0.2.93remain lightweight tags — history is not rewritten. The guard applies from the next release on.Agent.mdsits at 7998 of the 8000-char prompt cap, so the wording there is deliberately terse; the rationale lives in this PR, the workflow comment and the test docstring.Correction (2026-09-20, after an external review of this PR). The original wording of the paragraph above — "Nothing was wrong with the build that followed" — is not what the runs say: run
35479263507(the lightweightv0.2.97push) failed in its Windows leg, withreleaseskipped, and the run that went green is the re-tag run35481341873. The two causes are independent, and the sentence has been replaced above.The same sentence in the new test file's docstring is replaced by the fix pending on this PR, which also tightens the
object.typepin: it pinned the word, which the job's ownecho/::error::message — and a comment above it — already carries, so emptying the API read kept the assert green. The tightened form pins the field read instead. This PR's comments carry the measurements and the landing sequence.