Skip to content

docs(publisher-services): specify BE-04 durable distribution jobs - #814

Merged
ja573 merged 9 commits into
developfrom
feature/publisher-services/be-04-spec
Aug 14, 2026
Merged

docs(publisher-services): specify BE-04 durable distribution jobs#814
ja573 merged 9 commits into
developfrom
feature/publisher-services/be-04-spec

Conversation

@ja573

@ja573 ja573 commented Aug 14, 2026

Copy link
Copy Markdown
Member

Task: BE-04-SPEC (remediated under BE-04-SPEC-REMEDIATION-01, then BE-04-SPEC-REMEDIATION-02, then BE-04-SPEC-REMEDIATION-03)
Future implementation: BE-04 - Durable distribution jobs
Risk: HIGH
ORIGINAL AUTHORING BASE: fac86e38383e2059e8795698e1585932c35b5b6d (merge commit of PR #813, verified before any edit; preserved as historical authoring evidence)
BASE RECONCILIATION MERGE: 1cf5675c4c2f065feab8ccfb3cde06c368588aa6, merging repository-authoritative develop at 8703dd5ca2080bb97debc9d14cca33db9956f7b4 (merge commit of PR #815 / ADR-0008) into this branch with one ordinary merge commit
PR target: develop

Specification/control only.
Specification: DRAFT and NOT APPROVED.
BE-04 implementation: NOT AUTHORIZED.
Automatic job creation: OFF / NOT ACTIVATED.
Deployment: NONE. Production migration: NONE. Worker deployment: NONE. Production access: NONE.


Status

ADR-0008 is now repository-authoritative through PR #815, and this specification has been reconciled with it substantively.

The specification is not approved. Three independent review rounds identified remediation requirements — four findings in the first, five in the second, and two residual consistency defects in the third — and all eleven are addressed here; fresh exact-head independent review and explicit CTO specification approval remain required. Satisfying ADR-0008 is a necessary and not a sufficient condition for BE-04 implementation authorization — it approves no part of this candidate.


Base reconciliation after ADR-0008

This branch was authored from the ORIGINAL AUTHORING BASE above, before #815 merged. Current develop was then merged into this branch with a single merge commit — no rebase, amend, squash or force push. Conflicts arose in exactly two files and were resolved by keeping both sides' current truths:

  • CHANGELOG.md — the repository-authoritative ADR-0008-RECORD entry from develop and the BE-04-SPEC entry from this PR are both retained under the single existing ## [Unreleased] / ### Added headings. The ADR-0008-RECORD entry is not modified by this PR.
  • docs/publisher-services/task-status.md — the Last updated line, the single active BE-04 row and the programme notes carry both sides' facts.

The two bases are deliberately distinguished and neither replaces the other: fac86e38 is the authoring base the section 2.2 repository evidence was read from, and 8703dd5c is the repository-authoritative control state from which ADR-0008 governs. Between those two commits the repository changed only CHANGELOG.md and files under docs/ — no Rust source, migration, generated contract, workflow or manifest — so every merged-code fact in section 2.2 continues to hold. A later implementation base is separate from both and must be freshly verified.


First-round independent-review findings, and how each is resolved

# Finding Resolution
1 OFF mode could lose an onboarding. Section 9.4 permitted a SUPERUSER_API AutomaticPush activation to commit with no job while automatic creation was OFF; no sweep, retry or later replacement would ever create one. Section 9.4 rewritten. Such a transaction now fails and rolls back in full, with an enumerated zero-committed-change result (assignments, activation_id, configuration token, publisher row and its trigger effect, audit row, job and target rows). PullFeed, Manual, package-only, repair, disable and MIGRATION_BACKFILL writes remain permitted — the last as an explicit migration boundary, not a missed onboarding. No global sweep is introduced and turning the switch ON still enqueues nothing retroactively. One bounded new error, ThothError::DistributionJobCreationDisabledDISTRIBUTION_JOB_CREATION_DISABLED: the repository had no existing feature-disabled error, and reusing StalePublisherServiceConfiguration or DistributionPlatformNotAssignable would have distorted a merged contract. The new-error count moved from two to three at this round, reconciled in scope item 10, section 16.3 and the tests. Superseded: the second round's finding 5 raised it to four, and the third round's finding 1 corrected the section 9.4.3 sentence that still said "two to three".
2 Five attempts could become six. Lease expiry returned an expired RUNNING job to PENDING without consulting the budget, so an expired fifth attempt was claimable and T1 incremented to six. The "hard-bounded" claim was untrue: the check only enforced >= 0. T5 split into T5a (attempt_count < MAXPENDING, available_at now, count unchanged) and T5b (attempt_count >= MAXFAILED, completed_at set, never claimable again), decided inside the recovery statement from the row's own count. Claim eligibility independently requires attempt_count < DISTRIBUTION_JOB_MAX_ATTEMPTS. distribution_job_attempt_count_check becomes attempt_count >= 0 AND attempt_count <= 5, with a test tying the Rust constant to the migration literal. Three independent guards; the budget is not weakened.
3 Repair was treated as evidence of delivery. The matrix justified Repaired ⇒ no job with "the adapter was already active", which desired-state rows cannot establish. The rule is unchanged; its reason is corrected. Activated = zero enabled members ⇒ newly enabled desired-state group; Repaired = normalization where at least one member was already enabled; automatic creation is tied to a new Activated event, and a repair creates no job because it is not one. Sections 9.1, 9.2 and 17.3 now state explicitly that no inference about adapter execution, external upload or back-catalogue delivery is made, and that latestBackCatalogueJob: null means only "BE-04 holds no durable job". Any future policy for onboarding historical or unknown-delivery state is separate and unauthorized here.
4 The claim SQL returned no rows. The normative statement ended in INSERT ... SELECT FROM claimed, which produces no result set, so claimDistributionJobs had nothing to return without a second, non-atomic query. The statement now carries an inserted_attempts CTE and a final SELECT ... FROM claimed JOIN inserted_attempts with the deterministic ORDER BY — one attempt per claimed job, exactly the jobs this invocation claimed, zero rows when none, no second claim query, no other worker's claim exposed, SKIP LOCKED and its justification retained. Target and payload resolution is specified as a bounded set of = ANY($ids) statements with no N+1 path.

Fresh-review findings (second round), and how each is resolved

A fresh independent full review of the remediated specification identified five further findings. The four above were re-checked for regression and are preserved as corrected; the corrections below touch their wording only where consistency required it.

# Finding Resolution
1 The migration locking model was factually wrong. Section 19.3 claimed that "every object created is new, so the migration takes no lock on any existing table". distribution_job declares foreign keys to public.publisher and public.work, and under PostgreSQL 17 creating or adding a foreign key acquires a SHARE ROW EXCLUSIVE lock on the referenced table for the duration of the transaction. Section 19.3 now states plainly that the migration does acquire table-level locks on the existing populated public.publisher and public.work, names the mode, and states what it blocks (concurrent INSERT/UPDATE/DELETE, not SELECT) and what it can wait behind. The genuinely true claims are preserved and kept distinct: the distribution_job* relations are new and empty, no existing application table is rewritten, there is no backfill or validation scan, and the migration creates zero job rows. The foreign keys are not weakened, deferred, made NOT VALID or dropped to avoid the finding — that is stated as explicitly unauthorized. No production duration is claimed. Section 25.3 now requires: the forward migration on a representative populated disposable PostgreSQL; observed pg_locks captured from a second session while the migration runs, naming publisher and work with their exact modes; the transaction/migration duration; a deterministic lock-contention fixture showing what happens when another session already holds a conflicting writer lock; the unchanged-relfilenode proof that no rewrite occurred despite the locks; the down migration on a populated database; and schema.rs reconciliation. Sections 19.3 and 22 record that production migration execution remains separately authorized and that its lock window must be accounted for in that authorization, with disposable measurements as inputs, never a production prediction.
2 lastError semantics contradicted themselves. Section 7.2 said last_error_code/last_error_detail "mirror the most recent attempt's values", but T5a and T5b both close the newest attempt as ABANDONED and leave last_error_* untouched — so the fields demonstrably did not mirror the newest attempt. One coherent BE-04-local semantic is fixed: the fields are the most recent worker-reported failure of the job. T3/T4 set them from the worker-reported FAILED attempt; T2 success clears them; T5a and T5b leave them untouched; and cancellation (T6, T7, T8) is stated explicitly as neither setting nor clearing them. The consequences are stated rather than left to be found: a job terminalized by T5b may legitimately have a null lastError when no worker ever reported a failure, and that null must not be patched with a synthetic code; where an older worker failure exists it is retained, and it must not be described as the reason the current attempt was abandoned or as the direct terminalization cause; attempt history remains the authoritative record that the final attempt was ABANDONED on lease expiry. The mirror alternative is rejected on its own terms — distribution_job_attempt_error_result_check forbids error fields on a non-FAILED attempt, so mirroring an ABANDONED attempt could only be done by inventing an error no worker reported. Reconciled across sections 7.2, 11.2, 14.1, 16.2, 17.1, 18.2, 25.1, 25.10 and 26, with six new required tests: T5a and T5b each with and without a previous FAILED attempt, success clearing, and cancellation behaviour.
3 The staff-report statement count did not add up. Section 17.4 claimed a five-statement bound while its own table described a publisher page, a configuration-change statement, the assignment loader, the latest-job loader, the target loader and the attempt loader. The target and attempt loaders are separate DataLoaders and each issues its own set-based statement. The minimum correction is taken: the full-field bound moves from five to six; the target loader and the attempt loader become separate numbered rows; all six statements remain set-based; the count stays constant in N for the supported bounded page size; and the three new BE-04 DataLoaders (latest job, targets, attempts) are preserved. The two loaders are explicitly not combined to preserve the accidental number five. The bound is now also stated per dispatch chunk — with the exact arithmetic required if any allowed page size ever produces more than one chunk — rather than assuming a single chunk. Sections 25.1, 25.12 and 26 require the measurement at page sizes 1, 25 and 200. Superseded in part: the third round's finding 2 found that the "full job-aware selection set" named here cannot reach six, and split the requirement into an explicitly five-statement job-only selection and a six-statement full report selection.
4 The role-composition wording overreached ADR-0008. Section 15.3 said materially that "roles compose additively in the merged model", which asserts a general rule ADR-0008 deliberately declines to make. Replaced with the narrow, BE-04-owned statement: the BE-04 authorization matrix explicitly permits a principal that holds both SUPERUSER and DISSEMINATION_WORKER to exercise the independently authorized operations of both roles. This is a BE-04-specific matrix decision and does not establish a general role-composition, aggregation or inheritance rule. Preserved unchanged: SUPERUSER alone does not get worker operations; DISSEMINATION_WORKER alone does not get administrative operations; a dual-role principal may be allowed both only because BE-04's matrix says so; no Metrics-role consequence; no generic machine-role rule beyond ADR-0008; no credential or provisioning architecture. A sweep of the specification, the report and this body found no other equivalent general claim.
5 The invalid errorCode API contract was unspecified. Sections 18.1 and 25.10 required a malformed or over-length worker error_code to be rejected with a "stable error", while section 16.3 fixed exactly three new ThothError variants and named none of them for it — leaving the implementing agent to invent the contract. The merged thoth-errors model was inspected and reuse was rejected on the evidence: InvalidSubjectCode is subject-code specific and echoes the caller's input back; InvalidUuid, InvalidTimestamp, InvalidFileExtension and InvalidMetadataSpecification name different subjects; every other candidate falls through into_field_error's catch-all to INTERNAL_ERROR, which is not acceptable for a deliberately validated worker input contract and would make a client contract violation indistinguishable from a database outage. One bounded variant is therefore added: ThothError::InvalidDistributionJobErrorCodeINVALID_DISTRIBUTION_JOB_ERROR_CODE, with the stable sanitized public message The supplied distribution job error code is not a valid classification code. — a fixed string that echoes no part of the rejected value, discloses nothing further, is raised at resolver entry so no job or attempt state changes and the claim token stays valid. "Exactly three" becomes "exactly four" everywhere (section 16.3, scope item 10, sections 25.1, 25.10, 26, the CHANGELOG). One factual correction was made while specifying it: CompleteDistributionJobInput carries no errorCode, so failDistributionJob is the only operation that can raise this error, and no errorCode field is added to the complete input to manufacture symmetry — both input paths are otherwise unchanged.

Provisioning wording — consistency sweep only (second round). Section 15.5's existing statement was retained rather than rewritten, with one clarifying sentence separating the halves: editing the zitadel.rs setup role list is an ordinary repository source change inside BE-04's implementation scope, while executing zitadel setup, creating the role, granting it and issuing or rotating credentials are separately authorized operational actions outside this specification's authority. No provisioning architecture is invented and no scope is widened.

Third-round findings (residual consistency), and how each is resolved

A fresh independent review of the twice-remediated specification found no new architectural defect and two residual consistency defects, both created by the second round's own corrections. Every earlier correction is preserved and was re-checked for regression.

# Finding Resolution
1 The error inventory in section 9.4.3 was stale. After the second round added InvalidDistributionJobErrorCode, section 16.3 fixes four new ThothError variants — but section 9.4.3's live text still said that section 16.3 "raises the total new error count from two to three accordingly", which a reader arriving at the fail-closed rule would take as the current inventory. Section 9.4.3 now states the two facts that are true: ThothError::DistributionJobCreationDisabled is exactly one bounded new error for the creation-disabled condition and the only new error the configuration path raises; and it is one of the four variants section 16.3 fixes as BE-04's complete error inventory, the other three (StaleDistributionJobClaim, DistributionJobAlreadyTerminal, InvalidDistributionJobErrorCode) belonging to the worker operations of section 16 and to section 18.1's errorCode validation contract. The history is not rewritten to suggest InvalidDistributionJobErrorCode was ever an OFF-mode error — it was added for worker input validation and is recorded that way. A sweep of the live normative specification found no other two-to-three or three-error total; the only surviving three-variant sentences are the explicitly past-tense chronologies in the implementation report and in the round tables above.
2 The six-statement bound was asserted against a selection that cannot reach it. Section 17.4's table correctly enumerated six statements including BE-02's assignment loader, but the acceptance text defined the measured selection as "the full job-aware selection set" — latestBackCatalogueJob with its targets and attempts. That selection never invokes the assignment loader: merged BE-03 resolves enabledDistributionPlatforms through BE-02's request-local loader from that field's own resolver, so a query that does not select the field does not issue the statement. The specified test would have measured five and been asserted against six. Two selections are now named, and both are measured at page sizes 1, 25 and 200. Full job-only selectionlatestBackCatalogueJob { … targets { … } attempts { … } } with no enabledDistributionPlatforms — expected five statements: publisher page, latest configuration change, latest-job loader, target loader, attempt loader. Full report selection — the same job selection plus configuration { … enabledDistributionPlatforms } — expected six: the five above plus the existing BE-02 assignment-loader statement. The mandatory page-size evidence uses the full report selection when asserting six, and additionally asserts five for the job-only selection, so the test pins GraphQL selection-dependent execution rather than merely counting queries: it fails if a job field silently acquires the assignment loader, if an unselected loader dispatches anyway, or if any loader stops batching. The arithmetic is stated mechanically — job-only = 2 root + 3 job loaders = 5; full report = 2 root + 1 assignment loader + 3 job loaders = 6. Sections 17.4, 25.1, 25.12 and 26 item 16 were updated together.

Nothing was combined to reach a number. The three new ADR-0007 loaders remain three separate loaders, each built through configured_loader with the approved explicit 200/10; every statement remains set-based with no N+1 path; the per-dispatch-chunk qualification is retained and now covers both bounds; and the full-page bound remains a single dispatch at page sizes up to and including 200. No architectural decision was reopened, no bound weakened, no loader merged, no error removed and no scope widened.

ADR-0008 reconciliation

ADR-0008 is added to the Dependencies header as APPROVED AND REPOSITORY-AUTHORITATIVE through PR #815 and to section 2.1's approved-ADR hierarchy. Section 6.3 is rewritten from an open "adjacency finding" into a durable ADR-0008 boundary: the cross-programme question was escalated, ADR-0008 resolved it, DISSEMINATION_WORKER is approved as Publisher-Services-specific, Thoth Metrics WP5 does not use that role and its eventual role name and permissions remain its own work, BE-04's distribution_job* tables/Rust types/lifecycle APIs remain programme-local, there is no generic shared job framework, and the ADR prerequisite is satisfied — while approving nothing in this document and authorizing no implementation.

A new section 6.4 enumerates ADR-0008's exactly seven approved conventions (PostgreSQL durability, explicit state machines, database uniqueness, leases, claim tokens, deterministic idempotency, FOR UPDATE SKIP LOCKED where justified) and then attributes BE-04's own requirements — stale-token rejection, deterministic ordering, database-enforced concurrency, bounded lease semantics, the deduplication formula, the GraphQL worker protocol, the permitted/forbidden operation lists, credential provisioning — to BE-04's HIGH-risk requirements, to the AGENTS.md controls, or to outside the task. None is presented as additional ADR-0008-approved cross-programme architecture.

SUPERUSER remains denied the three worker operations — now stated correctly as BE-04's own least-privilege choice, since ADR-0008 requires neither allowing nor denying it, and since sharing an unscoped-check implementation pattern with is_superuser() is not sharing authority. Former stop condition 13 — which contemplated the CTO later deciding machine identity must be settled first — is spent and is replaced by an ADR-0008 compliance stop condition. Section 32 no longer claims the section 6.3 matter is unresolved.

ADR-0008 itself, PR #815 and PR #799 are untouched.


What this PR contains

File Change
docs/engineering/ai-delivery/tasks/BE-04.md NEW, then REMEDIATED THREE TIMES - the bounded BE-04 specification candidate (DRAFT, not approved)
docs/engineering/ai-delivery/implementation-reports/BE-04-SPEC-implementation-report.md NEW, then REMEDIATED THREE TIMES - bounded evidence record, incl. all three rounds' finding remediation (sections 5.4.1-5.4.3) and the ADR-0008 reconciliation. Its documentation-only, path-containment and untouched-file evidence blocks now compare against origin/develop with a three-dot diff rather than against the superseded authoring base, since the base-reconciliation merge brought develop's ADR-0008 files into the branch; the recorded results are re-verified and unchanged
docs/publisher-services/task-status.md MODIFIED - durable BE-04 tracker state
CHANGELOG.md MODIFIED - one BE-04-SPEC entry under the existing ### Added; the merged ADR-0008-RECORD entry is not altered

No change under thoth-api/, thoth-api-server/, thoth-client/, thoth-errors/, thoth-export-server/, src/, migrations/, .github/, Cargo.toml, Cargo.lock or Makefile. schema.rs and policy.rs are byte-identical to the base. No migration created. No role created or granted. No identity-provider action. No implementation branch created.

What the specification settles

  • Data model - four closed enums (distribution_job_kind, distribution_job_status, distribution_job_attempt_result, distribution_job_cancellation_reason) and the distribution_job / distribution_job_target / distribution_job_attempt relations with every named FK, unique constraint, check constraint and index fixed. Claim token, worker identity, claim time and lease expiry are non-null iff the job is RUNNING; attempt_count is bounded 0..=5 by the database; UNIQUE (claim_token) binds a token to exactly one attempt for all time.
  • Deduplication - derived from BE-02's real activation semantics (one shared activation_id per linked group, newly minted on every enable/re-enable/repair) as PUBLISHER_BACK_CATALOGUE:<publisher_id>:<activation_id>, stored in one unique text column and proven equal to that formula by a database check constraint. Idempotency is ON CONFLICT DO NOTHING, not check-then-insert. One linked OAPEN/DOAB activation ⇒ one job, two targets, one adapter execution; a genuine re-enable ⇒ a new legitimate job; a linked-state repairno job, with no delivery inferred.
  • Creation matrix - AutomaticPush new activation creates; PullFeed, Manual, repair, package-only, no-op, stale and MIGRATION_BACKFILL never do; a qualifying activation under OFF fails closed; behaviour read from code-owned descriptors, never inferred from names.
  • Atomicity - creation extends BE-03's existing single coordinator transaction between the lifecycle writes and the single publisher UPDATE. No second transaction, savepoint, hook or after-the-fact path. Inherits and restates the set_work_updated_at_with_relations N-work-row cascade rather than claiming a single-row transaction.
  • State machine, claims and leases - complete transition graph with the T5a/T5b budget split and the lastError = most-recent-worker-reported-failure semantic; FOR UPDATE SKIP LOCKED selected against four recorded alternatives and justified for this workload; deterministic ordering; clamped batch and lease; eligibility requiring every target still enabled under the job's own activation and the budget unexhausted; a single atomic claim statement that returns exactly its own claimed rows; attempt creation atomic with the claim; budget-aware lease-expiry recovery; bounded exponential backoff as an absolute timestamp; a five-attempt budget that is five on every path.
  • Retry and cancellation - retry encoded by failDistributionJob(retryable: true) rather than an invented mutation; superuser-only cancelDistributionJob justified by the approved rollout plan's stage-6 rollback, invalidating a live claim, failing closed from every terminal state, deleting no history, and explicitly unable to undo an already-performed external upload.
  • Worker authorization - one narrow unscoped DISSEMINATION_WORKER role, approved by ADR-0008 as Publisher-Services-specific, with its operation-level matrix owned here; exactly three permitted operations; no publisher scope, no Metrics permission, no CDN_WRITE reuse; worker identity derived from the authenticated principal; SUPERUSER deliberately denied the worker operations while retaining cancel and the report; a dual SUPERUSER + DISSEMINATION_WORKER principal allowed both roles' operations as a BE-04 matrix decision that creates no general composition rule; complete deny/allow matrix; no credential or provisioning mechanism specified.
  • Additive reporting - one nullable latestBackCatalogueJob on BE-03's superuser-only summary plus jobStatuses / withoutBackCatalogueJob filters. "No job" is null, never collapsed into a status and never read as delivery evidence. BE-03's configuration-only surfaces are untouched. Three new ADR-0007 request-local loaders (latest job, targets, attempts) keep the statement count constant in page size, at two selection-dependent bounds: six set-based statements for the full report selection, which includes enabledDistributionPlatforms and so invokes BE-02's existing assignment loader, and five for the full job-only selection, which does not. Both are measured at page sizes 1, 25 and 200.
  • Inactive by default, and fail-closed - a specified THOTH_DISTRIBUTION_JOB_CREATION switch defaulting to OFF, following the merged mutation-guard convention, consulted at exactly one site, with no sweep, backfill, startup scan or lazy creation — and, while OFF, a qualifying SUPERUSER_API AutomaticPush activation is refused rather than committed jobless.

Rollout and rollback implications

Merge remains inactive; migration execution is separately authorized; the worker is deployed before automatic creation is enabled; while creation is OFF a new SUPERUSER_API AutomaticPush activation fails closed rather than committing without a job; creation is enabled only under separately approved rollout control; then pilot one controlled publisher/platform, observe, and expand only on evidence. Turning ON performs no uncontrolled global sweep. Rollback by setting creation OFF prevents further qualifying activations from committing without jobs.

Explicit boundaries

No generic job framework, universal lease abstraction, shared worker/service-role convention, generic machine role, metrics-job abstraction, universal queue or cross-programme identity model is established; a future generic job/queue abstraction requires its own later cross-programme ADR. No observed-delivery state, work-level choice, work-upsert or withdrawal job, metadata outbox, delivery fingerprint, scheduled reconciliation or uploader execution is specified. thoth-dissemination remains the execution engine; DIS-02 consumes these jobs and is not implemented here. MIG-01 still creates zero onboarding jobs; APP-01, APP-02 and DIS-01 keep their recorded dependencies.

Lifecycle

BE-04.md is DRAFT, is not approved, and its implementation authorization is recorded as SEPARATE AND ABSENT. BE-04 implementation requires, later and separately: fresh exact-head independent review of this specification and any further remediation it requires; exact CTO specification approval; that approved content reaching develop; a freshly verified implementation base; and explicit CTO implementation authorization. feature/publisher-services/be-04 does not exist and must not exist before then.

Live review, authorization and merge evidence is this pull request's record (ADR-0005) and is not transcribed into repository files.

ja573 added a commit that referenced this pull request Aug 14, 2026
The original ADR-recording prompt expanded the CTO-approved ruling after
approval. Independent review identified that governance error. This
remediation narrows the repository record to the exact five decisions the
CTO approved on 2026-08-14; no new CTO decision was made and no
re-approval was sought.

Decision 1 is narrowed back to a dedicated least-privilege domain-specific
role, no generic catch-all machine role, unscoped only for a genuinely
global workload, explicit policy guard, explicit authorization matrix, and
SUPERUSER not automatically implying machine-role authority. Mandatory
permitted-operation lists, forbidden-operation lists and separate
provisioning/credential controls are no longer presented as approved
ADR-0008 architecture; they are referenced only as existing controls or as
requirements owned by an adopting specification.

Decision 2 keeps the approved content and drops the additional Metrics
entitlement/credential architecture. Decision 3's approved list is narrowed
to exactly the seven approved conventions, removing deduplication keys as a
separate convention, bounded lease semantics, stale-token rejection,
deterministic ordering and database-enforced concurrency. Decision 4's
boundary is recorded around the distribution_job* tables, Rust domain types
and lifecycle APIs, no longer separately elevating GraphQL operations, the
state machine or a service-worker protocol. Decision 5 is recorded exactly,
with the ADR-0005 exact-head-review and merge mechanics distinguished as
existing repository-process controls rather than approved clauses.

PR #814 and BE-04's specification are untouched; the removed properties may
still be required by existing AGENTS controls or by an adopting
specification. Documentation only: no runtime, policy, schema, migration,
workflow or production change, and no implementation authorization.
ja573 added 5 commits August 14, 2026 16:27
Reconcile feature/publisher-services/be-04-spec with repository-authoritative
develop at 8703dd5 (PR #815 / ADR-0008 merge).

Conflicts resolved in CHANGELOG.md and docs/publisher-services/task-status.md
only, preserving both ADR-0008's repository-authoritative control state and the
BE-04-SPEC specification candidate's DRAFT, BLOCKED, NOT AUTHORIZED state.

Documentation and control records only. No runtime, schema, migration, GraphQL,
workflow, role, deployment or production change.
Correct the four independent-review findings in the BE-04 specification
candidate and reconcile it with the now-repository-authoritative ADR-0008.

- OFF mode no longer loses an onboarding. A SUPERUSER_API transaction that
  would produce a new Activated group with an AutomaticPush member while
  automatic job creation is OFF now fails and rolls back in full, with an
  enumerated zero-committed-change result and one bounded new error,
  DistributionJobCreationDisabled. PullFeed, Manual, package-only, repair,
  disable and MIGRATION_BACKFILL writes remain permitted, and no global sweep
  is introduced.
- Five attempts now means five. Lease-expiry recovery is split into T5a
  (within budget, back to PENDING) and T5b (budget exhausted, directly to
  FAILED), claim eligibility independently requires an unexhausted budget, and
  the migration bounds attempt_count at 0..=5, tied by test to
  DISTRIBUTION_JOB_MAX_ATTEMPTS.
- Repair is no longer treated as evidence of delivery. The rule is unchanged,
  but its reason is now that a repair is not a new zero-enabled-to-enabled
  activation; no adapter execution, upload or back-catalogue delivery is
  inferred, and a null latestBackCatalogueJob means only "no durable job".
- The normative claim SQL now returns the jobs it claimed, through an
  inserted_attempts CTE and a final ordered SELECT over claimed, in one atomic
  statement, with target and payload resolution specified free of an N+1 path.

ADR-0008 is recorded as a dependency and in the authority hierarchy, section
6.3 is rewritten as a durable ADR-0008 boundary, a new section 6.4 enumerates
the seven approved shared conventions and attributes BE-04's own concurrency
requirements correctly, and former stop condition 13 is replaced by an
ADR-0008 compliance condition.

Specification and control records only. The specification is not approved and
BE-04 implementation remains NOT AUTHORIZED.
Remediate the five findings of the fresh independent full BE-04 specification
review. Documentation and control records only; the specification remains DRAFT
and not approved, and BE-04 implementation remains NOT AUTHORIZED.

1. Migration locking model corrected. Section 19.3 claimed the migration takes
   no lock on any existing table. That was false: distribution_job declares
   foreign keys to public.publisher and public.work, and PostgreSQL 17 takes a
   SHARE ROW EXCLUSIVE lock on the referenced table while a foreign key is
   established. The section now states the locks, their mode and what they
   block, preserves the true claims (new empty relations, no rewrite, no
   backfill, zero job rows), refuses to weaken the foreign keys, claims no
   production duration, and requires observed pg_locks evidence, duration, a
   deterministic lock-contention fixture and a relfilenode proof.

2. lastError semantics made coherent. The fields are fixed as the most recent
   worker-reported failure: set by T3/T4, cleared by T2, untouched by T5a, T5b
   and all cancellation. A T5b-terminalized job may legitimately carry a null
   last error; a retained older failure is never the abandonment or
   terminalization cause; attempt history stays authoritative. Six new tests.

3. Staff-report statement count corrected from five to six, with the target and
   attempt loaders as separate numbered rows, all six set-based, the three new
   loaders preserved, and the bound stated per dispatch chunk.

4. Role-composition wording narrowed. Permitting a dual SUPERUSER +
   DISSEMINATION_WORKER principal both roles' operations is stated as a
   BE-04-specific matrix decision establishing no general composition,
   aggregation or inheritance rule.

5. Invalid errorCode contract specified. Adds one bounded variant,
   ThothError::InvalidDistributionJobErrorCode ->
   INVALID_DISTRIBUTION_JOB_ERROR_CODE, with a fixed sanitized message that
   echoes no caller input, raised before any transition so no job or attempt
   state changes. Error-variant count moves from three to four.

Also clarifies the zitadel setup role-list boundary without widening scope: the
declaration is a repository source change; role creation, grant and credential
provisioning remain separately authorized operational actions.

No runtime, migration, GraphQL, role, identity-provider, deployment or
production action.
The documentation-only, path-containment and untouched-file evidence blocks
compared against the original authoring base fac86e3. Since the base
reconciliation merge 1cf5675 brought repository-authoritative develop into this
branch, that range also contains the ADR-0008 files delivered by PR #815, which
are develop's content rather than this pull request's contribution.

Restate the three checks against origin/develop with the three-dot diff, which
is what the pull request actually changes, and record why the authoring base is
the wrong baseline for them. The recorded results are re-verified and unchanged:
four files, no code, workflow, manifest or build path touched, schema.rs and
policy.rs unchanged, no migration created.
Correct the two residual consistency defects the third independent review
found in the BE-04 specification candidate. Both were created by the second
round's own corrections; no architecture is reopened and no bound weakened.

Section 9.4.3 still said section 16.3 raised the total new error count "from
two to three", which stopped being true when InvalidDistributionJobErrorCode
was added. It now states that DistributionJobCreationDisabled is exactly one
bounded new error for the creation-disabled condition and the only new error
the configuration path raises, and that it is one of the four variants section
16.3 fixes as the complete BE-04 error inventory. The other three are recorded
against the worker operations and section 18.1's errorCode contract, so the
history is not rewritten as if InvalidDistributionJobErrorCode were ever an
OFF-mode error.

Section 17.4 asserted its six-statement bound against a selection that cannot
reach it: merged BE-03 invokes BE-02's assignment loader from the
enabledDistributionPlatforms field resolver, so a job-only selection issues
five statements, not six. Two selections are now named and both are measured
at page sizes 1, 25 and 200 - full job-only (2 root + 3 job loaders = 5) and
full report, adding enabledDistributionPlatforms (2 root + 1 assignment loader
+ 3 job loaders = 6). Asserting both pins GraphQL selection-dependent
execution rather than merely counting queries. Sections 17.4, 25.1, 25.12 and
26 item 16 move together.

The three ADR-0007 loaders stay separate, configured_loader keeps its explicit
200/10, every statement stays set-based with no N+1 path, and the
per-dispatch-chunk arithmetic now covers both bounds. Nothing was combined to
reach a number.

Documentation only. No code, migration, schema.rs, policy.rs, workflow, role,
deployment or production action. The specification remains DRAFT and not
approved, and BE-04 implementation remains NOT AUTHORIZED.
@ja573
ja573 marked this pull request as ready for review August 14, 2026 17:12
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@ja573
ja573 merged commit ed32712 into develop Aug 14, 2026
10 checks passed

ja573 commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

CTO implementation authorization — BE-04

BE-04 implementation is authorized against the freshly verified exact implementation base:

develop @ ed32712766c8f5a1951bb53ec3192e18f067c7d2

The repository-authoritative specification is docs/engineering/ai-delivery/tasks/BE-04.md as merged through this PR. Implementation must use the bounded branch feature/publisher-services/be-04 targeting develop, remain within that approved specification and ADR-0008, and return for fresh independent exact-head review before any merge. A HIGH-risk implementation merge will require separate explicit CTO merge authorization bound to the reviewed exact implementation head.

This authorization covers repository implementation only. It does not authorize deployment, environment migration execution, production migration execution or rollback, identity-provider changes, role grants or credential provisioning, worker deployment, THOTH_DISTRIBUTION_JOB_CREATION activation (OFF -> ON), a pilot, dissemination, external platform calls, workflow changes/dispatch, production access, or any action on PR #799. Those remain separately gated.

Authorized by CTO on 2026-08-14.

ja573 added a commit that referenced this pull request Aug 15, 2026
Three findings were raised against the addendum's first revision. All three are
corrected here; the three substantive addendum decisions (NULL-safe attempt-error
constraint, Route A composite loader, thoth-client gate resolution) are not
reopened.

Finding 1 - the statement arithmetic was not mathematically exact. It used one
page-global boolean, `2 + (3 if J else 1) * C_job + C_assign`, but cost is
decided per dispatch chunk: a multi-chunk page can contain a chunk that resolves
jobs (3 statements) and another that resolves none (1 statement). Replaced
throughout by

    statements = 2 + 3 * C_job_nonempty + 1 * C_job_empty + 1 * C_assign

with the four named cases evaluated mechanically - job-only/report on a page
with jobs give 5 and 6, and on a page without give 3 and 4 - plus a worked
multi-chunk example and a rule that multi-chunk pages are derived from the
actual per-chunk classification. Also removes the self-contradictory
`C_job = C_assign = 1`: for the job-only selection C_assign is 0, because that
selection does not select enabledDistributionPlatforms. Loader-dispatch
acceptance is now stated per loader - each selected first-level loader has its
expected chunk count, each unselected loader has zero, and the second-level
loaders have zero report-path dispatches - rather than as a blanket claim that
every loader dispatches once.

Finding 2 - the control records conflated "addendum 01 is not approved" with
"BE-04 never had an approved specification". The live GitHub authority shows
otherwise: PR #814 is merged, its merge commit is the authorized base
ed32712, and it carries the CTO's explicit
BE-04 implementation authorization against that base, naming the merged BE-04.md
as the repository-authoritative specification. Corrected to APPROVED BASELINE /
ADDENDUM 01 NOT YET APPROVED across the specification header, sections 6.3 and
31, the new section 34.0, the tracker and the changelog. The baseline approval
and the implementation authorization are preserved as historical authority; the
implementation candidate is properly authorized work, and is blocked because the
contract it was built against is being corrected, not because it was
unauthorized. The authorization is insufficient - not void - for the corrected
contract. Clearly labelled historical DRAFT and remediation narrative is
retained, and no review or approval comment identifier is transcribed into a
committed file (ADR-0005 section 5).

Finding 3 - an unsupported inference about BE-02. The claim that an unexpected
chunk count would apply "equally" to BE-02's merged assignment loader is
withdrawn: ADR-0007 requires field-specific query-count evidence per adopting
field and establishes no universal sublinear bound for arbitrary arrival timing.
The escalation rule now requires the cause to be classified on evidence as
BE-04-specific, a shared-foundation dependency/runtime regression, or another
execution shape; only the second escalates to the owning Shared Thoth GraphQL /
Backend Architecture programme, the control that a genuine shared finding must be
surfaced rather than worked around locally is preserved, and no claim about
another field's loader may be made without verifying that field.

Documentation only. No runtime, migration, contract, workflow or manifest
change. PR #816 and PR #799 untouched.
ja573 added a commit that referenced this pull request Aug 15, 2026
Committed control documents must stay truthful before review, after review,
before merge and after merge (docs/engineering/AGENTS.md section 1.1, ADR-0005
section 6). The addendum's records carried wording whose truth would change
merely because PR #817 is reviewed, approved or merged. Removed and replaced by
durable form; no substantive addendum decision is reopened.

BE-04.md
- Status becomes "APPROVED BASELINE + ADDENDUM 01 AUTHORITY CONDITION BELOW".
- The header now states, durably: the approved baseline and its implementation
  authorization through PR #814; that this document carries the corrected
  addendum content; that approval authority is the CTO; the authority condition
  that the corrected content is repository-authoritative when the exact
  CTO-approved content is reachable from develop; and that live review, approval
  and merge evidence is the GitHub history of PR #817.
- Section 34.0 restated as baseline and addendum authority rather than as a
  current approval state; sections 6.3, 31 and 34.5 follow. Section 31 no longer
  calls the implementation pull request draft, and states durably that the
  candidate was built against the baseline contract and does not satisfy the
  corrected one.
- Section 33 no longer waits for someone to fill an approval block. It records
  that approval, review, authorization and merge are GitHub-owned terminal
  evidence, and that copying them here would need an approval-state commit,
  which ADR-0005 section 4.1 item 10 prohibits. No PR #814 comment identifier is
  transcribed.

Implementation report
- "Independent reviewer: required and absent" becomes a statement of the
  requirement and of reviewer independence, with the live review decision and
  its exact-head binding left to GitHub.
- The preflight table is labelled a point-in-time observation record rather than
  a claim about current pull-request state.
- No committed statement calls addendum 01 unapproved; approval authority and
  live approval state are GitHub-owned, and the report issues no approval
  decision.

Tracker
- BE-04 status becomes
  "BLOCKED - IMPLEMENTATION CANDIDATE REQUIRES CORRECTED-CONTRACT
  RECONCILIATION", which survives addendum review and merge.
- Durable facts retained: the approved baseline through PR #814; candidate #816
  properly authorized against ed32712; the
  candidate does not satisfy the corrected contract; continuation needs the
  corrected content repository-authoritative, a fresh base and a new CTO
  implementation authorization; live satisfaction is GitHub-owned.
- PR references no longer commit draft or approval state.

CHANGELOG
- The entry describes the durable change and attributes approval and merge state
  to GitHub, so it stays true after approval and merge without another commit.

Documentation only. No runtime, migration, contract, workflow or manifest
change. PR #816 and PR #799 untouched.
ja573 added a commit that referenced this pull request Aug 15, 2026
Two residual documentation defects, plus one found by running the search
variant-aware rather than literal.

1. The implementation report described its own framing as the
   "approved-baseline / not-yet-approved-addendum distinction". That is
   transient workflow state and becomes false when the CTO approves the
   addendum. Replaced by the "approved-baseline / corrected-addendum authority
   distinction", stated durably: the baseline is CTO-approved and
   repository-authoritative through PR #814, addendum 01 is the corrected
   specification content, the CTO is its approval authority, and its live
   review, approval and merge state is GitHub-owned.

2. The PR #817 changelog entry still described the tracker as updated to
   "BLOCKED - IMPLEMENTATION CANDIDATE EXISTS - SPECIFICATION ADDENDUM
   REQUIRED", which stopped being the tracker status when the previous commit
   made it lifecycle-neutral. It now states the current durable status,
   "BLOCKED - IMPLEMENTATION CANDIDATE REQUIRES CORRECTED-CONTRACT
   RECONCILIATION". The rest of that entry is unchanged.

3. A variant-aware search (not[- ]yet[- ]approved) found a hyphenated instance
   the earlier literal-space search missed: the tracker's programme note 14
   still called the addendum "not-yet-approved". Replaced with its approval
   authority and authority condition. The report's consistency table is also
   reworded so its rows no longer contain the very tokens they report as absent,
   and it records that the search must be run variant-aware.

Documentation only. No substantive addendum decision is reopened: the NULL-safe
attempt-error CHECK and its truth-table requirements, Route A and its loader
ownership, the per-chunk statement arithmetic and the 5/6/3/4 examples,
ADR-0007's 200/10, the evidence-classified escalation rule, the BE-02
non-inference, the thoth-client gate resolution, the approved-baseline and
implementation-authorization history, and the post-addendum implementation gate
are all unchanged and verified present.

No runtime, migration, contract, workflow or manifest change. PR #816 and
PR #799 untouched.
ja573 added a commit that referenced this pull request Aug 15, 2026
Bring the BE-04 implementation report, the programme tracker and the
changelog into line with the corrected specification and with what the
reconciliation actually measured.

Implementation report:

- records both authorized episodes without one erasing the other: the
  original base `ed32712766…` with the PR #814 authorization as valid
  history, and the corrected base `8c0c54bd…` with the fresh CTO
  reconciliation authorization on PR #816 comment 5301898691;
- records the reconciliation preflight, the merge commit, the single
  conflict and how it was resolved, and the files the reconciliation
  touched;
- adds section 6.2.1: the withdrawn and corrected CHECK expressions, why
  the old one admitted an open attempt carrying error fields, the exact
  observed pg_get_constraintdef, and the eight-case truth table on both
  INSERT and UPDATE;
- records that Correction A has no schema.rs impact as a reviewed
  conclusion, because Diesel's table! macro does not encode CHECK
  constraints;
- re-runs and records the migration evidence at the reconciled head,
  including the disposable-database CLI forward/revert/re-apply cycle,
  with both timing figures explicitly not production predictions;
- replaces section 11.1 with the full twelve-case query-count matrix -
  two selections, three page sizes, job and no-job fixtures - showing
  derived equal to observed in every case and identical across five
  re-runs, and rewrites 11.2 around the composite loader's key, value,
  L1/L2/L3 shape, totality and fail-closed behaviour;
- updates the command gate to the reconciled head's figures and states
  the thoth-client executed test counts for both workspace profiles;
- removes the "report statement count diverges" limitation rather than
  rewriting it, because the corrected implementation satisfies the
  corrected contract, and records that no unresolved issue remains and
  that stop condition 23 did not fire, so nothing is claimed about
  BE-02's loader.

Tracker: BE-04 moves to the durable, lifecycle-neutral status
IMPLEMENTATION CONTROLLED BY PR #816 - RUNTIME INACTIVE. It records the
PR #814 baseline authority as history, PR #817's corrected specification
as repository-authoritative, the fresh reconciliation authority as
GitHub-owned on PR #816, and the condition that repository authority for
the implementation depends on its exact content becoming reachable from
develop. New next-action item 16 records the reconciliation. Nothing
asserts a state that a later GitHub event would falsify.

Changelog: the merged #817 entry is preserved; the BE-04 implementation
entry is reconciled in place - not duplicated - to describe the NULL-safe
attempt-error check and the single composite report loader with its
measured counts, and the statement that the implementation intentionally
carries the three-loader divergence is removed.

Two stale internal cross-references in the report are corrected.
ja573 added a commit that referenced this pull request Aug 17, 2026
Codex raised a P1 on the tracker: it described BE-04 in transient workflow
terms that merging PR #816 would immediately falsify, which is exactly what
docs/engineering/AGENTS.md section 1.1 forbids in a committed file. Merging
would have required an immediate corrective commit.

The tracker now records the durable decision instead. BE-04's status becomes
IMPLEMENTATION DELIVERED - RUNTIME INACTIVE in the summary paragraph, the
task-table Status cell and the Acceptance cell. Present-tense lifecycle
wording is removed throughout: "implementation candidate", "implementation
branch", "IMPLEMENTATION CONTROLLED BY PR #816", the claim that the
implementation is unmerged and that develop still lacks the distribution_job
relations, and "merge would authorize". Repository authority is stated as an
authority condition that attaches when the exact independently approved
content is reachable from develop, and live review, approval, CI and merge
state is left to GitHub under ADR-0005.

Historical traceability is kept: delivery through PR #816, the baseline
through PR #814, corrected Addendum 01 through PR #817, the doctrine merged
through PR #820, and the authorizations recorded on issue #821 all remain.
The wording is now true both before and after PR #816 merges.

contracts.md registered thoth-pub/baboon as a verified consumer of the Thoth
GraphQL and metadata export APIs on 2026-08-16, after this implementation
report's consumer matrix was written, so BE-04 lacked the explicit Baboon
impact assessment the cross-repository gate requires. Section 10.7 adds it as
row 8, REMAINS COMPATIBLE, with the reasoning enumerated: the GraphQL change
is additive, no existing field is removed, retyped, made stricter or given a
required argument, no existing default changes incompatibly, the export
format and export-availability semantics are untouched, the discovery fields
Baboon queries are unmodified, and DISSEMINATION_WORKER governs only the new
worker mutations. No Baboon source change or downstream task is required.

The assessment is dated honestly: it was added during this review remediation
after Baboon became a verified consumer, not during the original
implementation. Baboon was read only through this repository's own
repository-map records and was not modified.

The matrix now covers every currently verified consumer, with thoth-sphinx
kept separate as a planned future consumer rather than as compatible, and
cc-license excluded on evidence as a crate this repository consumes.

Section 1.9 records this episode: starting head baab314, incorporated
develop 6f192ea, a two-file write budget, no new files, and no runtime,
specification, GraphQL, migration or cross-repository source effect. baab314
is recorded as this episode's authorized starting state, an ordinary merge
that pre-dates it and that this agent did not create. Episode counts, the
authority table, the commit tables, the write-budget and action-authorization
matrices and the confirmations are reconciled accordingly.

The prior process-exception history is preserved unchanged: one authorized
push, two unauthorized pushes, their two unauthorized automatic staging-pr-*
publications, the CTO's acceptance, no retroactive authorization and no
cleanup requirement.

Documentation and control only. No runtime, source, test, migration,
specification, ADR, CHANGELOG, workflow or contract-map change, and no
downstream repository was touched.
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.

1 participant