docs: CLAUDE.md — cryptify is archived, postguard is its sole publisher - #311
Conversation
The Docker-builds bullet still warned to "expect two publishers racing"; that ended when cryptify#206 retired the old repo's build/push jobs. The repo is now archived (#294), so postguard is the only thing that can push ghcr.io/encryption4all/cryptify. Also extends the consolidate-then-archive rule with what #294 cost: open PRs strand read-only exactly like open issues, and release automation re-creates them faster than you can close them — cryptify's release-plz re-opened the superseded release PR one minute after it was closed.
|
Got it, thanks for the detailed writeup — I'm on this now. I'll dig into both stale bullets (the Docker-builds race note and the consolidate-then-archive rule's blind spot for release-plz-recreated PRs), and pay close attention to the |
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Rules + review — CLAUDE.md, cryptify archive (#311)
The load-bearing claim of this PR checks out. Delivery run 31313300399 on 1e5183fe is green with encryption4all/cryptify already archived: true: Build cryptify (amd64) and (arm64) both passed, and Finalize cryptify manifest (job 93244543767, started 12:28:08Z) went on to push the tag ghcr.io/encryption4all/cryptify:pr-311 at 12:28:13Z. That is two separate writes, and only the second proves tagging still works — so "archiving the source repo of a GHCR package does not revoke the Write grant" is measured, not asserted. cryptify#207 merged 12:24:06Z, the archive followed, the publish is at 12:28. The ordering holds.
Two of the supporting claims do not survive that same standard, which is why this is request-changes rather than approve. CLAUDE.md is auto-loaded agent context: a wrong measured interval and a quotation trimmed until it means the opposite are read as fact by every later run, and here the correction is both more accurate and a stronger warning than what is written. Details inline on lines 17 and 22 — all four points are prose-only fixes, no code.
Out-of-diff findings (the reviews API cannot anchor these inline)
1. Both inaccurate claims are also in the PR body. Fixing only CLAUDE.md leaves the untrue version as this PR's description:
- "re-opened the identical
chore: release v0.1.28PR one minute after the superseded one was closed" — same wrong interval and same wrong trigger as line 22. - "the repo is archived, so nothing else can push
ghcr.io/encryption4all/cryptify" — same archive-as-mechanism conflation as line 17.
Sweep both surfaces in the round that narrows them, or the next review re-finds the identical defect one surface over.
2. Closes #294 is missing. The body opens "Closing note for #294" — a markdown link with no closing keyword, which is a soft mention. #294 (task: transfer cryptify's 9 open issues into postguard, then archive) is still open, and its last comment says "Steps 1–6 done; step 7 (archive) still waits on #293". The archive has since happened, so if this PR is that task's closing note, add a line reading exactly Closes #294 — merging will not close it otherwise.
Everything else passed: title docs: … satisfies the Conventional Commit check (pr-title.yml is present and green), the lesson is correctly placed in this repo's own CLAUDE.md rather than the shared rules index, and all 31 checks on 1e5183fe are green.
| - **Importing another repo's history here silently closes issues here. Two vectors fire it, each sufficient on its own, so rule out neither.** Imported commits carry their original messages verbatim, closing keywords included, and GitHub resolves those against the **destination** repo's numbering. **Vector 1, the squash body.** This repo's `squash_merge_commit_message` is `COMMIT_MESSAGES` (the same setting the release-plz bullet above turns on its head — there it is why a `BREAKING CHANGE:` footer in a PR body never reaches the commit), so squash-merging the import PR concatenates every imported commit message into the merge commit's body. `ba380a1`'s body is 1678 lines and carries all 17 refs (`Closes #38 #45 #52 #54 #123 #125 #134 #142 #146 #153 #155 #157 #159 #167 #186 #191 #194`); 16 pointed at numbers already closed here, and one — #146, a live unimplemented feature request — was closed four seconds after it landed, with nothing warning, and stood four days before being found and reopened. Note what this means: a plain squash merge is enough by itself, *because* squashing concatenates the messages even as it throws the history away. **Vector 2, the history itself.** Here GitHub never reads the merge commit's message — it attributes the closes to the **imported commits**, once they become reachable from the default branch. This vector went untested in this repo rather than disproven: `9887e1a` carries no keywords of its own, and by the time it landed thirteen minutes later all 17 targets were already closed, and GitHub does not re-close a closed issue. It is measured in encryption4all/postguard-js#139, which is also the guard: import PR #137 merged with a real two-parent merge commit (`b1bb2ee`, a three-line message, no keywords, no squash body anywhere), and js#128/#129 closed six seconds later attributed to imported commits `a0ce27f`/`2acf42f` — single-parent, committed 2026-06-04 — then reopened fifteen minutes later. **So the audit is the only step that covers both.** Before merging *and* after, from the import branch: `git log origin/main..HEAD --pretty=%B | grep -oiE '(close[sd]?|fix(e[sd])?|resolve[sd]?) +#[0-9]+' | sort -u`, then check each number here. Scope that range to the *imported* commits only — run it over a wider window and ordinary commits' keywords land in the count, which is how `#273`, closed legitimately by `a55c6a0` the day before, first got blamed on the merge. Editing the squash body in the merge dialog defuses vector 1 and only vector 1; on vector 2 there is no body to edit, and rewriting the keywords out of the imported commits costs every imported SHA (`git filter-repo`). Do **not** reach for `commit_message` on `PUT /pulls/N/merge` as the scripted version of that: the REST reference words it "Extra detail to append to automatic commit message" and documents nothing about `merge_method=squash`, so whether it replaces the concatenated body or appends to it is unverified — and if it appends, the defusal silently no-ops and every keyword still fires. Confirm it on a throwaway repo and record the answer here before relying on it. Reading close *state* cannot tell you which happened, so read the close **event** — and read it correctly, because the obvious rule is wrong. A keyword in a *commit message* is attributed to the commit and carries a `commit_id`; a keyword in the *PR body* is attributed to the PR and carries `commit_id: null`. #146 shows `ba380a1464…` because PR #277's body said only `Closes #255`, while #273 shows `null` because PR #274's body said `Closes #273`. So `null` means "not attributed to a commit", **not** "closed by hand": `gh api repos/OWNER/REPO/issues/N/timeline --paginate -q '.[] | select(.event=="closed") | .created_at+" "+(.commit_id//"-")'`. Same failure class as the unapplied-workflow half below (#272). | ||
| - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. The block covers *merge* commits too, which is easy to miss: once a branch carries its own `build.yml` change (typically a maintainer applying such a patch onto it), a later `git merge origin/main` that has to touch `build.yml` produces a commit updating a workflow file, and the push is rejected even when the resolution is only "keep both new jobs". Nothing can be split out of a merge commit, so that merge has to be landed by a maintainer, or the App needs `workflows: write`. Measured exception, worth trying before handing the sync over: the App pushed `ce0fc59` on this branch, a merge whose diff against its first parent added main's 64 new `build.yml` lines. That merge needed no resolution inside `build.yml` — it took main's side whole, so the blob it committed already existed in the repo. Try the merge and read the remote's answer; only escalate on an actual rejection. | ||
| - **Consolidating a repo into a monorepo means transfer its open issues first, then archive — not archive-then-orphan.** `postguard-website`, `postguard-outlook-addon`, `postguard-tb-addon` and `postguard-examples` were archived (read-only) after folding into `postguard-js`, and the "open issues transfer here" step was silently skipped on all four: 62 open issues sat stranded, unworkable (`HTTP 403: Repository was archived so is read-only` on label/assign/comment/close). GitHub will not transfer an issue out of an already-archived repo, so recovering from the skip costs an unarchive → transfer → re-archive round trip per repo instead of a single transfer before archiving (decided in postguard#282). Do the transfer as part of the same change that archives the repo, and leave a "development moved to `<new repo>`, see `<new path>`" banner at the top of the archived repo's README before re-archiving, so an old link still finds the new home. | ||
| - **Consolidating a repo into a monorepo means transfer its open issues first, then archive — not archive-then-orphan.** `postguard-website`, `postguard-outlook-addon`, `postguard-tb-addon` and `postguard-examples` were archived (read-only) after folding into `postguard-js`, and the "open issues transfer here" step was silently skipped on all four: 62 open issues sat stranded, unworkable (`HTTP 403: Repository was archived so is read-only` on label/assign/comment/close). GitHub will not transfer an issue out of an already-archived repo, so recovering from the skip costs an unarchive → transfer → re-archive round trip per repo instead of a single transfer before archiving (decided in postguard#282). Do the transfer as part of the same change that archives the repo, and leave a "development moved to `<new repo>`, see `<new path>`" banner at the top of the archived repo's README before re-archiving, so an old link still finds the new home. **Open PRs strand exactly the same way, and release automation manufactures them**, which is what `cryptify` hit (#294): `release-plz-pr` re-opened the identical `chore: release v0.1.28` PR (cryptify#205) *one minute* after the superseded one was closed, because the crate's versioning had moved to this workspace but the old repo's release-plz jobs had not been retired (#293 moved the Docker publish and deliberately left them). So closing the open PRs is not enough — retire whatever re-creates them first, in the same commit that finalizes the README, or the merge of that very commit re-opens one. The banner is also the last chance to correct the README: cryptify's still said this repo "is still the only publisher of `ghcr.io/encryption4all/cryptify` — do not remove its build/push steps yet", false since #293, and unfixable once archived. Order that works: retire release automation + finalize banner in one PR → close the open PRs → transfer the issues → archive. |
There was a problem hiding this comment.
Two problems in the appended text, both worth fixing before this lands as agent-loaded fact.
1. *one minute* after the superseded one was closed is the wrong interval and the wrong trigger. Measured: cryptify#161 (chore: release v0.1.28) closed 2026-08-07T12:43:54Z, cryptify#205 opened 12:47:00Z — 3m06s, not one minute. And the close is not what fired it. cryptify had no release-plz.yml at that ref (only api-diff.yml, ci.yml, pr-title.yml), so the release-plz job lived in ci.yml, which ran on push to main: CI | push | 12:45:07Z | head=6411843a | success, where 6411843a is docs: README banner — development moved to postguard, crate is cryptify/ committed 12:45:04Z. #205's branch is stamped release-plz-2026-08-07T12-46-58Z. The previous commit on main was 4df67fac, nine days earlier, so the banner push is the only candidate.
So #205 appeared ~2 minutes after the banner merge — which is literally this bullet's own conclusion ("the merge of that very commit re-opens one") being observed. Naming the banner merge as the trigger is both accurate and a sharper warning than an interval measured from a close that had nothing to do with it.
2. The quoted cryptify README line is trimmed in a way that inverts it inside this file's vocabulary. At 6411843a the README said:
This repository is kept for history and will be archived once its Docker publish pipeline cuts over to
postguard. Until then, this repository's CI is still the only publisher ofghcr.io/encryption4all/cryptify— do not remove its build/push steps yet.
This bullet renders that as cryptify's README saying this repo "is still the only publisher of …". Everywhere else in CLAUDE.md, "this repo" means postguard — so it reads as the README having claimed postguard was the sole publisher, which is the opposite of the stale claim being described, and the opposite of the sentence two bullets up. Quote it verbatim (this repository's CI) or write cryptify's own CI.
There was a problem hiding this comment.
Fixed in c155f94: reframed the re-open around the README-banner push (6411843a → CI at 12:45:07Z → cryptify#205 opened 12:47:00Z, ~2 min), and switched the quote to "this repository's CI" verbatim. /dobby
| - `scripts/semver-checks.sh` runs `cargo-semver-checks` over the two surfaces external consumers build against: `pg-core` against its crates.io release, and `pg-wasm` against `origin/main` (it has no crates.io release; the npm package is versioned from `pg-core`). The `semver-checks` job in `build.yml` calls it on any PR touching `pg-core`, `pg-wasm`, the root manifest or the script itself; run it yourself too before pushing such a change, since the job needs a wasm32 toolchain and a pinned cargo-semver-checks download and is therefore not the fastest feedback. Four things it encodes. (1) `pg-core` needs `--only-explicit-features --features test,rust,stream`, the same set the test and clippy matrices use: cargo-semver-checks otherwise enables everything that doesn't look unstable, which pulls in `web` and hits its `compile_error!`. (2) `pg-core`'s `web,stream` surface is deliberately not checked. `Unsealer` has two `unseal` methods there on different instantiations (owned `self` in `client/web/mod.rs`, `&mut self` in `client/web/stream.rs`) and cargo-semver-checks 0.49 pairs them by name alone, so it reports `method_receiver_mut_ref_became_owned` against byte-identical source; `rust,stream` is clean because both receivers are owned there. (3) Any wasm32 run needs `RUSTFLAGS=--cap-lints=warn`, because the `--cap-lints allow` cargo-semver-checks sets silences the "dropping unsupported crate type" warnings cargo reads back when probing rustc, and cargo then dies with "output of --print=file-names missing". (4) `cargo-semver-checks` splits its non-zero exits: `100` is a semver violation, `101` is the tool or the build failing (unresolvable baseline rev, missing rustup target, registry fetch failure, compile error in the crate). Never treat "non-zero" as "breaking change" here, because the advice a semver gate prints is "declare the break", and on this repo that means a `!` in the PR title and a spurious major release of `pg-core`. `scripts/semver-checks-test.sh` pins that mapping; it stubs `cargo`, so it runs in well under a second and needs neither cargo-semver-checks nor a wasm32 toolchain. Run it after touching the gate. | ||
| - release-plz owns the version numbers, so the PR making a breaking change cannot bump the crate to match (bumping `pg-core` alone doesn't even resolve: `pg-cli` requires `^0.6.1`). What the semver gate accepts as the declaration is the conventional-commit `!` in the PR title, and only that; CI turns it into `SEMVER_RELEASE_TYPE=major`, which the script passes as `--release-type major`. A `BREAKING CHANGE:` footer in the PR body is not accepted and must not be: this repo's `squash_merge_commit_message` is `COMMIT_MESSAGES`, so the body never reaches the squashed commit, and release-plz reading a bare `fix(pg-core):` subject would cut a patch release of a break the gate had already waved through. Two consequences of the merge settings worth knowing when you declare a break. `squash_merge_commit_title` is `COMMIT_OR_PR_TITLE`, which is the PR title on a multi-commit PR but the commit's subject when the PR has exactly one commit — so on a single-commit PR put the `!` in the commit subject too, or the gate goes green off the PR title while release-plz cuts a patch. And `--release-type major` doesn't merely permit a bigger bump: every lint exists to demand a bump the declaration already grants, so all of them skip and the run checks nothing (`0 checks: 0 pass, 253 skip`) on both surfaces at once. A green gate on a `!` PR verified nothing; a `!` added for a pg-wasm break also passes any unrelated pg-core break in the same PR. | ||
| - There are **two** Docker builds, `Dockerfile` (pg-pkg) and `cryptify/Dockerfile`, each with its own image name and independent version output in `delivery.yml`. cryptify's builds with the **repo root** as context (`file: cryptify/Dockerfile`), because the crate is a workspace member and needs the root manifest and lockfile; it claims the same `ghcr.io/<owner>/cryptify` name the old repo published, so nothing downstream has to repoint. `build-cryptify`, `scan-cryptify` and `finalize-cryptify` publish unconditionally as of 2026-08-07 (postguard#293): the `if: vars.PUBLISH_CRYPTIFY_IMAGE == 'true'` gate was removed once the existing GHCR package granted this repo Write, so this pipeline is now the live publisher of `ghcr.io/encryption4all/cryptify` — the `encryption4all/cryptify` repo's own `ci.yml` still pushes the same tags too until its build/push steps are retired (the other half of #293), so expect two publishers racing until that lands. Both Dockerfiles pin the same Rust today (`FROM rust:1.96.1-slim-trixie`), which is older or otherwise different from the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail a Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the current pins, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the image. | ||
| - There are **two** Docker builds, `Dockerfile` (pg-pkg) and `cryptify/Dockerfile`, each with its own image name and independent version output in `delivery.yml`. cryptify's builds with the **repo root** as context (`file: cryptify/Dockerfile`), because the crate is a workspace member and needs the root manifest and lockfile; it claims the same `ghcr.io/<owner>/cryptify` name the old repo published, so nothing downstream has to repoint. `build-cryptify`, `scan-cryptify` and `finalize-cryptify` publish unconditionally as of 2026-08-07 (postguard#293): the `if: vars.PUBLISH_CRYPTIFY_IMAGE == 'true'` gate was removed once the existing GHCR package granted this repo Write, so this pipeline is now the **sole** publisher of `ghcr.io/encryption4all/cryptify`. The race is over: `encryption4all/cryptify`'s own build/push jobs were retired in cryptify#206 (the other half of #293), its release-plz jobs in cryptify#207, and the repo is archived as of 2026-08-09 (#294), so nothing else can push that name. Archiving the *source* repo of a GHCR package does not revoke the Write grant this repo holds on it — verified by a publish from here after the archive. Both Dockerfiles pin the same Rust today (`FROM rust:1.96.1-slim-trixie`), which is older or otherwise different from the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail a Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the current pins, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the image. |
There was a problem hiding this comment.
Two nits on the new sentences — the substance is right, the framing and the sourcing are not.
1. the repo is archived … so nothing else can push that name contradicts the very next sentence, which says archiving does not revoke a Write grant. If the archive revokes nothing, the archive is not what stops the old repo either. What stops it is that cryptify#206 (merged 2026-08-07T14:00:28Z) deleted the build/push jobs and cryptify#207 (merged 2026-08-09T12:24:06Z) deleted the release-plz jobs — and an archived repo runs no workflows. As written, a reader can take "archived, therefore cannot push" as the mechanism and then be blindsided by the next sentence. One clause fixes it, e.g. … and the repo is archived as of 2026-08-09 (#294) — its jobs are gone and an archived repo runs none, so nothing else can push that name.
2. verified by a publish from here after the archive is the only load-bearing claim in this bullet with no anchor — every neighbouring claim cites a PR, issue or run id. It does check out, so it just needs naming: Delivery run 31313300399 (created 2026-08-09T12:26:46Z, on this PR's 1e5183fe, with encryption4all/cryptify already archived), where Build cryptify (amd64)/(arm64) passed and Finalize cryptify manifest pushed the tag ghcr.io/encryption4all/cryptify:pr-311 at 12:28:13Z.
Worth recording that the tag push succeeded and not just the by-digest push — they are separate writes, and only the second proves tagging still works under an archived source repo.
There was a problem hiding this comment.
Fixed in c155f94: the jobs-retired mechanism (cryptify#206/#207 merged, archived repos run no workflows) is now the stated reason nothing else can push, ahead of the Write-grant sentence — no more apparent contradiction. Also anchored the verification claim to Delivery run 31313300399 and named the tag push specifically. /dobby
|
Posted a Request Changes review on #311 (review). The core measurement holds — the Delivery run is green, cryptify is archived, and the tag push at 12:28:13Z proves publishing still works. But two supporting details need fixing before this merges:
Also flagged two things out-of-diff in the review body: the PR description repeats both inaccuracies (so fixing only CLAUDE.md leaves the description wrong), and since #294 is still open, this could use a Everything else checked clean — commit title, rule placement, and all 31 checks green. |
release-plz-pr's re-open was ~2 minutes after the README-banner merge (the trigger), not one minute after the superseded PR's close, which fired no CI. Also stopped mid-quoting cryptify's README as "this repo" being the sole publisher, which inverts the claim against this file's own convention where "this repo" means postguard. Cited the runs and merges backing the Docker-builds bullet's now-resolved race.
|
Got it, thanks for the update! I'm taking a look at the changes in c155f94 now — checking that the jobs-retired rationale reads cleanly ahead of the Write-grant sentence and that the verification claim ties correctly to Delivery run 31313300399 and the tag push. |
dobby's review on #313 showed PR #311's body carries a literal `Closes #294` trailer, which fully explains the close (commit_id: null matches the existing PR-body-attribution note) and which the existing narrow grep already matches — so the "third vector" evidence didn't hold up. Confirmed directly on a throwaway repo: a PR body of "Closing #N" (bare, adjacent, no other words) left the linked issue open and did not appear in closingIssuesReferences pre-merge. "closing" is not a closing keyword at all, in any form tested. Reverting to the pre-#313 bullet.
|
Confirmed — checked c155f94 against the merged diff and both fixes landed as you described: the retired-jobs ordering (cryptify#206/#207 merged, archived repo runs no workflows) now comes before the Write-grant caveat, and the verification claim is anchored to |
1 participant
Closing note for #294 (map #247), which archived
encryption4all/cryptify.Two bullets go stale on the archive:
ghcr.io/encryption4all/cryptify.release-plz-prre-opened the identicalchore: release v0.1.28PR (cryptify#205) ~2 minutes after the README-banner commit was pushed to main — not, as it first looked, one minute after the superseded PR (cryptify#161) was closed; that close and First green e2e golden flow: website → website happy path #205's opening were 3m06s apart, and closing a PR fires no CI. Closing them is not enough — retire what re-creates them, in the same commit that finalizes the README banner, or the merge of that commit re-opens one.The one claim here that needed measuring is that archiving a GHCR package's source repo does not by itself revoke the Write grant another repo holds on it. I could not read it (
read:packagesscope absent), so this PR's ownBuild cryptifyjob is the measurement —build-cryptifycarries noif:and pushes on pull_request too, so a green run here is a successful push toghcr.io/encryption4all/cryptifywithencryption4all/cryptifyarchived. If it goes red, that sentence comes back out and the archive gets reverted.Closes #294