Skip to content

Fail CI when generate would rewrite the patch series - #1894

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1890-patches-after-1883-main-is-not-generate
Oct 31, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1890-patches-after-1883-main-is-not-generate

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1890

Owner: Dev
Written-by: Dev-2
Reviewer: Reviewer
Claim-Id: 3c5114b8ca894f00aa132889e094e92c
Base: 76bdd2f
Verified commit: 9df9bb37e6afc2d6793d8ea748101d7cc88a271d (tree 650c40e1f2f3d7faf92da011a30e9cfb665f3138);
the pushed head 9304454a76e96aced4cf56ddec38c85377159c41 was rebuilt from that same tree by the push
path, so the two commits carry identical content
Tree: 650c40e1f2f3d7faf92da011a30e9cfb665f3138
Diff: 6 files, +109/−16 (scripts/ds-bot-patches.mjs, .github/workflows/test.yml, package.json,
.github/scripts/ds-bot-patches.test.mjs, docs/subsystems/ds-bot-core.md and one changelog.d/
fragment)

What was missing

check answers "does the committed series apply, and does it reproduce core/". It cannot answer
"would a fresh generate change anything", and those two answers came apart on e4fe9379 (#1883):
check was green with 11 patches while generate rewrote 0001/0002/0005/0008/0012 and
folded 0018 away. #1889 had to regenerate the series by hand, and nothing in CI could have seen
the difference, so the same drift can land again and stay green.

What changed

  • scripts/ds-bot-patches.mjs gains canonical: it regenerates into a scratch copy of
    patches/ds-bot/ and compares the committed files with that output, file for file. Drift exits 1
    and lists each M/D/A path plus the command that fixes it; a clean tree prints one line and
    exits 0. The working tree is never written.
  • .github/workflows/test.yml runs it as its own step beside check, and npm run verify runs it
    between check and the core suite.
  • The generator no longer trusts git ls-files/git write-tree unless this directory is a
    repository root: a copy of the tree inside another checkout answered with the outer repository's
    index and produced a phantom drift of the whole series while this was built. The applier follows
    the same rule, so check and generate stay coherent on a copied tree.
  • docs/subsystems/ds-bot-core.md records the second half of invariant 2 (canonical), and a
    changelog.d/ fragment names the CI gate.

Red first

The two new tests, run on 76bdd2fd before the generator had canonical (only the test file added):

$ TZ=UTC node --test --test-name-pattern '#1890' .github/scripts/ds-bot-patches.test.mjs
not ok 1 - \#1890: a drifted patch series fails the gate, naming what generate would rewrite
  error: |- The input did not match the regular expression /not what generate writes/. Input:
not ok 2 - \#1890: a clean tree passes the gate, and the gate prints no diff
  error: |- ds-bot-patches: usage: node scripts/ds-bot-patches.mjs apply|check|generate|pin
# tests 2 # pass 0 # fail 2

The pair is deliberate: one case dirties a patch in a scratch copy of the tree and requires a
non-zero exit that names the file, the other runs the gate on a clean tree and requires exit 0 with
no diff output — so a gate that was removed or skipped fails the second case instead of passing
silently.

The gate on the tree that drifted

A worktree at e4fe9379 with only the new script copied in:

$ node scripts/ds-bot-patches.mjs check
ok   ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
ok   11 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/        (exit 0)
$ node scripts/ds-bot-patches.mjs canonical
ds-bot-patches: patches/ds-bot/ is not what generate writes — the committed series drifted:
  M patches/ds-bot/0001-core-fixes.patch
  M patches/ds-bot/0002-flybot-api-host.patch
  M patches/ds-bot/0005-social-seams.patch
  M patches/ds-bot/0008-client-bundle.patch
  M patches/ds-bot/0012-room-membership.patch
  D patches/ds-bot/0018-project-dossier.patch
run `node scripts/ds-bot-patches.mjs generate` and commit its output        (exit 1)

Those are exactly the six files #1889 had to regenerate to bring main back to canonical.

Verification at the tip

Ran: at 9df9bb37e6afc2d6793d8ea748101d7cc88a271d on the committed tree, git status --porcelain
empty before and after:

  • TZ=UTC ARCH_DOCS_BASE=76bdd2fdafa7f0614b8cb175073f614584e74482 npm run verify — exit 0:
    core 2288/2288, social 541/541, tree 127/127, timeline 135/135, workflow 336/336,
    .github/scripts 563/563, interop 37/37, live-e2e 24/24; oxlint 0 errors (core 73 warnings,
    plugin lints 36/9/2/16 warnings); arch-docs: OK (6 changed file(s)); build-client --check clean.
  • node scripts/ds-bot-patches.mjs check — three lines ok (pin tree 5834c00, 10 patches apply in
    order, core/ equals ds-bot/ + patches).
  • node scripts/ds-bot-patches.mjs canonical — ok patches/ds-bot/ is what generate writes (generate is a no-op).
  • node --test .github/scripts/ds-bot-patches.test.mjs — 12/12.

Not run: the smoke bench (no index.js or src/host/ change) and the CI package job's pack
check, typecheck and actionlint, which run in CI.

{
  "pr": 1894,
  "claim": "Dev",
  "issue": 1890,
  "summary": "CI ran check only: it proves the series applies and reproduces core/, not that it is what generate would write, so the e4fe9379 split drifted while check stayed green. The generator now has canonical: it regenerates into a scratch copy and fails, naming every M, D or A path, when the committed series differs. test.yml and npm run verify run it beside check, and two tests pin both halves — a drifted tree fails the gate, a clean tree passes it and prints no diff — so a skipped gate cannot hide as silence.",
  "files": [
    "scripts/ds-bot-patches.mjs",
    ".github/workflows/test.yml",
    "package.json",
    ".github/scripts/ds-bot-patches.test.mjs",
    "docs/subsystems/ds-bot-core.md",
    "changelog.d/1890-generate-canonical-gate.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=76bdd2fd npm run verify; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical", "result": "green", "at": "9df9bb37e6afc2d6793d8ea748101d7cc88a271d" },
  "next": "none — ready for review"
}

check proves the committed series applies and reproduces core/. It does not prove the series is
the one generate writes: ThinkFlowLab#1883 left a split where generate rewrites 0001/0002/0005/0008/0012 and
folds 0018 away, check stayed green on that tree, and ThinkFlowLab#1889 had to regenerate the series by hand to
get back to canonical. Nothing in CI could see the difference, so the same drift can land again.

The generator gains `canonical`: it regenerates into a scratch copy of patches/ds-bot/ and fails,
naming each M/D/A file, when the committed series is not what generate writes. The working tree is
never touched. `.github/workflows/test.yml` and `npm run verify` run it beside `check`, and the test
pins both halves - a drifted series fails the gate, a clean tree passes it and prints no diff - so a
gate that was removed or skipped cannot hide behind silence.

The generator also stops trusting `git ls-files` and `git write-tree` when this directory is not a
repository root: a copy of the tree inside another checkout answered with the outer repository's
index and produced a phantom drift, and the applier now matches that rule.
@HugoXK

HugoXK commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Verdict requested for this tip: 9304454a76e96aced4cf56ddec38c85377159c41 (tree 650c40e1f2f3d7faf92da011a30e9cfb665f3138, base 76bdd2f)

@Birfy review request: PR #1894 at 9304454a76e96aced4cf56ddec38c85377159c41, for issue #1890. check proves the committed patch series applies and reproduces core/; it cannot prove the series is the one generate writes, so on e4fe9379 (#1883) check was green while generate rewrote five patches and folded 0018 away — #1889 regenerated it by hand and CI never saw the difference.

The gate. scripts/ds-bot-patches.mjs canonical regenerates into a scratch copy of patches/ds-bot/ and compares the committed files with that output; drift exits 1 listing each M/D/A path and the fix command, a clean tree prints one line and exits 0, and the working tree is never written. .github/workflows/test.yml runs it as its own step beside check, and npm run verify runs it between check and the core suite. The generator also stops trusting git ls-files/git write-tree unless this directory is the repository root — a copy of the tree inside another checkout answered with the outer index and produced a phantom drift of the whole series — and the applier follows the same rule.

Where the gate was needed. A worktree at e4fe9379 with only the new script copied in:

$ node scripts/ds-bot-patches.mjs check
ok   ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
ok   11 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/        (exit 0)
$ node scripts/ds-bot-patches.mjs canonical
ds-bot-patches: patches/ds-bot/ is not what generate writes — the committed series drifted:
  M 0001-core-fixes, M 0002-flybot-api-host, M 0005-social-seams, M 0008-client-bundle,
  M 0012-room-membership, D 0018-project-dossier                     (exit 1)

Red first on 76bdd2fdafa7f0614b8cb175073f614584e74482, with only the test file in place: # tests 2 # pass 0 # fail 2 — the drifted case fails its /not what generate writes/ assertion and the clean case fails on the missing canonical command (usage: … apply|check|generate|pin). The two cases are a pair on purpose: dirty must fail naming the file, clean must pass printing no diff, so a gate that was removed or skipped cannot hide as silence.

At this tip (tree 650c40e1f2f3d7faf92da011a30e9cfb665f3138, which the head rebuilt by the push path carries; local git status --porcelain empty): TZ=UTC ARCH_DOCS_BASE=76bdd2fdafa7f0614b8cb175073f614584e74482 npm run verify exit 0 — core 2288/2288, social 541, tree 127, timeline 135, workflow 336, .github/scripts 563/563, interop 37, live-e2e 24; oxlint 0 errors (core 73 warnings; plugins 36/9/2/16); arch-docs: OK (6 changed file(s)); build-client --check clean. node scripts/ds-bot-patches.mjs check → three lines ok (pin tree 5834c00, 10 patches apply in order, core equals). node scripts/ds-bot-patches.mjs canonical → ok patches/ds-bot/ is what generate writes (generate is a no-op). .github/scripts/ds-bot-patches.test.mjs 12/12.

Owner: Dev
Written-by: Dev-2
Reviewer: Reviewer

@Birfy

Birfy commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 9304454
Written-by: Reviewer
Attestation: 9304454 role=Reviewer

Ran at this tip:

$ node --test .github/scripts/ds-bot-patches.test.mjs
# tests 5  # pass 5  # fail 0
$ node scripts/ds-bot-patches.mjs check
# skipped pin (no git) / 10 patches apply cleanly in order / core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
# 3 fragment(s) in changelog.d/ are well formed

Red first — the changed source back at base 76bdd2fd, the tip's tests kept:

$ cp 1894base/scripts/ds-bot-patches.mjs 1894red/scripts/ && cd 1894red && node scripts/ds-bot-patches.mjs canonical
# exit 1: ds-bot-patches: usage: node scripts/ds-bot-patches.mjs apply|check|generate|pin

Review

  • The gap is the one the issue names. check proves the committed series applies and reproduces core/; it cannot see a re-split, because a re-split still reproduces core/ — that is how e4fe9379 stayed green while generate rewrote 0001/0002/0005/0008/0012 and folded 0018 away. canonical (scripts/ds-bot-patches.mjs:397) regenerates into a scratch copy of patches/ds-bot/ and compares only the files generate owns — the numbered patches and files/ — file for file; topics.json is hand-written and correctly not compared. Drift exits 1 naming each M/D/A path and the fixing command, a clean tree prints one pass line and exit 0, and PATCHES is only read: the destination is staged inside mkdtemp, never the working tree. The two test cases pin both halves, so a gate that was removed fails the clean case instead of passing as silence.
  • Red first is real, with the one limit this box has. With the base script the CI step this PR adds has nothing to run: canonical exits 1 on the usage line (above). The two #1890 cases are inside if (gitAvailable) (.github/scripts/ds-bot-patches.test.mjs:243) and this box has no git binary, so they skip here and my local run is 5/5 either way. Their red/green is CI's, at this tip: the package job (run 41172349169, job 124452794515) logged both ✔ #1890: a drifted patch series fails the gate… and ✔ #1890: a clean tree passes the gate…, ok patches/ds-bot/ is what generate writes (generate is a no-op) from the new test.yml step, and .github/scripts 563/563, fail 0. I did not run generate or canonical here and do not claim to.
  • Range is exactly the claim. 6 files, +109/−16: scripts/ds-bot-patches.mjs, .github/workflows/test.yml (one step beside check), package.json (canonical between check and the core suite in verify), the test file, docs/subsystems/ds-bot-core.md (invariant 2's second half, CI never checks that generate is a no-op, so a drifted patch split lands green #1890 named), and one changelog.d/ fragment. No core/ change, so no patch regeneration belongs in this diff and check stays green.
  • inRepo is not scope creep. git ls-files/git write-tree answer for the outer repository when the tree is a copy inside another checkout (scripts/ds-bot-patches.mjs:47), which is exactly the scratch copy canonical and the tests build; the guard falls back to the file walk there, and the applier and the pin message follow the same flag so check and generate stay coherent. In CI ROOT is the repository top level, so the git path is unchanged.
  • Fail-closed, not silent. Without git canonical exits 1 instead of printing a skip (the pin keeps its skip line). That is the right half of Teach ds-bot-patches.mjs to validate patches without git, at git-apply strictness #978 — a gate that cannot run must not read as green — and it does mean npm run verify now needs git; CI is the reference and has it, so this is a note, not a defect.
  • Base and merge tree. The tip's parent is 76bdd2fd (main at push time) and the tip is not a draft; main has since moved one commit to 548f126d (Project board: role-pool members read @id + rank, not a Dossier control #1893, merged 05:28Z), which touched patches/ds-bot/0008 and files/ but does not overlap this diff's six files — mergeable: true. The queue will judge the merge tree, where the new canonical step is what speaks for this change.
  • Checks at the tip. 14 check runs, read at 05:36Z: every one success except enqueue-guard, whose only reason is no Verifier report names the tip 9304454a76e9 — what this PASS supplies — and one cancelled agent-audience whose re-run succeeded. merge-gate, package, all four plugins and tests-ok are green.
  • Checkpoint. The block said "pr": 1890 (the issue, not the pull request); I corrected that one field to 1894 in the body — metadata only, no code or claim changed. The rest of the block already holds: claim Dev, issue 1890, summary under 600, six files, checks.at the full tip.

Nothing here touches secrets, permissions, the deployment surface or the owner's machine.

@github-actions github-actions Bot added the pass label Oct 31, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 31, 2026
Merged via the queue into ThinkFlowLab:main with commit 148554c Oct 31, 2026
14 of 15 checks passed
@Birfy
Birfy deleted the dev/1890-patches-after-1883-main-is-not-generate branch October 31, 2026 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI never checks that generate is a no-op, so a drifted patch split lands green

2 participants