Skip to content

Adopt this repository into the paranext organization - #1

Merged
tjcouch-sil merged 31 commits into
mainfrom
adopt-into-paranext-org
Sep 11, 2026
Merged

tjcouch-sil merged 31 commits into
mainfrom
adopt-into-paranext-org

Conversation

@tjcouch-sil

@tjcouch-sil tjcouch-sil commented Aug 31, 2026

Copy link
Copy Markdown
Member

Seeds this repo as the paranext organization's source of truth for the editor packages, and documents what that means for everyone who works here or consumes it.

Part of PT-4500.

What this does

Ownership and provenance. Adds a Where this repository lives section: this is a standalone copy of eten-tech-foundation/scripture-editors carrying full history, MIT license, and attribution — deliberately not a GitHub fork, because while a repo is a fork GitHub always defaults new PRs to the parent and offers no setting to change that. Adds a SIL Global / United Bible Societies copyright line alongside ETEN Tech Foundation's; the license stays MIT.

Relationship to paranext-core. Documents that paranext-core does not install these from npm — its preinstall builds them from source and stages them, and its package.json files reference the staged folders with file: specifiers. The consequence worth knowing: this repo's package.json files are authoritative for its own dependencies, so adding or bumping one here flows into paranext-core with nothing to restate on the consuming side.

The platform-yalc branch. Explains why it still exists — the name is historical (paranext-core no longer uses yalc), but its coordination role is real: a breaking change lands on main first and platform-yalc moves forward only once the consuming side is ready.

The eten-tech-foundation remote. How to add it (named eten-tech-foundation, not upstream — we don't track it), pull from it, and contribute back. We may never use it, but the option is deliberately kept open.

Transferring work. docs/transferring-work-from-eten-tech-foundation.md — this repo was seeded with a curated ref set (main, platform-yalc, release-prep, and all 80 release tags), so feature branches were left behind. The guide covers moving branches, tags, and open PRs across, plus re-pointing an existing clone non-destructively.

Release process

Replaces the npm-publishing machinery with a process modeled on paranext-core's. Consumers pin a revision of this repository, not a published version, so a release is a repository tag and one tag pins both packages. Tags are v<platform-editor version>.

  • scripts/bump-versions.ts + bump-versions workflow and composite action — sets both package versions on a bump-versions-<version> branch. Tested locally; it moves both manifests and correctly leaves the workspace: specifier alone.
  • publish workflow — verifies the requested version matches packages/platform/package.json before tagging (a tag disagreeing with the manifest would pin consumers to a build claiming a different version), creates the release, and can open the follow-up bump branch.
  • Removes the scribe publish workflow, the tag-triggered npm publish steps, and the auto-bump-after-publish job. test-publish.yml becomes test.yml, a pure CI workflow.

Nx Cloud references are dropped — no account for this org, and the caching doesn't earn standing one up. It was already commented out in the test workflow ("credits exhausted").

Notes for review

  • Nothing here changes the editor's architecture, public API, or package names. Package names stay @eten-tech-foundation/*; renaming was explicitly out of scope.
  • packages/scribe removal is a separate PR so the diff against upstream stays easy to reason about.
  • The release process is the piece I'd most like a second opinion on — in particular the choice to tag v<platform-editor version> at the repo level rather than keeping per-package platform_v* / utilities_v* tags. Since consumption pins one repo revision, one tag pinning both packages seemed the honest unit, but it's a judgment call.
  • The workflows are not exercised by this PR. bump-versions and publish are workflow_dispatch-only and want a real dispatch before anyone relies on them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VCWPCf35QD5sTkFghsTfUX


This change is Reviewable

Merge order

This is step 1 of eleven PRs that cannot all be green at once.

Immediately after this merges, move platform-yalc onto the new main
(npm run move-platform-yalc) — it currently carries no committed dist/, and until it does,
paranext-core's staging falls back to a pnpm/nx build that no consumer runner can perform.

In that window verify-platform-yalc.yml fails by design: it reads dev-packages.json from
paranext-core's main, which still predates staged consumption, so there is no lockfile contract to
compare against. npm run move-platform-yalc -- --skip-verify is the intended way through, and this
is the one window it is for. Then merge
paranext-core#2745, then the nine consumer
PRs. Full sequence is in core#2745.

This repo is now maintained at paranext/scripture-editors and is the source of truth
for the editor packages. It carries the full history of
eten-tech-foundation/scripture-editors, its MIT license, and its attribution, but it is
deliberately not a GitHub fork: while a repository is a fork, GitHub always defaults new
pull requests to the parent and offers no setting to change that.

Documents what that means in practice:

- Where this repository lives, and its relationship to paranext-core — which builds these
  packages from source and stages them rather than installing them from npm, so this
  repo's package.json files are authoritative for its own dependencies.
- Why the platform-yalc branch still exists. The name is historical, but its coordination
  role — letting a breaking change land on main before build servers pick it up — is not.
- How to set up the eten-tech-foundation remote, pull from it, and contribute back, so
  that option survives even though we do not track it.
- How to move branches, tags, and open PRs across from the other repository, since this
  one was seeded with a curated ref set rather than a full mirror.

Replaces the npm-publishing machinery with a release process modeled on paranext-core's.
Consumers pin a revision of this repository, not a published version, so a release is a
repository tag and one tag pins both packages. Adds a bump-versions script, workflow, and
composite action, plus a publish workflow that verifies the requested version matches the
manifest before tagging. Removes the scribe publish workflow and the tag-triggered npm
publish and auto-bump jobs.

Drops the Nx Cloud references. There is no account for this organization, and the caching
does not earn standing one up.

Adds a SIL Global and United Bible Societies copyright line alongside ETEN Tech
Foundation's. The license stays MIT, which is what keeps contributing back possible.
…pushes

paranext-core's package-lock.json records these packages' dependency closure, so a
dependency added, removed, or re-ranged here breaks every core build the moment
platform-yalc moves — core's `npm ci` refuses to run until its lockfile is refreshed.
That refresh is a one-command PR in core, but nothing reminded the person moving the
branch to open it.

Adds a workflow that runs on every push to platform-yalc: it recomputes what core's
lockfile should record from this repo's manifests (applying the same transforms core's
staging applies — devDependencies dropped, workspace: specifiers rewritten to the sibling
staged folder, the package list read from core's own dev-packages.json so the repos
cannot drift) and passes when core's main matches. When it does not, it scans open core
PRs touching package-lock.json for one that brings it in sync — the refresh being in
flight is fine — and otherwise fails with the exact commands to fix it. Version-only
bumps pass untouched; npm ci does not check the version of a file: dependency, so
releases need no core PR.

The README's platform-yalc section documents the same flow for humans.

Verified against live data: the in-sync path against the core PR branch, and a fabricated
dependency change exercising the mismatch diff, the open-PR scan (152 open PRs, paginated),
and the failure instructions.
@tjcouch-sil
tjcouch-sil force-pushed the adopt-into-paranext-org branch from ec221be to 75adc47 Compare September 1, 2026 22:27
The workflow catches a missing core lockfile PR after the push; this catches it before.
Operates on a temporary branch from origin/platform-yalc (never the often-stale local
branch), restores the original checkout on exit, aborts cleanly on rebase conflicts, and
runs the same verify-consumer-lockfile-sync check the CI workflow runs. --dry-run stops
short of pushing; --skip-verify is the documented emergency hatch.
@tjcouch-sil
tjcouch-sil force-pushed the adopt-into-paranext-org branch 2 times, most recently from 75adc47 to b3b88d4 Compare September 2, 2026 14:47
paranext-core consumes these packages by copying them out of a checkout rather than
installing them from a registry, so until now every core developer needed this repo's
toolchain — pnpm, nx, a full vite build and api-extractor run — during `npm install`, just
to run Platform.Bible. That build is also where a whole class of install failures lives.

Committing dist reduces the consumer's work to a copy. Only people changing the editor
build it.

The obligation this creates is that dist can go stale against src, and a stale dist is
invisible in review: the source diff looks right while consumers get old code. CI now
rebuilds and fails if the committed output differs (scripts/verify-committed-dist.mjs).
Every published artifact is byte-deterministic — verified by rebuilding and comparing
hashes — so the check is exact rather than advisory.

Also stops packing dist/*.tsbuildinfo. That is TypeScript's incremental cache, the one
non-deterministic file in the build, and it was being copied into consumers' staged copies
as though it were a shipped artifact.
@tjcouch-sil

Copy link
Copy Markdown
Member Author

Added: the built dist/ is now committed (closes #4, brought into this round rather than deferred).

packages/platform/dist and packages/utilities/dist are tracked, so paranext-core consumes these packages by copying rather than building — no pnpm, no nx, no build during its npm install. Measured on a checkout that has the committed dist: ~7 seconds instead of several minutes, and the whole Volta/pnpm class of install failures leaves the consumer path entirely.

Guarding the obvious risk (a stale dist is invisible in review):

  • scripts/verify-committed-dist.mjs + a CI step that rebuilds and fails on any drift.
  • Determinism verified before relying on it — rebuilt and compared hashes; every published artifact is byte-identical.
  • The one non-deterministic file was dist/*.tsbuildinfo, which turned out to be TypeScript's incremental cache being packed into consumers as though it were a shipped artifact. Now excluded from both git and files.
  • README documents the rebuild-and-commit obligation.

Two things reviewers should know:

  1. nx caches dist as a target output, so a cache hit restores nx's copy over the committed one and can delete files the cache predates — I hit exactly that (17 spurious deletions). paranext-core's staging builds with --skip-nx-cache. If you see unexplained dist deletions locally, git checkout -- packages/*/dist.
  2. Until this merges and platform-yalc rebases past it, core still builds via its fallback path — verified working, so there is no window where core breaks.

A --stat summary cannot distinguish a genuine source change from a build that is not
reproducible across machines, which is exactly the question a CI failure here raises.
nx declares `dist` a cached target output, and earlier steps in this job populate that
cache, so a cache hit restores nx's copy over the committed one — the check would then be
grading the cache instead of the source. It also explains an earlier spurious failure of
this step: the first run after committing dist rebuilt over it incrementally.
…tput

The first dist commit captured a mid-build state: tsc's per-file declarations under
dist/converters/ plus an index.d.ts that merely re-exported them. api-extractor consumes
those and emits the rolled-up API surface as index.d.ts, then removes them — so which
files exist depended on which nx targets last ran, and CI's freshness check failed
against a local build for exactly that reason.

Commits what a canonical build leaves: the rolled-up index.d.ts and the bundles. The
intermediate per-file declarations are gitignored, since whether they are present is not
meaningful and nothing references them — `types` resolves to dist/index.d.ts.
Same class of intermediate artifact: api-extractor rolls the declarations into
dist/index.d.ts and the map beside them becomes meaningless, but a plain `build` leaves it,
so its presence depends on which targets last ran. CI caught it as an untracked file.
tjcouch-sil and others added 10 commits September 3, 2026 11:15
`homepage`, `repository`, and `bugs` still named eten-tech-foundation, so
everything npm derives from them — the repository link on npmjs.com, `npm
repo`, and the "report an issue" link — sent people to the repository this
one replaces.

The package names and their `@eten-tech-foundation` scope are deliberately
unchanged: publishing continues under that scope, and renaming would break
every consumer. Only the location of the source moves.

`packages/scribe` is left alone; #2 removes that package.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017jvJ2eYTfxJaBkKoQbVeiW
`publish.yml` created the release tag after nothing but a build. The workflow
it replaced gated every publish on `needs: test`, and the committed-dist check
lives in `test.yml`, which only triggers on `main` — so a release cut from any
other branch was verified by nothing, and a tag cannot be un-cut. Run the same
checks `main` gets, all of them before the release step.

The bump step after the release could never have worked: `actions/checkout`
cleans the workspace, which takes `node_modules` with it, and the composite
action then runs `tsx`. By that point the tag exists and `bump-versions` is not
idempotent, so the failure has to be unpicked by hand. Reinstall first.

`verify-consumer-lockfile-sync.mjs` compared `dependencies` and
`peerDependencies` only. `peerDependenciesMeta` is what marks a peer optional,
so deleting an entry makes that peer required and npm pulls it into the
closure: verified against a staged `file:` dependency, making `yjs` required
this way fails every core `npm ci` with "Missing: yjs from lock file" while
both dependency lists read identically. Compare it too, structurally, since its
entries are objects.

That guard also spent one API request per open core PR — 154 today — to decide
which touch the lockfile, which exhausts the anonymous rate limit it runs under
locally. Look at recently-updated PRs first and walk the rest only when the
alternative is failing. A file list at the page limit now counts as a candidate
rather than being read as "no lockfile here", a deleted head fork is skipped
instead of throwing, the raw.githubusercontent reads authenticate like the API
reads, and an empty `CORE_BRANCH` falls back to `main` instead of building a
malformed URL.

`verify-committed-dist.mjs` capped its failure output at 200 lines, which
bounds nothing when `index.js.map` is three megabytes on one line. It also now
records the blind spot it cannot cover on its own — `git status` sees no
ignored path — and points at #5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
`nx.json` still carried ETEN's Nx Cloud access token and workspace id, disabled
only by an underscore on each key. Nx Cloud is off for this repo and the
credentials are not ours; remove them. Its `sharedGlobals` also named
`.github/workflows/ci.yml`, which this repo has never had, so no workflow
change has ever invalidated a cached target — point it at `test.yml`.

paranext-core consumes these packages by staging a built copy out of a
checkout, so `yalc` has no remaining role: drop the dependency and the
`devpub` scripts, and replace both "Develop in App" sections with the flow that
works now (`npm run build:editor` in core).

Badges pointed at the old organization and at `test-publish.yml`, which no
longer exists, and showed an npm version for packages that are not published to
npm — a repository tag is what pins them. Two docs named that workflow too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
`test.yml` ran on `main` only, so a release cut from `release-prep` or a
`hotfix-*` branch had no formatting, lint, typecheck or test run against it
anywhere. paranext-core covers those branches in its own test workflow and
therefore runs no checks at publish time; this repo had the triggers narrower
and the gap sat in the middle. Widen the triggers instead of duplicating the
checks into `publish.yml`.

The committed dist stays checked at publish, because core has no equivalent of
it: a tag is where consumers copy their editor from, a tag cannot be un-cut,
and a stale dist inside one looks like nothing at all. `extract-api` is the
build, so it replaces the separate build step rather than adding to it.

The post-release checkout now uses `clean: false`, matching core, rather than
reinstalling — and only runs when the bump ref actually differs from the
branch already checked out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
…efreshes

Deciding whether one open core PR touches `package-lock.json` costs an API
request, and core carries 150+ open at a time. Run locally by
`move-platform-yalc` there is no token, so the scan exhausted the 60/hour
unauthenticated limit and failed the check exactly when it mattered.

A lockfile refresh for the change being pushed is worked on alongside it, so
PRs untouched for two weeks are not it. `move-platform-yalc --skip-verify`
remains the escape hatch if that ever stops being true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
The failure told you to refresh core's lockfile and open a PR, which is right
when the refresh does not exist yet. It said nothing about the case where it
does and this did not find it — the PR has gone untouched past the search
window, or the two changes are being landed together on purpose — leaving the
escape hatch discoverable only by reading the script.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
…d at

Core carries 155 open PRs, nearly all stale. Measured against it, a 7-day
window selects 51 and a 14-day window 60, against an unauthenticated budget of
60 requests an hour — 7 days fits with room for the listing calls, 14 does not.

The failure line also reported three bare numbers with no way to tell what they
counted, which read as though most PRs had been skipped. Say plainly how many
were examined, how many exist, and how many of those examined touch the
lockfile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL
- `bump-versions-action` and `publish.yml`'s version check spliced `workflow_dispatch` inputs
  straight into bash via `${{ }}`. They now arrive through `env` and quoted expansions, so what
  someone types cannot become shell.
- `COMPARED_SECTIONS` omitted `optionalDependencies`. npm records it for a staged `file:` package
  and pulls its entries into the closure, so adding one would pass this check and then fail every
  paranext-core `npm ci` — the breakage the check exists to catch.
- `CLAUDE.md` and `docs/dependency-updates.md` still routed maintainers through `nx devpub`, which
  this change removes from both published packages. The packing gate is now the `npm pack --dry-run`
  paranext-core's staging step actually calls, so a break there is a break here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
…ble temp path

The verify-script snapshot went to `$TMPDIR/verify-consumer-lockfile-sync-<pid>.mjs` and was then
executed. Pids are small and enumerable, `copyFileSync` follows an existing symlink, and there is a
window between the write and the run — so on a shared machine another local account could have this
script execute their code inside a checkout that is about to be force-pushed. It now writes into a
0700 `mkdtemp` directory and removes the whole directory afterward.

`git rebase --abort` also ran unguarded inside the rebase's own catch. A rebase that fails without
starting — an unreachable `origin/main`, or a leftover rebase directory — makes the abort fail too,
and that throw escaped over the conflict message: the operator saw `Command failed: git rebase
--abort` instead of the real cause, and was told the branch was back at origin's state when the
reset above had already moved it. The abort now has its own try, and the fallback reports the
original rebase error and where the checkout actually is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
…-run

- `verify-consumer-lockfile-sync` read core's files through
  raw.githubusercontent, which serves `max-age=300`. The recovery this check
  prescribes — land the core lockfile refresh, then re-run — could therefore
  still be answered from the pre-merge blob, while the open-PR fallback no
  longer matched because that PR had just closed, leaving `--skip-verify` as
  the only way out of a problem already fixed. The contents API serves the same
  bytes at `max-age=60`; the raw media type carries a lockfile past the 1MB cap
  on the JSON representation.

- It also folded every repo in core's `dev-packages.json` into one list and
  resolved each `packagePath` against this checkout. A second dev repo would
  make it read manifests that are not here. It now selects this repo's entry by
  `folder`, which is stable across the org move in a way `cloneUrl` is not —
  matching on the URL would have replaced the accurate "that core branch
  predates staged-dependency consumption" message with a wrong one for the
  whole transitional window.

- `bump-versions` ran `git checkout -b` and `git commit` unconditionally, so a
  re-run (branch exists, exit 128) or a publish whose post-release version
  equals the released one (nothing staged, exit 1) failed after `publish.yml`
  had already cut the tag — which cannot be un-cut. It now uses `-B` and skips
  the commit when the manifests already say the new version, so the step the
  workflow's own comment called "unpicked by hand" is re-runnable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6

@rolfheij-sil rolfheij-sil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude: Scope note first, for Matt and TJ: this PR got one AI review pass plus a handful of spot checks by Rolf, not the adversarial re-verification we gave paranext-core#2745, studio#179 and the eight extension PRs — so weight it accordingly, and please don't read "no blockers" as "thoroughly proven". The one thing I did verify mechanically: the committed dist (5 + 6 files, ~4 MB) matches files/.gitignore exactly with no .tsbuildinfo, verify-committed-dist.mjs is byte-exact and runs on every PR and before tagging, bump-versions.ts leaves the workspace:~ specifier alone (that path is live), move-platform-yalc.mjs refuses dirty trees/local-only commits and uses --force-with-lease, and CI is green on the tip.

No blockers. Notes, most useful first:

  1. Tag scheme — keep yours. One repo-level v<platform-editor version> is the honest unit because consumers (core's dev-packages.json, studio's productInfo.json) pin one repository revision for both packages; per-package tags would imply an independence the consumption model doesn't have, and the publish event that justified platform_v*/utilities_v* is gone. One gap: the tag doesn't disclose the utilities version — have publish.yml put it in the release body (read packages/utilities/package.json in the workflow).
  2. move-platform-yalc locally hits the anonymous rate limit. verify-consumer-lockfile-sync.mjs sizes its 7-day window against the 60 req/h unauthenticated budget (its own comment says ~51 PRs), but npm run move-platform-yalc is the documented local command and GITHUB_TOKEN isn't set outside Actions. A 403 currently surfaces as a bare GET … -> 403 Forbidden (fetchJson, L52–56). Suggest catching 403 and telling the caller to export GITHUB_TOKEN=$(gh auth token), and saying so in the README section.
  3. #1 and #2 conflict with each other (confirmed with git merge-tree): README.md (the link-reference block you edit vs the scribe lines #2 deletes) and pnpm-lock.yaml. Whichever merges second needs a manual resolve + lock regeneration — not a rebase-clean situation.
  4. Manual npm-publish path is still fully wired: .npmrc (_authToken=${NODE_AUTH_TOKEN}, always-auth), both packages' publishConfig + prepublishOnly/postpublish, and scripts/prepare-publish.ts. CI no longer calls it, but anyone with scope rights could still pnpm publish by hand. Given the "no npm publishing" decision, either remove it or note in the README that it's deliberately kept.
  5. README "Releasing" steps 2/4 say "merge the bump-versions-<version> branch"main is protected, so that's "open a PR from it"; the old flow auto-opened one, the new composite action only pushes the branch.
  6. Repo hygiene vs the ticket (not this PR's job, but unowned today): no CODEOWNERS (named in PT-4500 item 3), delete_branch_on_merge is off (core has it on), rulesets empty (classic protection is on; details need an admin), the Reviewable badge is in the PR body but the app isn't installed on this repo, and the _nxCloudAccessToken you remove from nx.json has been in upstream's public history for a long time — worth a quick "is it inert?" with whoever owns that Nx Cloud account.
  7. The workflows are unexercised, as you say — a single workflow_dispatch of bump-versions and publish on a test tag (or a fork) before the first real release would be the actual test.

One design question that spans both repos (also raised on paranext-core#2745): this PR mints v<version> tags, but the consumers still pin the force-pushed platform-yalc branch, and this repo carries the machinery to keep that branch in sync with core (verify-consumer-lockfile-sync.mjs, move-platform-yalc). If consumers pinned the tags this release process creates, that machinery and the branch itself could retire — a bump would be a core PR changing the pin and the lockfile together. Asking here because it changes what this repo's release process needs to guarantee (nothing more than "tag = built dist matches source", which publish.yml already checks).

Three workflows set up Node 20 while `package.json`'s Volta pin — the version
every developer runs — is 22.22.0. That gap matters most in `publish.yml`,
which rebuilds `packages/*/dist` and compares it against the committed copy
before cutting the release tag consumers copy their editor out of: any
Node-sensitive difference in that output either fails the comparison with a
diff nobody can reproduce locally, or, where the difference is invisible to it,
ships bytes no developer ever ran.

`node-version-file: 'package.json'` makes setup-node read the Volta pin, so the
three stay in step with local development by construction. The repo declares no
`engines`, so the pin is what it resolves. `test.yml`'s matrix loses its node
dimension, which only ever held one value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
tjcouch-sil and others added 4 commits September 9, 2026 18:05
Core stages these packages inside its own tree and depends on them with `file:`
specifiers, so its `npm ci` validates the staged manifests' dependency sections
against `package-lock.json` and aborts on any difference. Pinning a tag whose
dependency lists differ from the revision core's lockfile was generated against
therefore breaks every core build, and any paratext-10-studio release pinned to
that tag, with no fix available from either of those repos.

Nothing said so. Release step 6 now does, and notes the case that needs
nothing: a release that only moves the packages' own version numbers, which
`npm ci` does not check for a `file:` dependency.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
`platform-yalc` is the revision paranext-core stages the editor from, and
staging is a copy of the committed `dist/` — but no workflow ran on that
branch. A commit landing there without a rebuild, whether a direct fix or a
rebase conflict resolved in `src/`, shipped stale bytes to every consumer with
nothing checking. It now runs the same job `main` does; the dist check is
`run-many`, not `affected`, so it really executes there.

`move-platform-yalc` did its reset and rebase before the consumer lockfile
check, and nothing put the branch back when that check failed — for a real
mismatch or equally for a GitHub rate-limit. The branch was left diverged from
origin with nothing saying so, and the next run's "commits origin does not
have" guard then described this script's own rebase as the operator's stray
work. It now resets back to origin's state and says the rerun is safe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
paratext-10-studio's `productInfo.json` no longer states a revision for
`devPackageRepos` — it follows paranext-core's `dev-packages.json` — so the
release steps no longer need consumers changed in two places at once, and the
warning about the two disagreeing no longer describes anything that can happen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
…ease steps

From rolfheij-sil's review of #1:

- `move-platform-yalc` is the documented local command, and `GITHUB_TOKEN` is
  set only inside Actions — so the anonymous 60-requests-an-hour budget this
  check is sized against is the one developers actually run under. Exhausting it
  arrived as a bare `GET … -> 403 Forbidden`. It now says what happened and how
  to fix it.

- A repo tag pins both packages but its name carries only platform-editor's
  version, so which scripture-utilities a consumer gets was invisible from the
  releases page. `publish.yml` now reads it and puts both in the release body.

- The release steps said to merge the `bump-versions-<version>` branch. The
  target branches are protected and the workflow only pushes the branch, so
  that is opening a PR from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
@tjcouch-sil

Copy link
Copy Markdown
Member Author

Thanks — and the scope note is fair, so I've treated the notes as leads rather than as a clean bill.

Pushed in 1b073fb3:

  • Utilities version in the release body (note 1). publish.yml now reads packages/utilities/package.json and puts both packages in the release body as a table. Agreed on keeping the tag scheme.
  • Rate limit (note 2). fetchJson now recognises a 403 that is the anonymous budget and says export GITHUB_TOKEN=$(gh auth token) instead of a bare GET … -> 403 Forbidden. It keys on x-ratelimit-remaining: 0 or the absence of GITHUB_TOKEN, so a genuine permissions 403 still reads as itself.
  • Release steps 2 and 4 (note 5). Corrected to "open a PR from the bump-versions-<version> branch and merge that" — the branches are protected and the composite action only pushes.

Also on this branch from the earlier round, since they overlap with what you looked at: move-platform-yalc now resets platform-yalc back to origin's state when the lockfile check fails (it previously left the branch rebased and diverged, and the next run then blamed the operator for its own commits); the snapshot copy moved into a mkdtemp directory; test.yml now runs on platform-yalc too, so the committed dist/ is actually verified on the branch core stages from; and all three workflows take Node from the Volta pin rather than hardcoding 20 — publish.yml most of all, since it rebuilds and compares that dist/ before cutting the tag.

Not done, deliberately:

  • Note 3 (conflict with Remove the scribe package #2). Real — I confirmed it. That is sequencing for TJ, and whichever lands second needs the manual resolve and a lock regeneration.
  • Note 4 (manual npm-publish path). Left as-is. Removing .npmrc, publishConfig, prepublishOnly/postpublish and prepare-publish.ts is more than a review fix, and I'd rather not half-remove a publish path. Worth a decision, then one change.
  • Note 6 (repo hygiene). Needs an admin; not something I can do from here.
  • Note 7 (workflows unexercised). Agreed, and I'd add: nothing I changed in publish.yml has been exercised either. A workflow_dispatch on a throwaway tag before the first real release is the actual test.
  • The Nx Cloud token. I checked: eten-tech-foundation/scripture-editors is public and its main carries the identical token, so this move doesn't change its exposure. Still worth the "is it inert?" conversation, but it isn't a finding against this PR.

On the design question (tags vs the platform-yalc branch): TJ's call, and it's open on core#2745. One data point from the other side of it — paratext-10-studio's productInfo.json no longer names an editor revision at all; devPackageRepos[].branch is now optional and absent means "use whatever core's dev-packages.json pins". So whichever way that decision goes, it is now made in exactly one place.

The 403 handling added earlier covered only half of what it needed to. GitHub
answers a rate limit with 403 **or** 429: the primary limit zeroes
`x-ratelimit-remaining`, a secondary limit sends `retry-after` instead, and the
429 form of both was not matched at all. The `!GITHUB_TOKEN` fallback also
labelled every unauthenticated 403 a rate limit, including a genuine
permissions failure — it now says so hedged rather than confidently wrong,
since a token is the right advice either way.

Verified across nine response shapes: both statuses with each header, a
permissions 403 with a token and requests remaining, 404, 500, and a bare 403
with and without a token in the environment.

The README half of that review point was missing entirely. The section on
moving `platform-yalc` now says the check spends tens of requests against the
60-an-hour anonymous budget, that Actions sets `GITHUB_TOKEN` but a local run
does not, and gives the one line that fixes it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
@tjcouch-sil

Copy link
Copy Markdown
Member Author

Correcting my earlier reply on note 2 (the anonymous rate limit) — I reported it done, and only half of it was. Both halves are now in 0d456a84.

What was missing: you asked for the 403 to be caught and for the README to say so. I did the first and skipped the second without noticing; the README section on moving platform-yalc had no mention of GITHUB_TOKEN at all. It now says the check spends tens of requests against the 60-an-hour anonymous budget, that Actions sets the token but a local run does not, and gives the one line.

And the detection was too narrow. Checking GitHub's actual behaviour rather than assuming: a rate limit comes back as 403 or 429, the primary limit zeroes x-ratelimit-remaining, and a secondary limit sends retry-after instead. My first version matched only 403-with-remaining-zero, so it would have missed the 429 form of both and every secondary limit. It also treated any unauthenticated 403 as a rate limit, which would confidently mislabel a genuine permissions failure — that case now gets the same advice (a token helps either way) under a hedged description rather than a wrong one.

Verified across nine response shapes: 403 and 429 with each header, a permissions 403 with a token and requests remaining, 404, 500, and a bare 403 both with and without a token in the environment. The check still passes end to end with and without a token.

tjcouch-sil and others added 6 commits September 10, 2026 13:43
The section a reader reaches first when asking how to move this branch gave the
raw `git rebase && git push --force-with-lease` sequence, and only mentioned
`move-platform-yalc` further down under dependency changes — so the guards were
documented somewhere other than where the question gets asked.

It now leads with the script and what it refuses (another branch, a dirty tree,
commits only on your machine, a lockfile that no longer matches), and links to
the existing section for the check, its flags, and the token. The hand sequence
stays only as what skipping it costs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
paranext-core builds these packages only when the pinned revision has no
committed `dist/` to copy. Saying it "builds them from source" describes the
fallback as if it were the path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
`verify-consumer-lockfile-sync` threw when core's `dev-packages.json` had no
`packagePath`/`stagingFolder`, with a message saying "there is no lockfile
contract to verify against it" - a no-op reported as a failure. That is core's
`main` today, and stays so until the staged-dependency change lands there, so
every push to `platform-yalc` fails and `move-platform-yalc` refuses to push.
The only way through is `--skip-verify`, which also disables the checks that DO
apply once it lands.

It now logs the reason and passes. Deliberately not extended to core PRs: the
`packagePath` and `stagingFolder` read from the branch are what locate the
manifests and name the lock entries, so a run that cannot read them cannot
compute what to compare against a PR either.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6
`etc/<package>.api.md` is API Extractor's report on the public type surface. It
is committed so that a public API change — an added export, a changed
signature, a removed type — arrives as a readable diff in the pull request that
makes it, rather than buried in a rolled-up declaration bundle.

Nothing kept it current. `extract-api` runs api-extractor with `--local`, which
rewrites the report rather than failing on drift, and neither half of the
safety net named `etc/`: the README's `git add` line listed only the two `dist`
paths, and the verifier scoped its `git status` to the same two. So changing a
public API and following the README committed a fresh `dist` alongside a stale
report, and CI passed — it regenerates `etc/` on every run and then looks away.

Add both `etc/` directories to the check and to the README, and rename the
constant, the messages, and the section heading to say "build output" now that
they cover more than `dist`. The heading rename moves its anchor, so the
in-page link to it moves too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012F9WPtSNrcpEAfFdu3trBw
`pnpm nx run-many -t extract-api` — what CI ran, what the README told you to
run, and what the freshness check's failure message printed — finishes with
`packages/utilities/dist/index.d.ts` holding tsc's 13-line re-export stub
instead of api-extractor's 648-line rollup.

`packages/platform`, `libs/shared`, `libs/shared-react` and `libs/test-data`
each carry a TypeScript project reference to `packages/utilities`, so their
`build` emits utilities' declarations into utilities' own `outDir`. That write
happens below nx's task graph: `build` dependsOn `^build` and `extract-api`
dependsOn only its own `build`, so nothing orders `utilities:extract-api`
against `platform-editor:build`, and whichever writes last wins. CI has been
passing on scheduling luck — the runner happens to overlap the two so the
rollup lands second. A pristine clone here loses that race every time, which
means following the documented steps committed a broken type surface: that stub
re-exports `./converters/**`, the shape that collided xmldom in paranext-core.

Extract one package per nx invocation, dependents before the packages they
depend on, so the last write is always an `extract-api`. Cache phasing was tried
first and does not work — `platform-editor:build` misses the cache and re-runs.

`scripts/published-packages.mjs` holds the ordered list and the rationale, and
is now the single source for the verified paths too, so the rebuild order and
the checked paths cannot drift apart. Issue #5 removes the need for any of this
ordering; until then the script is the only supported way to rebuild.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012F9WPtSNrcpEAfFdu3trBw
@tjcouch-sil
tjcouch-sil merged commit bb0778b into main Sep 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants