docs(publisher-services): specify BE-03 protected service configuration - #808
Merged
Conversation
Add the complete bounded implementation specification for BE-03, the protected publisher service-configuration surface, together with the programme decision its phase boundary requires. The specification settles, against the merged BE-01 and BE-02 code rather than against the design document alone: - the protected PublisherServiceConfiguration type, reusing BE-02's existing PublisherDistributionPlatformAssignment rather than duplicating it; - the owner-and-superuser query and its complete deny/allow matrix, resolved through the existing ZITADEL publisher-role model. An account may hold scoped roles for several organisations, so ownership is a per-publisher role check. No new ownership table, role or authorization framework is introduced; - the superuser-only staff report: paginated, deterministically ordered with the mandatory publisher_id tie-breaker, and filterable by publisher, package and enabled platform; - the superuser-only replace mutation in exactly one transaction, with the required logical sequence and a deterministic lock order. Two design questions the design document left open are resolved explicitly. The concurrency token is a dedicated publisher.service_configuration_updated_at column with a strict per-publisher monotonicity rule. publisher.updated_at is rejected because the set_updated_at trigger moves it on any publisher metadata edit while an assignment-only configuration change would not move it at all; a computed maximum across publisher and assignment timestamps is rejected because it is not one stored token comparable in one atomic predicate. The audit source is a closed PostgreSQL enum with SUPERUSER_API and the reserved MIGRATION_BACKFILL, with the ALTER TYPE ADD VALUE consequences recorded. Actor identity is the existing ZITADEL user id stored as user_id text, exactly the convention publisher_history already uses, so no actor format is invented and no local account table is required. Two reuse constraints are specified rather than left to implementation. BE-02's enable/disable take a pool and open their own transaction on their own connection, so calling them from inside the BE-03 transaction would block on the publisher row lock the transaction itself holds; the specification requires an additive, behaviour-preserving connection-scoped refactor instead of a second lifecycle implementation. The report must reuse BE-02's existing request-local ADR-0007 assignment loader rather than creating a second equivalent loader. The BE-03/BE-04 phase boundary is surfaced as a PROPOSED programme decision rather than guessed: BE-03 owns desired configuration only and creates no job row, no placeholder job and no fabricated job status, while BE-04 later extends the same transaction boundary. Specification and control records only. No runtime, schema, migration, GraphQL, workflow or infrastructure change; no implementation; and the reserved branch feature/publisher-services/be-03 is not created.
Add the BE-03-SPEC implementation report and an explicit template-conformance map in BE-03.md. The specification uses the house structure established by BE-01 and BE-02, which is a superset of task-specification-template.md rather than a literal section-for-section copy. The map makes template completeness auditable by naming the section that satisfies each required template field. The report records the preflight, the branch sequencing rationale, the merged code actually inspected before each design decision, the two rejected concurrency-token alternatives with the evidence against them, the BE-02 pool-connection composition hazard and its additive resolution, and the one programme decision left PROPOSED for the CTO rather than guessed. Specification and control records only. No runtime, schema, migration, GraphQL or workflow change; feature/publisher-services/be-03 remains absent.
Answer four material defects found by the control-side pre-review of the BE-03-SPEC content, consistently across the specification, its implementation report, the programme decision record, the tracker and the existing changelog entry. Documentation only: no runtime code, migration, schema.rs, generated SDL, client artifact, Cargo file or workflow is touched, no implementation branch is created, and no authorization is granted or implied. Finding 1 - protected-read authorization contradiction. Section 11.1 previously allowed PUBLISHER_ADMIN or "any publisher-scoped role satisfying the check", which contradicted the prescribed PolicyContext::require_publisher_for call. That helper requires exactly Role::PublisherUser for non-superusers, and policy.rs contains no role inheritance. Adopt the least-privilege matrix: superuser allow for any publisher; PUBLISHER_USER scoped to the target publisher allow; PUBLISHER_USER scoped elsewhere, PUBLISHER_ADMIN, WORK_LIFECYCLE or CDN_WRITE without PUBLISHER_USER, no applicable role, and anonymous all deny; writes superuser-only. Commercial package visibility is not widened by an unrelated scoped capability, and any future widening is routed to an explicit CTO decision through a new stop condition rather than a looser matrix. Finding 2 - OAPEN/DOAB split-state repair was unreachable. A membership-only diff is empty for a pair that is already both-enabled but split by activation or timestamp, so BE-02's normalization would never run. The mutation now calls the connection-scoped enable primitive once per desired linked group unconditionally, and the primitive alone decides no-op versus repair, so no second linked-platform algorithm is introduced. Redefine the true no-op as the conjunction of unchanged package, equal normalized enabled membership, and every requested enabled group already fully normalized. A repair is a committed change: it bumps the configuration token and writes exactly one audit row inside the same transaction, distinguishable through configurationVersion alone, with no activation identifier added to the audit JSON. Require the primitives to return an Unchanged/Changed outcome so the caller can tell the cases apart. Finding 3 - reconcile APP-01. Keep BE-03 owning desired configuration and BE-04 owning durable job state, and add an explicit APP-01 reconciliation naming the five capabilities BE-03 alone supports and the four that require BE-04. State that the candidate refines, and in that narrow respect supersedes, the earlier wording assigning back-catalogue-status inspection to a BE-03-only dependency; APP-02 remains dependent on BE-03 and BE-04. The decision remains PROPOSED and programme-local, requiring no ADR. Finding 4 - canonical token/audit semantics could be bypassed. Fix one authoritative service-configuration write coordinator owning package, enabled-platform desired state, canonical version token and audit history in one caller-owned transaction under an explicit source/actor context, and require the GraphQL mutation to call it. BE-02's pool-level enable/disable are retained unchanged but reclassified as lower-level domain/compatibility functions barred from new production configuration call sites, with required call-site enumeration and bypass-search evidence; they have zero production call sites today. For the MIG-01 seam, keep SUPERUSER_API and MIGRATION_BACKFILL and redefine the audit actor column as actor text NOT NULL with a non-blank check constraint and source-scoped provenance, so an authenticated superuser id and a controlled non-secret migration control identity both satisfy it without inventing credentials; MIG-01 is bound to the same coordinator, token update and audit invariants. The rejected alternative, deferring MIGRATION_BACKFILL, is recorded with its reasons. BE-03 implementation remains NOT AUTHORIZED, feature/publisher-services/be-03 remains absent, and the BE-03/BE-04/APP-01 phase boundary remains PROPOSED pending an explicit CTO decision.
…dings Answer the fresh independent specification review of PR #808 at exact head 5dc2b1d (CHANGES REQUIRED; one P1, six P2, no BLOCKED-level finding) in one bounded documentation pass. No already-settled architecture is reopened. P1 - reconcile the protected surface with ADR-0001 section 4.4 by exposing effective capability codes. Add `effectiveCapabilities: [PublisherCapability!]!` to `PublisherServiceConfiguration`, derived on read from the canonical subscription package through BE-01's merged `ThothPackage::capabilities()` in that mapping's canonical order, persisted nowhere, not an input, not in the audit JSON, and protected by the same single section 11.1 read decision. `PublisherCapability` joins `ThothPackage` as an enum becoming SDL-reachable. Recorded explicitly as reconciliation with approved ADR-0001/BE-01 architecture, not a new decision and not another deferral. P2 - record that storing the token on `publisher` under the existing `set_updated_at` trigger also moves `Publisher.updatedAt` on every committed change, including platform-only changes and linked-state repairs, while a stale request and a true no-op move neither timestamp; that `Publisher.updatedAt` is not the configuration concurrency token; and that the new field may appear as an additional key in future `publisher_history.data` snapshots. Keep the generic publisher history and the configuration audit distinct. P2 - require the connection-scoped enable primitive to check `is_assignable()` itself before any write, alongside the retained pool-level early check and the coordinator's whole-set pre-validation, with a direct `enable_on(JISC_NBK)` regression. P2 - align the staff report's list arguments with the merged `Option<Vec<T>>`/`[T!] = []` convention. P2 - correct the write-bypass search scope to the crates this repository actually contains; `thoth-app` is a separate repository. P2 - remove transient pull-request and CI lifecycle claims from the committed report under ADR-0005, and fill the empty observations heading. P2 - replace the self-blocking `PROPOSED - AWAITING CTO DECISION` status and its literal-`APPROVED` stop condition with a durable authority condition stated identically in decisions.md and BE-03.md. Documentation and control records only: no runtime code, migration, schema.rs, generated SDL, client artifact, Cargo file or workflow is touched. BE-03 implementation remains NOT AUTHORIZED, `feature/publisher-services/be-03` remains absent, and the programme decision remains a candidate subject to CTO specification approval of the exact reviewed content.
Answer the second fresh independent specification review of PR #808 at exact head a1acd3f (CHANGES REQUIRED; one P1, one P2) in one bounded documentation pass. That review independently confirmed all seven findings of the previous review RESOLVED and re-confirmed the settled architecture - protected effective capabilities, PUBLISHER_USER/SUPERUSER authorization, the canonical write coordinator, the connection-scoped BE-02 lifecycle composition, OAPEN/DOAB normalization and repair reachability, true-no-op semantics, optimistic concurrency, the audit/source model, the MIG-01 seam, the BE-03/BE-04 ownership boundary, the APP-01/APP-02 reconciliation, the migration shape, the GraphQL protected exposure, the DataLoader approach and rollout/rollback. None of that is reopened here. P1 - a second existing trigger on public.publisher was unaccounted for. Verified in thoth-api/migrations/20250000_v1.0.0/up.sql: alongside set_updated_at (BEFORE UPDATE, diesel_set_updated_at()), publisher carries set_work_updated_at_with_relations (AFTER UPDATE, publisher_work_updated_at_with_relations()), which on NEW IS DISTINCT FROM OLD executes one set-based UPDATE work SET updated_at_with_relations = current_timestamp FROM imprint WHERE work.imprint_id = imprint.imprint_id AND imprint.publisher_id = NEW.publisher_id. A committed configuration operation that UPDATEs the publisher row therefore has three observable persisted consequences, not one, and work.updated_at_with_relations is a public downstream freshness signal - resolved on the public Work type, a filter argument on six anonymous queries, orderable, selected by the generated WorkLastUpdatedQuery / WorksLastUpdatedQuery, and the value thoth-export-server's MetadataRecord compares against its Redis cache timestamp. Record both triggers in section 2.1 item 8. Rewrite section 6.4 to state all three consequences, that a platform-only change and an OAPEN/DOAB linked-state repair newly cause the cascade relative to merged BE-02 (which touches only publisher_distribution_platform and does not UPDATE publisher), that other publishers' works are never touched, that stale, true-no-op and rolled-back cases fire neither trigger, and that the public signal reveals refreshed catalogue state without directly exposing package, capabilities, enabled platforms, actor, source or audit content - while acknowledging that timing correlation remains possible. Extend the section 18.4 evidence to a three-value six-case table proved against a real disposable PostgreSQL database with at least two imprints, at least two target works and a control work of another publisher, and add a catalogue-scale write-amplification and lock-footprint measurement recorded without production extrapolation. Correct sections 7.3 step 9 and 7.8, whose "touches exactly one publisher", "no multi-object ordering" and 17-value-bound claims now apply only to BE-03's explicit application-level ordering and statement count: the transaction writes one publisher row plus bounded configuration/audit rows plus N related work rows through one set-based trigger statement, never an application loop, so row-lock footprint and transaction duration grow with catalogue size while the publisher FOR UPDATE lock is held. No production duration and no safe catalogue size is invented. Keep the canonical token at publisher.service_configuration_updated_at and introduce no second configuration/version table. Section 6.3 now states the separate-table alternative's genuine benefit honestly - it would avoid both triggers for platform-only or version-only persistence - and records that it would not avoid the same cascade for canonical package changes while BE-01's subscription_package remains on publisher, and that eliminating the cascade fully would require wider changes to approved BE-01 storage or shared publisher trigger semantics that this finding does not justify. Acceptance is therefore evidence-conditional: section 7.9 states that BE-04 inherits the cascade with the transaction boundary, section 17.1 no longer permits a "no observable downstream data change" reading while making clear the cascade is not distribution activation, and new stop condition 19 requires BLOCKED and a separate architecture escalation - never a silent trigger, storage or token change - if the measurements are bad. The former stop condition 19 is renumbered 20. P2 - correct the inaccurate write-bypass search count. The conclusion holds: zero production call sites of BE-02's pool-level enable/disable. The report now quotes the exact command, its exact per-file output (14 in graphql/distribution_platform_tests.rs, 44 in model/publisher_distribution_platform/tests.rs, 58 total), records zero non-test production hits, states that all matches are confined to those two cfg(test)-gated files, and drops the approximate count. It is framed as authoring-time evidence that does not waive section 18.9's requirement for the future implementation to rerun the search at its own head. Documentation and control records only: no runtime code, migration, schema.rs, generated SDL, client artifact, Cargo file or workflow is touched. BE-03 implementation remains NOT AUTHORIZED, feature/publisher-services/be-03 remains absent, and the programme decision remains a candidate subject to CTO specification approval of the exact reviewed content.
Member
Author
|
CTO control record for BE-03-SPEC at exact head Independent review: a fresh independent exact-head specification review returned APPROVED with no findings for this exact head. The review was read-only and was not submitted as a GitHub review under the CTO/user identity. CTO approval and merge authorization (2026-08-12): the CTO explicitly approves this exact BE-03-SPEC content as the authoritative implementation specification for BE-03 and approves the contained Publisher Services programme decision establishing the BE-03/BE-04/APP-01/APP-02 boundary, subject to this exact approved content becoming reachable from Authorization boundary: specification/control repository integration only. This does not authorize BE-03 implementation, creation of the BE-03 implementation branch, deployment, environment or production migration execution, MIG-01, backfill, assignment creation, distribution activation, OBSERVE/ENFORCE, production access, BE-04 implementation, or action on PR #799. Any head change before merge invalidates this exact-head approval and merge authorization. |
ja573
marked this pull request as ready for review
August 12, 2026 19:53
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Aug 12, 2026
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
1 participant
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.
Task identity
This PR is a specification and control-record change only. It does not authorize or implement BE-03 runtime behaviour.
Scope
Creates the bounded implementation specification for BE-03 protected publisher service configuration and records the associated programme decision candidate. The specification defines:
PUBLISHER_USERpolicy model;Proposed programme decision
docs/publisher-services/decisions.mdrecords a phase-boundary decision candidate,PROPOSED IN THIS SPECIFICATION CANDIDATE, under a durable ADR-0005 authority condition rather than a mutable status word:This explicitly refines the narrow earlier APP-01 wording that implied back-catalogue job status was available from BE-03 alone.
Remediation already incorporated
The exact head includes three ordinary remediation commits, each on top of the then-current head, with no amend, rebase, squash or force-push at any point.
One bounded commit addressing four control-side pre-review findings:
PUBLISHER_USERscoped to the target publisher (orSUPERUSER); unrelated publisher-scoped roles do not inherit access;One bounded commit answering the first fresh independent specification review of head
5dc2b1dd(one P1, six P2):effectiveCapabilitiesderived fromThothPackage::capabilities(), reconciling BE-03 with ADR-0001 section 4.4;publisher.updated_atconsequence of the token's location is recorded explicitly;[T!] = []convention;One bounded commit answering the second fresh independent specification review of head
a1acd3f6(one P1, one P2), which independently confirmed all seven findings above resolved and re-confirmed the settled architecture:public.publishercarries a second materialUPDATEtrigger,set_work_updated_at_with_relations, so a committed configuration change also refresheswork.updated_at_with_relationsacross that publisher's catalogue through one set-based statement. Both triggers are now recorded; the three-value observable semantics, the newly-caused cascade for platform-only changes and linked-state repairs relative to merged BE-02, the real transaction write footprint (one publisher row + bounded configuration/audit rows + N related work rows) and the catalogue-size lock/duration growth replace the earlier single-publisher-row framing. The canonical token stays onpublisher; the separate-table alternative's genuine benefit and its failure to avoid the same cascade for canonical package changes are both stated. Six-case database evidence with target and control works, plus a catalogue-scale write-amplification measurement recorded without production extrapolation, are required, and a new stop condition mandates escalation rather than any silent change to shared publisher trigger or storage semantics if that evidence is bad;#[cfg(test)]-gated files — replacing an inaccurate approximate count, and framed as authoring-time evidence that does not waive the future implementation's own search requirement.Audit / migration seam
The candidate keeps
SUPERUSER_APIandMIGRATION_BACKFILLsource values. The proposed audit actor is a non-blank text identity with source-scoped provenance: authenticated ZITADEL user ID for superuser API changes and a stable non-secret control identity for a future separately authorized MIG-01 path. MIG-01 remains unimplemented and must use the same configuration persistence invariants while creating no job or dissemination side effect.Branch history
The branch was originally created from BE-02-CLOSEOUT-01 head
1f2cb585b25336ab9806adaeff9538b1ac3fa8ea. That closeout is now merged intodevelopas7aeb715f9815a41b6357d8b6a7037ac62ebb25bb. The current merge base remains the closeout head; no rebase, merge-from-develop, squash or history rewrite was performed.The PR diff against current
developis exactly five documentation/control files:CHANGELOG.mddocs/engineering/ai-delivery/tasks/BE-03.mddocs/engineering/ai-delivery/implementation-reports/BE-03-SPEC-implementation-report.mddocs/publisher-services/decisions.mddocs/publisher-services/task-status.mdValidation
Authoring evidence records:
CI triggered normally by this draft PR is repository-controlled; no workflow is manually dispatched by this task.
Authorization boundary
This PR requires fresh independent exact-head specification review. The authoring/remediation agent has not approved it. Specification approval and merge are separate CTO-controlled gates; implementation requires a later fresh-base authorization after the approved specification is repository-authoritative on
develop.