Repository navigation
feat: a run names its fields, and the plugin stops deciding who has access - #209
Merged
SearheiParkhamchuk merged 25 commits intoOct 26, 2026
Merged
Conversation
The per-leaf decision collapses to "is this leaf translatable, is it in the selection, and is its source non-empty". `TranslationStrategy`, both of its implementations and `StrategyContext` are deleted, and the target's current value stops deciding anything. `sourceChangedByLeaf` is replaced by the selection list, so `ChangedLeaves` stops travelling through the pipeline context. The selection algebra arrives beside it: `narrowSelection` intersects a starting list with each answer, a `TranslationFilter` is either the addresses a request named or the name of a rule, and a `LeafAnswer` is complete in itself.
Each target locale carries its own filters: the leaf addresses a request named, or the name of a narrowing rule. The stored row follows — it lives inside Payload's single `input` column, so the shape changes without a migration — and `target_lngs` becomes a list of entries rather than bare locale strings. `pickHost` loses its strategy condition, so two requests over one document that differ only in what they translate now share a single run. A row written before 1.0 still reads: `strategy: "skip_existing"` maps to the drift rule, which is the nearest meaning rather than the same one. BREAKING CHANGE: `POST /enqueue` refuses a body still carrying `strategy`, `selection` or `criteria` with a 400 that names what to send instead. See docs/DEPRECATIONS.md#the-strategy-leaves-the-run.
The handler expands the row's filters into leaf addresses and hands the pipeline a finished list; the pipeline knows nothing about filters. One rule exists, `stale`, answered through the criteria the plugin was built with — an unknown name fails the run rather than widening it to the whole document. What a run is fixed on becomes a named context built once per call and never held on the handler, because the queue runs jobs in batches and one instance serves them all. The permission checks leave the handler for the target document: every read carries the requester's identity, and the single write demands a value only the check can mint, so a write cannot be sent without the collection's rules having been asked about exactly what it carries.
Provenance keeps its one job, reporting: it answers which leaves drifted and the run decides. The recorder stops re-basing a leaf the run did not send, which a selection makes wrong — a field nobody translated would otherwise start claiming it was made from the current source, and the out-of-date marker would go quiet for it. The receipt holds a per-leaf map or nothing, so a pre-0.15.0 whole-document digest now reads as "claims nothing about any leaf". The staleness route reports which leaves drifted, because the refresh button that stood for "bring this locale up to date" goes with the strategy.
An automatic run names no filters, so it covers the whole document until auto-translate gains its own control over what a run translates. BREAKING CHANGE: `AutoTranslateStrategy` is no longer exported and `AutoTranslateConfig.strategy` is removed. A host that set `skip_existing` had automatic runs that left translated fields alone; those runs now re-translate whole documents, at full provider cost, over reviewed text. The option is refused rather than ignored. See docs/DEPRECATIONS.md#the-strategy-leaves-the-run.
The strategy select goes from both forms, and with it the refresh action on the status list — its job, "bring this locale up to date", is now a request naming the drift rule.
The suites run the real queue runner and read the stored row, so they carry the shape change: filters per target locale, the drift rule answered in the worker, and a selection that leaves its siblings untouched.
Two entries in DEPRECATIONS — the strategy leaving the run, and the stored row's `target_lngs` and `strategy` pair — plus the task contract for each step of the work and the corrected verification paragraph in the rewiring plan.
`TranslateDocumentHandler` is replaced by `translateDocument(deps, payload, input, scope)`. Four concerns that were private methods become siblings — `selectLeaves`, `runModules`, `translateOrWrap`, `reportNothingSent` — and what a run is fixed on moves to `run.ts`, so the entry's body is the order and nothing else: nine statements, one per step. Reading the source and asking the collection's rules about it become one call, `readSourceIfAllowed`, which returns a value only it can mint. Narrowing, the module chain and the provider call all demand that value, so paying the provider before the rules were asked no longer compiles. The brand's key is a symbol this module never exports, so unlike a string brand no literal can forge it. - `Handler<TInput, TOutput>` is deleted: one implementer, and the interface declared two parameters while the real call takes three — hiding `scope`, which carries the caller's transaction and identity - `model.ts` is deleted; `TaskHandlerInput` already said the same thing one layer down, and the wiring stops re-spelling the input field by field - `scope` becomes a required parameter, so a run cannot be started by accident with no requester and Payload's access checks off Nothing published changes: `src/index.ts` names neither the class nor the interface, and `exports` allows no deep import, so the removed names were reachable only from inside this package.
`mayWrite`, `rebuildRequester` and `checkTranslationPermission` leave `features/translate-document/` for `server/shared/access/permission.ts`, and `PermissionCheckFailed` and `RequesterMissing` move with them — leaving the two error classes behind would have made `shared/` import `features/`, the upward edge the move exists to remove. `visibleIds` and `getAllCollectionIds` move from `features/_lib/` to `server/shared/payload/collections.ts`; `isCollectionAvailable` stays. Both moves exist so that the entry points which must start asking the host's collection rules can reach the function that asks, without importing across a feature boundary. No behaviour changes: the bodies travelled unedited, and the only difference in the moved asker is two import lines. Four test suites mock by module path and were re-pointed; no assertion changed.
POST /translate/field and POST /translate/stale/dismiss ask the collection's update rule before any read, any provider call and any write. The field route no longer pays for a caller it would refuse; dismiss no longer lets a reader re-base the provenance receipt for everyone. Both ask through mayInitiate, which puts the empty-data probe in one place: mayWrite requires the data the write will send, and these two calls break that on purpose, so the exception is named once rather than repeated. An absent document raises no objection rather than failing the ask. Payload's evaluator loads the document itself when data is empty, and without disableErrors a missing id threw, turning a 404 into a 500. An actor-less request is still not refused; step 8 decides that.
AccessGuard.check gains an action: a closed, published union of the nine
route actions. Nine routes and both admin widgets name theirs, so a host
can allow enqueueing and refuse cancelling without writing two guards.
Implementing check({ req }) is unaffected; the property is additive.
The answer is now read in one place. Only a literal true allows, where any
truthy value used to; a guard that throws refuses instead of escaping the
error handler and breaking the request. A framework navigation signal is
not an answer and still propagates.
Three things owe a release note, which step 10 writes: the stricter
reading, the throw becoming a refusal, and the removal of the
undocumented deep import of client/shared/types/AccessGuard.
A request that names document ids had nobody ask, per document, whether the caller may change them. The update rule is now asked about each named id before anything is queued; refused ids are dropped and named in the response under `refused`, so a bulk selection of fifty with two refused still queues the forty-eight the caller is entitled to. One question per document, not one per document and locale: Payload's access rules receive no locale and it has no per-locale permissions. select_all is unchanged. It is already narrowed to what the caller may read, which Payload folds into one query, and asking the update rule there would cost one evaluation per document in the collection. The worker refuses before it spends either way. An empty string is no longer accepted as a document id. Payload skips the document load for one, so the rule was asked about nothing.
Running, cancelling and cancelling-by-collection acted on jobs with no check of whose work they were. A job is the plugin's own object and the host has no rule about it, so the question is reduced to one the host has already answered: a job may be cancelled by anyone who may change its document, and run by them too. TaskRunner gains an optional assignmentsOf(handles), so a handle can be placed on a document and a requester. Both runners implement it; a runner that does not still satisfies the interface, and the routes that need it refuse rather than proceed. run/:id also refuses a job nobody is recorded as having started. Without a requester the worker writes with the host's rules off, so letting any caller trigger one by naming its handle is the escalation this closes. A handle that resolves to nothing stays a 404, as it was — absence is not a refusal. cancel drops the handles it may not touch and names them, keeping 204 when none were refused. cancel-by-collection asks with the rows it already holds and needs no new read.
Steps before this one added asks. Each was a call a later edit could delete, and nothing would have noticed. Now enqueue, cancel and run accept only a value one of four named paths produces, so forgetting to ask does not compile. Narrowing the three methods surfaced 29 type errors across seven files, which was every place that reached the queue. Two paths ask: authorisedWork for documents, authorisedHandles for jobs. Two do not, and say so by name rather than by the absence of a call. authorisedByTheSave carries why the auto-translate hook must not ask - a host callback that throws inside the editor's afterChange rolls back their save. authorisedByTheReadFilter carries why select_all does not - Payload already folded the read rule into the query that found those documents. A refusal is reachable only through its collection, so two collections holding the same document id cannot be confused for one another. BREAKING CHANGE: code that calls TaskRunner.enqueue, cancel or run directly must obtain its argument from one of those paths. Implementing TaskRunner is unaffected: TypeScript compares method parameters bivariantly, so a runner declaring the wider types still satisfies the interface.
identityOf answered with a requester or null, and that null covered three different situations: the request carried no user, it carried one with no id, or it carried one whose auth collection this project cannot determine. The last two were getting the trust the first was granted on purpose, and the code already knew it - it logged that the write would go through without a permission check, and let it. Identity is now three-valued, and a scope can only be built from two of them, so an actor nobody can name has nowhere to act. The placeable kinds are listed rather than subtracted, which means a kind added later is refused until someone decides otherwise. Enqueue answers 403. The auto-translate hook skips instead, because inside the save's transaction a throw would roll the editor's own save back, and it says in the log why it skipped. Both unnameable branches now explain themselves; one of them used to answer silently. The no-user case is untouched, down to the shape of the scope it produces. Its pinned integration test passes unmodified. Payload's own auth always stamps the collection, so this is reachable only through a host's custom auth strategy or a local-API caller passing a hand-made user.
A module on the translation-run chain was handed the whole payload, so it could write the very document the run was translating - past the asks the routes and the queue had just been taught. The chain now hands each module a view whose writes refuse the run's own collection. Reads pass through, and so do writes anywhere else. Guarded: create, update, delete, duplicate and restoreVersion. The design named the first three; the other two were found by checking Payload's surface, and both take a collection and write. payload.db reaches the database directly and is not covered - the docblock says so rather than implying the seam is sealed. The refusal is a runtime guard, not a type. The run's collection is a value, so "any collection but this one" cannot be stated in a signature, and a narrowed type would have broken provenance, which passes the seam's payload into a factory declaring the full Payload. Provenance needed no change, as the design said; nothing breaks at compile time, so this is not the big bang the design expected.
Nine routes pass a literal action to the guard, and nothing pinned which literal each one passes: a route could ask about "enqueue" where it meant "cancel" and every check stayed green. The table that pins it is keyed by TranslatorAction, so a tenth action does not compile until the route that answers for it is named. An unattributed job refused by POST /run/:id is now pinned end to end, against the real queue runner, which step 6 owed. The release notes name every host-visible change of this work: the action on the guard and only `true` allowing, enqueue dropping documents the caller may not change, a job resolving to its document, the queue taking only branded work, an unnameable actor no longer read as no actor, and a chain module refused the run's own collection. @SInCE 1.0.0 on the three published additions. The brands and JobAssignment get none: they are not re-exported, so the package rule does not reach them.
A runner that cannot say which document a handle names cannot be asked about it, and Payload's rule is asked about a document, never a handle. Optionality bought only a worse failure: a project that builds while run and cancel answer 403, found from a status code instead of the compiler. BREAKING CHANGE: `TaskRunner.assignmentsOf` is required. A custom runner stops compiling until it is added. Its parameter is `readonly JobHandle[]`, an alias of `string`, so nothing about assignability moves. The `@since` tag is dropped — `TaskRunner` is not re-exported from `src/index.ts`, only `TaskRunnerProvider` is. See docs/DEPRECATIONS.md, "one permission decision point".
`TaskRunner`'s own docblock told a runner author to call `assertTaskRunnerContract`, while the suite lived in `__tests__`, which the build excludes — the promise could not be kept. It moves to `src/testing` and ships from a new `./testing` entry point. No minter is exported: `asWork` and `asHandle` stay private and the brand types appear nowhere in the emitted declarations. This also fixes repo-wide `check-types`, red since the brand landed. The `apps/dev` contract test took the runner factory from the built package and the suite from a path into `src/`, and a `unique symbol` makes those two copies unrelated types. Both now resolve to one copy. `vitest` becomes an optional peer dependency: the suite imports it at module level, so it is needed at import time, not at the call.
SearheiParkhamchuk
requested review from
ChiefCreator and
dogfrogfog
as code owners
October 26, 2026 11:00
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ages `mayInitiate` asks with empty `data`, which makes Payload load the document itself to answer — and a loaded document comes back in whichever locale the ask names. Three routes named three different ones: `POST /enqueue` the project default, `POST /field` and `POST /stale/dismiss` the locale being translated into. So a host rule that reads the document answered per language. Before a first translation the target locale holds nothing (`fallbackLocale: null`), so such a rule refused the very request that would have filled it, while the same rule allowed the same person through `POST /enqueue` for the same document. Rights in Payload have no language. `mayInitiate` now names the default locale itself and `targetLocale` leaves its signature, so naming the wrong one stops compiling. `mayWrite` keeps the parameter: that ask has to match the write it precedes. Two checks pin the locale, proven by replacing the choice with a literal.
…sked
`POST /enqueue` asked the host's `update` rule about every named document — or, on
`select_all`, read the whole collection — and only then refused a request carrying a user
it could not place in an auth collection.
Payload does not refuse such a user. `createLocalReq` fills a missing `user.collection`
with the admin collection and says so in its own source ("TODO (4.0): Instead of silently
falling back, throw an error"). So the rule was evaluated as whoever happens to share that
id there — a stranger — and a rule that throws turned a plain 403 into a 500 after
`docAccessOperation` had already killed the transaction.
The refusal now comes first. `handler.unnameable.test.ts` could not see the ordering
because it replaces `authorisedWork` wholesale; the new file leaves the ask in place and
watches whether it happened.
…apper drops its cast `getAllCollectionIds` defaulted its `user` parameter to `null`, and `enforcedAtTheRead` reads that as "nobody is asking" — so the read ran with the host's `read` rule not applied at all. `authorisedByTheReadFilter` then stamped every id it returned as work the rules had been asked about, which is the one claim the brand exists to make. One omitted argument was enough to make the claim false, and the suite stayed green when it was. The default is gone; forgetting the reader is now `TS2554`. `withErrorHandler` was generic over `(...args: any[])` and cast its own return with `as T`. No route ever passed anything but a request: the generality existed for two checks that exercised the generality. Those checks now state the contract instead — the wrapper hands the handler the request, and hands the handler's own response back — and the signature says `RouteHandler`, so the `any` and the cast are both unnecessary. Four checks cover what the two reads actually ask Payload for; each is proven by a mutation that ignores the reader.
Each check below was written against a mutation that the suite passed in silence, and is proven by that same mutation going red. - Every route the plugin registers answers the guard, and a navigation a guard asks for reaches the framework. Routes are composed in three separate places, so the existing table of factories cannot see a fourth; this boots the plugin and counts what it registered. Swapping the two wrappers on one route — which nothing caught — now names the route and the status it answered instead. - `getAllCollectionIds` and `visibleIds` name their reader to Payload. Nothing mentioned `enforcedAtTheRead` anywhere, across five callers. - A refused document is named once however many locales asked for it. Every fixture that refused anything asked for one locale, where a per-task list and a per-document list cannot be told apart. - A row carrying both the pre-1.0 locale list and the current targets answers from the current shape. Appending a locale to an old row produces exactly that row, and reversing the branch order left all 325 checks green. - A selection naming a leaf the schema does not offer sends nothing. The floor was covered only where the selection came from the schema, never where it came from a request. - A leaf the provider answered in part is not claimed as current. `legacy-row-upgrade.int.test.ts` carries the same upgrade path end to end, through the real queue runner: no integration suite replayed a row written before 1.0. The positive controls in both pin files become named exports, so what they assert is in the name rather than in a comment above an unguarded call.
SearheiParkhamchuk
changed the title
Fifty-eight comment blocks were judged with fresh eyes against the project's own rule —
only non-obvious why. Eight said something false, which is worse than verbose: a reader
builds on it.
- `authorisedByTheReadFilter` promised the host's `read` rule had been folded into the
query that produced its documents. Dropping the reader at the call site left every
check green, so the guarantee was not one. The text now states the obligation on the
caller, which is what the signature actually carries.
- `PermissionCheckFailed` claimed the only `mustPropagate = true`. There are two.
- `AuthorisedHandle` linked `{@link TaskRunner.cancel}`; `TaskRunner` is not imported
into that file, so neither link resolved.
- Two comments cited payload 3.84.1 against an installed 3.90.1. Both underlying claims
were re-checked and survive, so only the citations moved.
- `AccessGuard`'s `@example` still showed `check: ({ req })` after this branch added
`action` — the one copyable line in the published docs taught a guard that ignores the
parameter the release exists to add.
- `AccessGuardRequest` still said "API call" after the gate grew to cover widget renders.
- A wire type claimed snake_case "like every other translation endpoint";
`get-document-status` returns `relationTo`.
- A deprecation note named a version, which the ledger's own rules forbid.
Net 16 lines fewer, with every fact kept: what went is motivation, history and
re-derivation. `withAccessCheck`'s docblock said only what its signature says and what
a pin already holds.
|
🎉 This PR is included in version 1.0.0-next.6 🎉 The release is available on npm package (@next dist-tag) Your semantic-release bot 📦🚀 |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #208
Part of #48, which keeps the field-picker tree and the admin forms.
The second half of this branch — one permission decision point — has no issue of its
own. The nearest is #113, translated writes bypass collection access control, closed on
2026-10-02 by the partial fix it describes; what lands here is the rest of it, measured as
six gaps in
docs/plans/2026-10-25-asking-payload-everywhere.design.md.Two bodies of work, twenty commits, one branch. They landed together because the
second rests on the first: a run that names its fields is also a run whose fields have
an owner, and the permission work asks about that owner per document.
The branch name only says the first half.
Part 1 — a run names its fields, not a strategy
1a143e71…c792ac33A translation run used to carry a strategy —
overwrite, orskip_existing. Thestrategy existed because nobody ever asked the editor which fields they meant, so a rule
had to guess, and two of its clauses were guesses about state the caller never stated:
"is the target empty", and "did the source move".
A run now carries the list of leaves to translate, or nothing, plus a list of named
narrowing rules. One rule exists,
stale, answered by provenance. Ticks set the startinglist — every translatable leaf when nobody ticked anything — and each rule narrows it.
Narrowing is intersection, so rule order does not matter.
TranslationStrategyis gone as a port, with both implementations, from the pipeline,the HTTP input, the stored row, the host-facing task and both admin forms. The chain is
assembled per target locale instead of once at boot, because a rule named on one locale
and not another cannot be served by a list decided at startup.
Two defects the selection exposed are fixed here: the receipt no longer re-bases leaves a
run did not send (which would have silenced the out-of-date marker for a field nobody
translated), and the staleness route reports which leaves drifted, since the refresh
button that used to stand in for that is retired with the strategy.
Part 2 — one permission decision point
bc761f64… HEADThe plugin never decides who has access. It asks twice and answers neither question
itself:
AccessGuardis asked whether this kind of request is allowed at all,now with the action named —
check({ req, action });access.updaterule is asked about each document.Nine routes, the queue, the two admin widgets and the translation chain sit behind one or
both. The queue's three methods take branded parameters that only the plugin's own
handlers can mint, so work cannot reach it without an ask having happened — the brands are
deliberately not exported, because an exported minter would be a way around the thing being
prevented.
Three routes that used to act on an opaque handle now resolve it to a document first:
POST /run/:id,DELETE /cancel,DELETE /cancel-by-collection/:slug. A client cannotsupply that document itself — it would be claiming which document someone else's handle
points at — so the mapping is read from the runner, which is the only thing that owns the
job storage.
Two narrower holes close with it. A request whose user carries no
collectionused toread as no user, and a write with no user runs with Payload's checks off; it is now a
refusal. And a member of the translation chain used to receive the whole
payloadandcould write the very document under translation, past every ask above; it now receives a
view whose writes to the run's own collection throw.
Breaking changes
Each is documented in
docs/DEPRECATIONS.mdwith what a host sees and what to do.TranslationStrategy, its two implementations andcreateTranslationStrategyAutoTranslateStrategy,AutoTranslateConfig.strategyAccessGuard.check{ req, action }; onlytrueallows, and a throw refuses instead of becoming a500./client/shared/types/AccessGuarddeep importTaskRunner.enqueue/cancel/runTaskRunner.assignmentsOfvitestDELETE /cancel200 { refused }where it only ever answered204One addition worth naming
@focus-reactive/payload-plugin-translator/testingis a new entry point exportingassertTaskRunnerContract— the suite that asserts every obligationTaskRunnerowes itscallers.
TaskRunner's own docblock has been telling runner authors to call it while itsat in a directory the build excludes, so the promise could not be kept; now it can. No
brand minter is exported with it.
It also closes a hole this branch opened. The brand on
AuthorisedWorkis aunique symbol, which makes the built copy and the source copy unrelated types, and the one filethat imported both —
apps/dev's contract test — broke repo-widecheck-typesthe momentthe brand landed. Nothing caught it, because each step verified the package rather than the
repository. Both imports now resolve to one copy.
Left open, deliberately
server side it will talk to.
lands, an automatic run covers the whole document.
widening it later is a redesign, not a new name.
caller.
POST /fieldstill bypasses the runner, writes no receipt and does not clear staleness.turbo.jsonhascheck-typesdepending on^check-typesrather than^build, so therepo-wide type-check needs a
dist/that already exists. On a clean clone it fails withan unresolved module instead of a type error. Worth changing, and it touches every
package's task graph, so not here.
DELETE /canceldrops a handle the runner cannot place and answers204when everyhandle was unplaceable. Saying so needs a third category beside cancelled and refused,
which is an endpoint-contract change rather than part of this one.
Verification
Run from
packages/payload-plugin-translatorunless stated.bunx vitest runbun run check-typesbunx oxlint .bunx turbo run build(repo root)bun run test:integration(apps/dev)bunx turbo run check-types(repo root)apps/dev's ownbuildscript fails on aSeoFieldPathsmismatch coming frompackages/payload-plugin-seo, and on a vitest config overload. Both pre-date this branchand neither is touched by it.