From ae6aecd89e8b097b792a44154799470752d79a6a Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Sun, 13 Sep 2026 00:10:58 +0200 Subject: [PATCH 01/41] Plan Factory M3 work items, labels and gates Define repository ownership, work-item durability, command authorization and policy boundaries for issue #114. Order runnable implementation slices and specify acceptance tests with mutation checks. Record open scope decisions for review before implementation. --- .../plans/2026-09-12-factory-m3-work-items.md | 543 ++++++++++++++++ ...2026-09-12-factory-m3-work-items-design.md | 604 ++++++++++++++++++ 2 files changed, 1147 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-12-factory-m3-work-items.md create mode 100644 docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md diff --git a/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md new file mode 100644 index 00000000..b14cd17d --- /dev/null +++ b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md @@ -0,0 +1,543 @@ +# Factory M3 — work items, labels and gates — implementation plan + +**Date:** 2026-09-12 + +**Status:** Round 1, plan only. All implementation checkboxes and test outcomes below are pending. + +**Goal:** Start factory work from a tracker ticket with explicit, bounded autonomy; make repository +ownership, command authority and approval state visible and durable. + +**Issue:** [#114](https://github.com/artyomsv/code-spire/issues/114), fetched after its +`2026-09-12T21:47:45Z` update. + +**Design:** [Factory M3 design](../specs/2026-09-12-factory-m3-work-items-design.md). + +**Architecture:** An event-sourced work-item lifecycle owns workflow decisions. Existing run +records and the charge ledger retain execution truth; no run aggregate or transcript replay is +introduced. Explicit repository-role bindings replace account-workspace lookup. The gateway owns +keyed, authenticated webhook registrations. Work-source adapters reuse context transport with a +separate write facade. Current policy is checked at each boundary before durable effects leave +the orchestrator. The design's §2 explains the aggregate decision and its ADR-034 amendment. + +**Stack:** Java 25 / Quarkus / JDBC and Flyway; PostgreSQL + Kafka; React + TypeScript, vitest and +Testing Library; existing Gradle split test tiers. Keep the versions already pinned by the repo. + +**Branch:** `feat/factory-m3-work-items`, already checked out in +`E:\Projects\Stukans\code-spire-worktrees\feat-software-factory`, based on `origin/master` at +`27fe17b`. Do not create or switch a branch. The analyst reviews this same worktree. + +## Global constraints + +- Round 1 changes exactly the design and this plan. **Slice 0 opens the draft PR**, before any + production slice. Opening a draft is explicitly authorized by the brief; no extra permission + round is needed. The draft stays draft for analyst review. +- Commit each independently reviewable slice with imperative first line, maximum 72 characters, + and a body explaining nontrivial changes. No authoring attribution, coauthor trailers, model + names, vendor names or generated-by notices in commit/PR text. Requested PR title takes + precedence over generic commit-style templates. +- The running `spire-dev` stack and `spire-run-worker` Gradle process on `:34083` are shared. + Do not stop them, run compose down or kill their processes. Service tests use their own + Testcontainers resources. If a Docker test cannot coexist with the dev worker, coordinate + a safe window with the analyst; do not stop the worker unilaterally. +- **Never run concurrent Gradle test invocations in this worktree.** Run `./gradlew testFast` + then `./gradlew testServices`, sequentially, using `--rerun-tasks` for measured evidence. + On PowerShell use `.\gradlew.bat`. Targeted runs use `--tests` plus `--rerun-tasks`. +- Mutation checks use a scratch snapshot of the exact pre-mutation file, never a git restore. + A mutation must change a production line, compile, and kill exactly one designated test in + the selected run. Zero or more than one failure is not the required proof. Details below. +- Pure domain code in `spire-contract` and `spire-diff` stays framework-free. Extend module + purity/architecture checks for the new SPI; provider dispatch belongs only in ADR-020 + composition roots. No forge-specific branches in policy, saga or resource code. +- Preserve existing line endings. Java uses four-space indentation, TS two. Split new React + screens/components rather than growing the already-large settings components. Match existing + validation, icons, auth, encryption and CSS contract conventions. +- No plausible synthetic rows. Isolated test fixtures use `TEST-`/`CANARY-` names. A live canary + requires announcing its actual ids and exact cleanup `DELETE` before insertion; track remote + issue/branch cleanup too. This round creates no data. Do not include generic destructive SQL + against user-owned rows in a runbook and call it cleanup. +- Temporary files belong in this session's scratchpad: + `C:\Users\artjo\AppData\Local\Temp\claude\E--Projects-Stukans-code-spire-worktrees-feat-software-factory\5f317e7d-1b64-4305-bf1c-e3a370b07eb1\scratchpad`. +- Unknown capabilities, missing external evidence and test skips are visible outcomes. No stub + phase reports success in production. Proposed tests below are not evidence until executed. + +## Slice order and runnable exits + +Each slice includes its own API/read surface, tests and applicable documentation. A migration-only +or interface-only commit can exist inside a slice, but is not its review exit. + +| Slice | Depends on | Runnable exit | Review unit | +|---|---|---|---| +| 0 — Design and plan | None | These documents render, link to the updated issue, and are in an open draft PR. | **Opens the PR. Round 1 ends here.** | +| 1 — Repository registration and migration bridge | 0 reviewed | Register/read a repository with explicit accounts; existing reviews and runs still resolve identically during bridge. | Schema, bootstrap exchange, repository API and first detail view. | +| 2 — Repository cutover and per-kind webhooks | 1 | Repository screen is the entry point; workspace is absent from account form and runtime lookup; existing hook keys still verify. | All active resolvers, gateway kind routing and UI together. | +| 3 — Resolve people and edit bidirectional overrides | 2 | Enter a resolvable handle, reload its id-backed display, reject unresolved input; edit allow/deny on repository. | Directory adapters, registry/API and account/repository person controls. | +| 4 — Authorize `/fix` by effective push permission | 3 | A real inbound command reaches dispatch on measured write access with empty overrides; all four override cases work. | Permission adapters and both saga authorization layers. | +| 5 — First ticket, durable intake and suggest policy | 2, 3 | A signed issue label or rescan admits one durable item, visible on Work items; unknown/unlisted actors select nothing. | GitHub source, minimal profile registry, lifecycle/store/outbox and UI. | +| 6 — GitLab/Jira sources and recovery | 5 | Each source can admit a real fetched ticket through webhook or polling with the same actor rule. | Adapter parity, safe tracker writes and restart-safe scanning. | +| 7 — Full policy and dashboard approvals | 5 | Label changes and ceiling edits affect the next phase; a dashboard gate survives restart, resolves or expires. | Complete profile vector, transition checks, gates, Approvals and attention. | +| 8 — Prepared task to policy-controlled build/delivery | 4, 6, 7; design §11.1 resolved | Three labelled prepared tasks stop, await approval or build; missing later capabilities stay visibly waiting. | Artifact handoff, dispatch/result join and draft/regular delivery support. | +| 9 — External answers and human takeover | 8 | Tracker/PR answers resolve the same gate; human activity suspends automation and holds publication through restart. | Authenticated ingress, gate channel adapters, run control/publisher hold and resume. | +| 10 — Integrated evidence and release documentation | 1–9 | Acceptance proofs and mutation evidence are recorded; supported journeys demonstrated and remaining limitations named. | Final integration tests, runbook and measured status updates. | + +Slices are sequential review boundaries, not a request to launch parallel test runs or delegated +work. Some dependencies are independent for scheduling, but this plan requires no additional pane. + +## File map and contracts + +Abbreviations used only in the task file lists: + +- `C` = `spire-contract/src/main/java/dev/codespire/contract/`. +- `O` = `spire-orchestrator/src/main/java/dev/codespire/orchestrator/`. +- `G` = `spire-gateway/src/main/java/dev/codespire/gateway/`. +- `U` = `spire-ui/src/`. +- Java tests use the corresponding `src/test/java` package. Every new test class in this plan + lives there unless the `spire-e2e` module is named explicitly. + +| Surface | Create / modify | +|---|---| +| Repository | Create `O/repository/RepositoryRegistry.java`, `RepositoryAccounts.java`, `RepositoryResource.java`, `RepositoryMigrationBridge.java`; migrations in orchestrator `src/main/resources/db/migration/`. Modify `O/provider/ProviderRegistry.java`, `ProviderResource.java`, `ProviderInput.java`, `ProviderView.java`, `ScmProvider.java`, `ProviderClients.java`. | +| Active account consumers | Modify `O/provider/ReviewProviderResolver.java`; `O/pipeline/IntegrationSaga.java`, `ReviewRerunService.java`; `O/ingress/ManualRegisterResource.java`; `O/prompt/PromptSampleRenderer.java`; `O/factory/MachineAccounts.java`, `RunResource.java`, `FixRunDispatcher.java`, `FactoryPullRequests.java`, relevant credential assemblers and non-secret serving views. Confirm exact call sites by search before edits. | +| Gateway | Modify `G/RegistryWebhookEdge.java`, `WebhookProviders.java`, `WebhookCommands.java`, `registry/WebhookRepo*.java`; create registry snapshot outbox/publisher and work integration publisher; gateway migrations/configuration; all three SCM resource routes. | +| Identity/permission | Create `C/port/ActorDirectory.java`, `RepositoryPermissionSource.java`; `C/scm/ResolvedActor.java`, `RepositoryPermission.java`; `O/provider/ActorResolutionResource.java`; `O/factory/FixAuthorization.java`, `FixPermissionService.java`; implementations in existing `spire-scm-{github,gitlab,bitbucket}` packages. | +| Work-source SPI | New `spire-worksource/src/main/java/dev/codespire/worksource/WorkSource.java`, `WorkItemRef.java`, `LabelEvent.java`, capabilities/paging/actor records. Three `spire-worksource-{github,gitlab,jira}` adapter modules; reuse/refactor existing issue clients and `spire-http/PinnedJsonClient` transport. | +| Work-item lifecycle | Create `C/lifecycle/WorkItemLifecycle.java`, work-item state/command/value types; extend `C/event/DomainEvent.java`; create work integration/command wire hierarchies and `WorkItemIds`. Update envelope decoding, `O/pipeline/DomainEventSink.java`, `O/eventstore/JdbcEventStore.java`. | +| Orchestration | Create `O/workitem/WorkItemStore.java`, `WorkItemSaga.java`, `WorkItemTransitions.java`, `WorkItemProjection.java`, `WorkItemResource.java`, `WorkItemOutbox.java`, `WorkItemDispatcher.java`; `O/worksource/WorkSourceRegistry.java`, `WorkSourceClients.java`, `WorkSourceScanner.java`, resource and reconciliation classes. | +| Policy/approval | Create `O/autonomy/AutonomyRegistry.java`, `AutonomyResource.java`; pure policy values/resolver in contract lifecycle package; `O/workitem/GateExpiry.java`, `GateResource.java`, `GateAnswerRouter.java`, `HumanTakeover.java`; expand attention queries. | +| Run bridge | Modify `O/factory/RunResultSaga.java`, `FactoryRunProjection.java`, `FactoryPullRequests.java`, run dispatch assembly; contract run records/control; `spire-run-worker/.../RunControlListener.java`, `RunLauncher.java`, `OrphanWatchdog.java`; publication-hold handling in runtime/publisher. | +| UI | Create focused `U/components/repositories/`, `workItems/`, `approvals/`, `autonomy/` components and API modules. Modify `App.tsx`, `api.ts`, `ProviderFormModal.tsx`, `AccountCredentialFields.tsx`, `ReviewerFieldsSection.tsx`, `AccountsCells.tsx`, `SettingsWebhookRepos.tsx`, serving hooks and `AttentionBell.tsx`. | +| Build/docs | `settings.gradle.kts`, module builds, root test tiers, `spire-arch` tests, contract snapshots, Kafka provisioning, service `application.yml`, both packaged Compose variants and Helm/kustomize resources where channels/config require them; `docs/{DECISIONS,CONTRACT,DATA-MODEL,SCM-MAPPING,SECURITY,SMOKE-TEST,HISTORY,UNVERIFIED}.md`, factory docs and `CLAUDE.md`. | + +New filenames and method names are proposed contracts. Check repository state at the start of each +slice; do not copy obsolete line numbers from historical plans. If a composition root moves, update +the plan and its architecture allowlist together. + +## Slice 0 — publish this planning round + +**Files:** only this document and its linked design. + +- [ ] Confirm branch and clean initial worktree, read the complete brief, re-fetch issue #114 and + inspect ADR-041, factory requirements and the actual run/account/webhook implementation. +- [ ] Write the aggregate decision and why; repository migration; policy/identity/gate rules; + ordered runnable slices; exact proof and mutation obligations for criteria 1–7. +- [ ] Record under-specification in design §11 rather than choosing silent fallback behavior. +- [ ] Check Markdown links, `git diff --check`, exact two-file scope and absence of secrets/data. + No Gradle/UI suites are warranted for this documentation-only change. +- [ ] Commit `Plan Factory M3 work items, labels and gates`, with a body explaining the new + registry and workflow decisions and the acceptance-proof plan. Push with + `git push -u origin feat/factory-m3-work-items`. +- [ ] Write the PR body as a real UTF-8 scratchpad file and use `gh pr create --draft --base master + --head feat/factory-m3-work-items --title "Factory M3 — work items, labels and gates" + --body-file `. Link issue #114 without claiming to close implementation work. +- [ ] Verify the remote head equals the commit, PR is draft against master, and only these two + documents are in its diff. Report the PR number and the design questions. **Stop Round 1.** + +## Slice 1 — register a repository while preserving existing resolution + +**Files:** repository registry/bridge/resource and migration files in the map; gateway snapshot +outbox; first repository UI; ADR-042 draft in `docs/DECISIONS.md`. + +**Produces:** `RepositoryAccounts.resolve(repositoryId, role)` and a non-secret serving view; +`POST/GET /api/repositories`; versioned metadata-only registration snapshots. + +- [ ] Settle the org-migration choice in design §11.4. Write failing migration/service tests: + `RepositorySchemaMigrationTest.preservesAccountIdsCredentialsAndContextReferences`, + `RepositoryMigrationBridgeTest.replaysGatewaySnapshotWithoutDuplicateBindings`, + `RepositoryMigrationBridgeTest.leavesConflictingOriginsPending`, and + `RepositoryResourceTest.registersARepositoryWithExplicitRoleBindings`. +- [ ] Add repository and binding tables, revision checks, referenced-delete protection and + migration snapshot storage. Preserve UUID/AAD and V59 source recovery. Do not relax the old + account key before the bridge can preserve assignments. +- [ ] Publish/consume real gateway metadata with stable snapshot revision and outbox retries. + Provision `cs.registry-integration`, keyed by registration id. Do not read gateway SQL from the + orchestrator or send its webhook secret across this channel. +- [ ] Add repository registration/detail UI backed by the API, including empty/disabled/pending + roles. Show workspace on the repository. During this slice the old account field is explicitly + labelled legacy; it is removed at slice 2's cutover. +- [ ] Prove old review/run resolution equals new bindings for migrated fixtures, across restart, + multiple roles, hosts and nested namespaces. Duplicate source snapshots must not recreate + an account an operator has already rebound. +- [ ] Mutation: omit factory-role filtering in the binding resolver; run only + `RepositoryAccountsTest.reviewerNeverReceivesTheFactoryCredential`. Expect one assertion failure + with distinct `TEST-` credentials, restore snapshot, rerun green. Also kill the origin-match + guard with `rejectsAnAccountFromAnotherOrigin` and migration AAD/id preservation with the + migration test above, each as a separate mutation. +- [ ] Run relevant service/UI tests sequentially, demonstrate registration through the real API + in the isolated test stack, update ADR-042/upgrade notes, commit the slice for review. + +## Slice 2 — cut over to repository ownership and per-kind hooks + +**Files:** all active account consumers, gateway registry/edge/resources, account DTOs/forms, +repository UI and migrations; ADR-042 final decision text. + +**Produces:** runtime resolution solely by explicit repository binding; no account workspace in +new API/form; gateway key plus scope plus event-kind validation. + +- [ ] Write failing `RepositoryResolverCutoverTest.allDispatchPathsUseTheSelectedRepository`, + `RepositoryResolverCutoverTest.unmappedLegacyReviewCannotDispatch`, + `RepositoryWebhookKindsTest.preservesLegacyKeyAndRejectsWrongKind`, + `RepositoryWebhookKindsTest.refusesASecondWebhookForTheSameKind`, and criterion 7 tests below. +- [ ] Change every resolver caller; search `resolveByWorkspace`, `registration`, + `providers.resolve`, `MachineAccounts.resolve` and serving API usages. Include manual/rerun, + prompt, fix and result-time PR proposal paths, not only the HTTP run endpoint. +- [ ] Drop the old UNIQUE and workspace-by-role CHECK; finish migration mappings, remove active + account workspace access and then its column per the accepted bridge schedule. Retain scalar + role checks. Refuse origin/type edits on referenced accounts. Do not add global role uniqueness. +- [ ] Upgrade all three keyed SCM edges to product event-kind filtering. Preserve keys, secrets, + scope and rejection history during migration. Wire FACTORY activity separately from REVIEWER + commands; reserve ISSUE scope for source registration. Unknown kinds fail closed. +- [ ] Replace the webhook-row screen with repository detail and one hook control per kind. Support + registration without hooks, retries after partial save and legacy org/deep-link navigation. + Remove workspace from `ProviderInput`/`View` and `AccountCredentialFields`, not merely hide CSS. +- [ ] Mutation checks for criterion 7 are specified in the matrix. Additionally kill the scope + comparison with `RepositoryWebhookKindsTest.validSignatureCannotCrossRepositoryScope`; use + the same provider and a valid signature so a different guard cannot mask the mutation. +- [ ] Run migration, gateway, orchestrator and UI verification in sequence. Search for remaining + active workspace-only lookups; historical docs/bridge mappings are the only permitted matches. + Update serving API/upgrade contracts and commit the runnable cutover. + +## Slice 3 — resolve a person with the selected account + +**Files:** directory SPI/adapters, actor-resolution resource, account policy storage, repository +override registry, person controls; ADR-044 identity half. + +**Produces:** exact identity resolution with typed errors; stable-id persisted allowlists and +`ALLOW|DENY` repository fix overrides with readable display metadata. + +- [ ] Settle capability wording for Bitbucket/Jira person lookup before claiming universal handles. + Write criterion 6 tests and `ActorResolutionResourceTest.usesOnlyTheSelectedAccountsCredential`, + `ActorResolutionResourceTest.refusesAnAmbiguousMatch`, + `ActorResolutionResourceTest.rechecksSubmittedIdentityOnSave`, + `ActorDisplayTest.renamedHandleKeepsTheStoredId`. +- [ ] Implement directory adapters via configured account origin/auth. Exact match and stable id + are required; no username-to-id string coercion and no first search result. Implement by-id + refresh with stale display metadata on failure; never send a secret in a response. +- [ ] Add the repository override table and UI controls. One actor has one override; a contradictory + edit is a version conflict, not “last array entry wins.” Migrate verified legacy stable-id grants + to their real repository mappings. Flag unresolved legacy handles without granting authority. +- [ ] Add account and source person pickers with unresolved/error states. Show handle plus policy + effect; a count may accompany people but cannot replace their identities. +- [ ] Kill criterion 6 mutations, then separately kill account credential selection and returned-id + verification with the corresponding targeted tests. Restore and rerun each case green. +- [ ] Run adapter unit tests, resource persistence test and UI form round-trip; update SCM-MAPPING + identity capability notes and commit. At this exit overrides are editable; slice 4 activates + their new permission fallback without changing unrelated review policy. + +## Slice 4 — measure repository push access for `/fix` + +**Files:** permission SPI/adapters, `FixAuthorization`, `FixPermissionService`, both guards in +`IntegrationSaga`; authorization tests and ADR-044. + +**Produces:** explicit deny → grant → fresh effective push permission, still subject to existing +target, identity, observe-mode, spending and fix-chain guards. + +- [ ] Write the six distinct criterion 5 cases, plus inherited-rights/unknown-response adapter + tests. Read the official endpoint contracts linked by design §4.3 before writing fixtures. + Test Bitbucket's effective endpoint and pagination, not its explicit-grant endpoint. +- [ ] Implement adapters returning `CAN_PUSH|CANNOT_PUSH|UNKNOWN`, binding repository and stable + user. Enforce timeout/rate limits and origin-pinned requests; do not fall back to another token + when the assigned reviewer cannot inspect permission. No stale positive permission cache. +- [ ] Route `/fix` around the old common author-list guard into `FixAuthorization`. Keep self-loop, + observe-only and registration/target checks. Do not change `/review` or `/finding` semantics. +- [ ] Test through `IntegrationSaga.on` using the actual normalized command so a private + `FixAuthorization` unit test cannot conceal the outer guard. Assert dispatch count and the + refusal reason, with legitimate thread/finding/account prerequisites in every permission case. +- [ ] Prove override grant still cannot push a fork/trunk or exceed either FR-F32 cap. Retain + corresponding M2 regression suites; live-author permissions never substitute for push target + validation. Known stale PR-state/shared-branch debt stays documented unless explicitly fixed. +- [ ] Kill each criterion 5 mutation in its isolated method, then run the class green. Exercise all + three adapter contracts and inherited-access cases; record live-token limitations without + changing the operator's account privileges. Commit the authorization slice. + +## Slice 5 — admit the first ticket and display durable bookkeeping + +**Files:** `spire-worksource` and GitHub arm, source registry/scanner, minimum profile registry, +work-item lifecycle/store/saga/outbox/resource, wire/config changes, Work items screen; ADR-043. + +**Produces:** authenticated label intake and explicit rescan; attributable label reconciliation; +one durable suggest item without a run or mirrored issue content. + +- [ ] Write `WorkItemIntakeIT.signedLabelCreatesOneVisibleItemAcrossRedelivery`, + `WorkItemStoreTest.restartRehydratesOnlyWorkflowMilestones`, + `WorkItemStoreTest.rollbackLeavesNoGateEventOrOutboxEffect`, + `WorkItemProjectionTest.containsNoTrackerContentColumns`, and criterion 3 tests. +- [ ] Add the SPI/module dependencies and build purity/licensing checks. Reuse the issue client's + HTTP/auth implementation through a read facade and a separate work writer; no write methods on + a context-provider interface. GitHub candidates/fetch/label audit use bounded pagination. +- [ ] Add source registration with explicit account and repository, typed actor allowlist and + health/cursor state. Add minimum versioned profile/mapping/ceiling registry necessary for a + real suggest admission; do not embed profile behavior in a forge adapter. +- [ ] Implement work-item ids/generations, lifecycle decide/fold, typed event decoding and dedicated + work topics. Make JDBC event append, projection, dedupe and outbox one real transaction. + Route work events away from review history; test `WorkItemEventRoutingTest.neverWritesAReviewRow`. +- [ ] Extend the keyed gateway edge for a bound issue scope; signed delivery and scanner events + enter the same reconciliation path. Store control facts only. Start polling with conservative + unknown attribution when full history cannot be proven. +- [ ] Render a paginated Work items screen/detail from persisted workflow fields, ignored-label + reasons and a live tracker link; show tracker fetch errors separately from workflow state. +- [ ] Kill criterion 3 mutations, event-route isolation and rollback guards individually. For + rollback mutate the shared-transaction use and inject a failure after event append but before + projection/outbox completion; a compile failure is not a valid transaction test. +- [ ] Run SPI/adapter tests and service intake/restart/UI proofs; commit the first ticket slice. + +## Slice 6 — source parity, safe writes and downtime recovery + +**Files:** GitLab/Jira arms, shared provider transport, source clients/scanner, tracker ingress, +source settings UI, source capability docs. + +**Produces:** same source contract for three trackers, resumable polling and idempotent comment/ +transition effects, with unsupported audit/approval channels visible. + +- [ ] Write `GitLabWorkSourceTest.reconstructsCurrentLabelApplierAcrossPages`, + `JiraWorkSourceTest.attributesOnlyTheActualAddedLabel`, + `WorkSourceRecoveryIT.backfillWithoutAuditRemainsUnattributed`, + `WorkSourceRecoveryIT.removeThenReaddCannotReuseAnOldAllowedActor`, + `WorkSourceRecoveryIT.restartResumesAfterCommittedCursor`. +- [ ] Implement candidate/read/comment/transition/label-event operations and supported capability + reports for each adapter. Validate real Jira transition ids rather than treating arbitrary + status names as commands. Respect origin/auth compatibility and source account disable/rotation. +- [ ] Add authenticated tracker webhook normalization with source-bound project checks. If the + deployed Jira hook cannot be authenticated using a supported mechanism, support polling and + report the webhook limitation. Never accept an unverified hook just because its URL has a key. +- [ ] Reconcile current label set with additions/removals and stable event ordering; exhaust required + history pages or return unknown. No actor fallback to issue reporter/editor. Commit scan cursors + with reconciliation and cap each sweep; retries cannot duplicate items or lose pages. +- [ ] Implement source comments/transitions through outbox effects with deterministic markers and + uncertain-write handling. `WorkSourceEffectsTest.retryFindsThePreviouslyWrittenComment` must + observe a successful remote write followed by a client timeout before retry. +- [ ] Kill audit-completeness, remove/re-add and source-scope guards in targeted tests. Test credential + errors as health failures, not issue deletions. Show the supported operations on source settings. +- [ ] Run all three arm suites plus service recovery tests sequentially; record which token families/ + webhook variants have only documentation/WireMock evidence, and commit the parity slice. + +## Slice 7 — re-resolve policy and persist dashboard approvals + +**Files:** full policy resolver/registry/UI, transitions, gate storage/resource/expiry, attention, +Approvals screen; ADR-045 policy decision. + +**Produces:** checked profile vectors, visible clamps, current-policy phase decisions, durable gates, +expiry and operator answers. + +- [ ] Resolve the profile ordering choice with the analyst. Write criterion 2 and 4 tests plus + `AutonomyProfileTest.requiresDistinctProfilePrecedence`, + `AutonomyProfileTest.incomparableVectorsMeetWithoutWideningEither`, + `AutonomyProfileTest.omittedPhaseIsOff`, + `WorkItemPolicyIT.lowestEligibleLabelWins`, + `WorkItemPolicyIT.profileEditCannotWidenAnAdmittedVersion`, + `WorkItemPolicyIT.removedLabelStopsTheNextTransition`. +- [ ] Implement versioned vector/precedence validation, current allowed label selection, pinned + admission version and component-wise restriction. Record selection/clamp/reason with policy revision. + Include source disabled/allowlist removed, stricter caps and protected-path floor cases. +- [ ] Call the transition service from every entry point named in design §6.2. Re-read evidence + outside the transaction and compare registry revision inside it; stale external data must not + become authority after a newer local edit. Include outbox retries and operator resume. +- [ ] Implement gate open/resolve/expiry atomically with event/outbox and reservations. Write + `GateResourceTest.concurrentAnswersProduceOneResolution`, + `GateExpiryTest.exactDeadlineRefusesALateApproval`, + `GateExpiryTest.restartExpiresOpenGateAndReleasesReservation`, + `GateResourceTest.viewerCannotResolveAGate` and `GateResourceTest.replayedAnswerIsIdempotent`. +- [ ] Render Approvals and integrate attention using current OPEN/expired/clamped conditions. A + resolved gate disappears from open views. UI submits expected version and displays 409/503 + honestly; status union, renderer and filters change together. +- [ ] Kill criterion 2/4 mutations and each concurrency/expiry/auth guard with isolated tests; use + an injected clock and real PostgreSQL interleavings, not sleeps against the live scheduler. +- [ ] Run contract, orchestrator, UI tests sequentially; demonstrate restart and ceiling downgrade + through real APIs in the test stack; update ADR-045 and commit. + +## Slice 8 — connect policy-controlled work to the delivered run path + +**Files:** artifact reference handoff, dispatcher, run-result bridge, item/run FK metadata, +`FactoryPullRequests`, `PullRequestSink` and all three arms, work-item UI. + +**Produces:** criterion 1's accepted M3 journeys; actual one-task build and policy-aware delivery +boundaries. No production verifier is invented to reach the delivery test cases. + +- [ ] **Before coding, resolve design §11.1/§11.5.** Confirm prepared manual artifacts versus M4 + scope, execution ordering of review/deliver, and draft capability behavior. Update this task's + exact journey expectations if the analyst changes the boundary; never use no-op phase success. +- [ ] Write `WorkItemJourneyIT.threeProfilesProduceDifferentVisibleJourneys` and UI journey test + from the acceptance matrix. Use real persisted policy/source/item data; scripted execution is + permitted only in tests and identified as such. Also write + `WorkItemRunBridgeTest.itemRunCannotUseStandaloneAutomaticProposal`, + `WorkItemRunBridgeTest.duplicateResultAdvancesTheItemOnlyOnce`, + `WorkItemRunBridgeTest.ceilingChangesBeforeDeliveryPreventTheProposal`. +- [ ] Fetch human-supplied tracker artifact references/digests and bind gates to them. Missing or + changed artifacts require input/new approval. Dispatch through the existing run assembly/caps + using the repository's selected FACTORY identity and stable item/attempt linkage. +- [ ] Persist an effect claim before dispatch, recheck current policy before publishing and make + run/result association recoverable after crash. Do not reset attempts on re-admission or charge + the same run result twice. Existing standalone runs remain outside work-item gates. +- [ ] Implement the accepted work-ready/delivery-permit handshake from design §6.3. An item-linked + run starts with publication held, checkpoints without pushing, persists awaiting-delivery and + releases active compute. Resume only the trusted publisher on a current delivery permit; keep + workspace and hold through restart. Add contract/result/control fields, runtime lifecycle and + UI states together; never send a permit through a repository-writable file. Deduplicate charge + reporting across work-ready and terminal results. Add + `WorkItemDeliveryIT.deliverOffNeverPushesTheBuiltBranch`, + `WorkItemDeliveryIT.deliveryPermitPublishesWithoutRebuilding`, and + `WorkItemDeliveryIT.workReadyAndFinishedDoNotDoubleCharge` in the worker service tier. +- [ ] Prevent item-linked BUILD results from falling through `FactoryPullRequests.propose` before + their deliver transition. Implement policy-controlled PR opening, observed reviewer result and + land readiness according to the accepted order. Never mark missing review/verify as passing. +- [ ] Extend the sink with explicit draft capability/request semantics and update constructors, + withers, snapshots and each adapter. Unsupported `draft_pr` visibly blocks delivery. Do not + send a regular PR and label it a draft. Preserve find-by-head idempotency and existing FIX + source-branch semantics. +- [ ] Prove actual run execution separately in `WorkItemRunJourneyIT.preparedItemBuildsAndWaitsForVerification` + (`spire-run-worker` service tier): real containers and local test origin plus provider fixture, + distinct from a live-forge proof. This suite must share the existing Docker serialization lock. + Delivery tests supply valid prior phase results through an explicitly test-only phase driver; + the production handler for an unavailable verify capability continues to block honestly. +- [ ] Kill criterion 1 mutations and the standalone-proposal bypass separately. Run relevant run, + orchestrator, sink adapter and UI suites sequentially. Also remove the initial publication hold + and isolate `WorkItemDeliveryIT.deliverOffNeverPushesTheBuiltBranch`: exactly one test must fail + on the real remote's changed head. Restore and rerun green. Commit the complete runnable journey. + +## Slice 9 — answer outside the dashboard and take over safely + +**Files:** gate channel routing, normalized tracker/PR activity, takeover/resume, durable run control +publication hold, publisher/runtime/orphan finalization, UI suspended state and attention. + +**Produces:** one gate resolution path across three channels; takeover persists and suppresses new +effects, including salvage publication after a restart. + +- [ ] Write `GateChannelsIT.dashboardTrackerAndPrReviewResolveTheSameGate`, + `GateChannelsIT.prReviewCannotApproveAPlanGate`, + `GateChannelsIT.staleHeadAndDismissedReviewCannotApprove`, + `HumanTakeoverIT.humanCommentSuspendsUntilOperatorResume`, + `HumanTakeoverIT.knownMachinePushDoesNotTakeOver`, + `HumanTakeoverIT.gateReplyIsNotReprocessedAsTakeover`. +- [ ] Normalize source delivery identity/channel, actor and artifact/head. Re-read current + permission/review evidence before approval. Dashboard OIDC authority and tracker actor ids + must never be compared in the same namespace. Unknown approval capabilities are disabled. +- [ ] Implement human activity classification from real linked branch/PR observations. Persist + takeover, supersede gates and stop unstarted effects transactionally. Record resume actor/note; + re-resolve policy and head before allowing a fresh action. Transfers retire rather than resume. +- [ ] Design the publication hold through the existing `RunCommand` control vocabulary and worker + durable state; inspect actual runtime/finalization interfaces before editing. Hold must survive + queued delivery, restart and orphan recovery. Carry the hold to a trusted publisher control + channel rather than trusting a flag in a repository-writable file. Preserve local work and + standalone cancel semantics. Record the unavoidable already-in-progress push race honestly. +- [ ] Add `PublicationHoldIT.takeoverPreservesWorkWithoutPushingOnCancel` and + `PublicationHoldIT.orphanRecoveryKeepsTheDurablePublicationHold` in `spire-run-worker`, using a + real remote whose head is measured before/after and a deterministic pause before publication. + These are not satisfied by asserting that `CancelRun` was emitted. +- [ ] Add `WorkItemRetirementIT.transferRetiresOldIdentityAndInvalidatesOpenGate` and + `WorkItemRetirementIT.sourceOutageDoesNotPretendTheIssueWasDeleted`. +- [ ] Kill gate scope/head, human-vs-machine identity, expiry-on-answer, retired-state and + publication-hold guards independently. A mutation killed by an earlier unrelated refusal is + invalid; prove the fixture reached its intended production line. +- [ ] Run gateway, orchestrator and Docker worker suites one at a time, then UI tests. Document + native approval capabilities and takeover race limits. Commit the cross-channel slice. + +## Acceptance proof matrix + +All methods/classes in this matrix are **tests to add**, not claimed existing coverage. `O-test` +means `spire-orchestrator/src/test/java/dev/codespire/orchestrator/`. Tests exercise the public +resource/consumer path plus persisted outcomes; helpers may stub external HTTP at adapter edges. +Every integration proof has a visible UI assertion or a matching component test where required. + +| # | Ticket exit criterion and exact proof | Production mutation and isolated expected failure | +|---|---|---| +| 1 | **Three visibly different journeys.** `O-test/workitem/WorkItemJourneyIT.java#threeProfilesProduceDifferentVisibleJourneys`: label three otherwise identical `TEST-` prepared tasks under suggest/assisted/autonomous with ceiling autonomous, inspect persisted timeline/API: suggest stops before build; assisted waits at plan approval with zero runs; autonomous starts one build without approval. Then approve assisted and assert its recorded human decision and single dispatch. Assert exact phase/gate/effect fields; missing verify remains waiting. `U/components/workItems/WorkItemJourney.test.tsx` → `renders distinct suggest assisted and autonomous journeys` proves visible differences. Draft/regular PRs are separate delivery tests with a test-only prior-phase driver. Scope conditional on slice 8's explicit analyst decision. | First mutant: in the real transition policy branch change `approve` to proceed without opening its gate; run only the Java method, expect exactly one failed test. Restore. Second mutant: render all journey status labels as the same label; run only the named vitest case, expect one failure. Restore. A test that only compares three profile names is insufficient. | +| 2 | **Above-ceiling label clamped and says so.** `O-test/workitem/WorkItemPolicyIT.java#aboveCeilingLabelRecordsAndDisplaysClamp`: request autonomous at assisted ceiling; assert effective vector, durable clamp event after reload, attention API row and detail reason. `U/components/workItems/WorkItemPolicy.test.tsx` → `shows requested and effective profiles with the clamp reason`. | Mutate the effective-profile meet to retain requested authority; isolate Java method, one failure. Restore. Separately omit the clamp event/attention projection write; same isolated method must fail once. Restore. Mutate the UI clamp message to empty and run the named UI case for one failure. Each proves a different half of “and says so.” | +| 3 | **Unlisted and unattributable appliers ignored.** `O-test/workitem/WorkItemIntakeIT.java#unlistedLabellerSelectsNoProfile` and `#unattributedCurrentLabelSelectsNoProfile`, each using a mapped label that would otherwise dispatch, valid source/account/ceiling and assertions of ignored reason plus no run effect. Add `#allowedAttributedLabellerCanSelect` as positive control. | Delete the actor-membership check; run only `unlistedLabellerSelectsNoProfile`, one failure. Restore. Delete the attribution check; run only `unattributedCurrentLabelSelectsNoProfile`, one failure. Fixture for the latter has an actor hint that would pass membership but origin UNATTRIBUTED, so the origin guard alone distinguishes it. Use additional no-id test for real missing actor. Never combine both negative cases into one count. | +| 4 | **Lowering ceiling stops an in-flight item at next phase.** `O-test/workitem/WorkItemPolicyIT.java#loweredCeilingStopsAnInFlightItemAtTheNextPhase`: admit and start under higher policy, commit a lower ceiling with next phase off, then send the prior phase's result. Assert no next effect/PR and persisted stop/reason on detail. `#ceilingChangeBeforeGateAnswerRequiresANewDecision` covers waiting items. | Replace the transition's current ceiling lookup with admission-time ceiling, preserving all other checks; run only the first method, one failure. Restore. Separately bypass policy recheck in gate resolution and isolate the second method, one failure. The mutation must not be masked by also removing a label or disabling an account in the fixture. | +| 5 | **Push access without a list, refusal without access, overrides both ways.** `O-test/pipeline/FixPermissionSagaTest.java` methods `writerWithEmptyOverridesDispatches`, `writerOutsideTheLegacyAuthorListDispatches`, `readerWithoutOverrideIsRefused`, `explicitGrantLetsAReaderDispatch`, `explicitDenyStopsAWriter`, `unknownPermissionCannotDispatch`. Invoke the real saga entry; assert one/zero dispatch and exact decision reason. Each forge also adds `*RepositoryPermissionSourceTest#inheritedWriteAccessIsRecognized` and `#unknownResponseCannotGrant`. | Six separate compiling mutants: default unlisted to deny; restore the legacy outer author check for `/fix`; grant the reader result; skip explicit ALLOW; skip explicit DENY; map UNKNOWN to allow. Select the corresponding single method for each mutant; exactly one failure each, snapshot restoration and baseline pass between them. Test fixture is same-repository/open valid finding with available caps, so unrelated guards do not kill the proof. | +| 6 | **Type handle, store id, render handle; unresolved refused.** `O-test/provider/ActorResolutionResourceTest.java#handleEntryStoresStableIdAndReturnsHandle` sends a `TEST-` handle via resolve/save and asserts actual DB id (not its textual handle), then reads after restart. `#unresolvedHandleIsRejectedWithoutWriting` asserts 422 and unchanged rows. `U/components/accounts/ActorPicker.test.tsx` → `saves a resolved handle and renders it after reload`, plus `refuses unresolved input`. Resolved provider fixture data is explicitly synthetic and never inserted into the dev stack. | Replace the repository/account actor-id binding with input handle and isolate the first Java method, one failure. Restore. Remove refusal on not-found and attempt a raw text write; isolate the second, one failure. Restore. Render stored id/count in place of handle and isolate the named successful UI round-trip, one failure. Refuse a mutant that only changes the mocked response instead of production code. | +| 7 | **Repository shows workspace/accounts/one hook per kind; accounts have no workspace.** `O-test/repository/RepositoryResourceTest.java#repositoryOwnsWorkspaceAndRoleBindings`; gateway `dev.codespire.gateway.registry.RepositoryWebhookKindsTest#refusesASecondWebhookForTheSameKind`; `U/components/repositories/RepositoryDetail.test.tsx` → `shows workspace selected accounts and one webhook per event kind`; `U/components/SettingsProviders.form.test.tsx` → `does not offer workspace on an account`. UI uses distinct configured reviewer/factory fixtures, missing/disabled states and all enabled hook kinds. | Drop repository-account binding filter and isolate the Java test, one failure. Restore. Drop only the per-kind uniqueness constraint in an isolated migrated PostgreSQL test schema; allow duplicate insertion through the real registry API, isolate gateway test, one failure. Restore migration snapshot and recreate isolated schema. Hide the workspace/account/hook section, each as a separate UI mutant of the same named case, one failure each. Reintroduce workspace input on account form; run the named account-form case, one failure. | + +Criterion 7's database mutant must be a change to production migration/constraint code applied to a +fresh test schema, not a manual ALTER of the shared dev database. If an application guard masks the +constraint mutation, target the repository method directly within the same test while keeping +valid input; prove exactly the constraint being claimed. Record that selection explicitly. + +## Mutation protocol and additional guard obligations + +Each slice is responsible for every guard it adds, not only the seven headline criteria. Maintain +an evidence table with production path/line, snapshot hash, changed line, exact test selector, +baseline result, mutated failure name/count, restored hash and restored pass. The evidence belongs +in the slice's review notes, with a concise PR checklist pointer; no invented pass counts. + +1. Ensure no other Gradle test invocation is running. Copy the current source/migration to a unique + file under the session scratchpad and record its hash. The snapshot includes uncommitted work. +2. Run the selected test green with tasks forced. For example: + `.\gradlew.bat :spire-orchestrator:test --tests + 'dev.codespire.orchestrator.pipeline.FixPermissionSagaTest.writerWithEmptyOverridesDispatches' + --rerun-tasks`. Pass the actual argument as one shell token; line wrapping here is prose. +3. Edit exactly one production guard. Verify the intended text changed (match `\r?\n` if needed). + Re-run the exact method. Inspect JUnit XML for exactly one failure, the designated assertion, + no compilation errors, no setup/container failure, and no skipped target. Targeting one method + is intentional; this does not claim the full suite contains only one affected assertion. +4. Restore by copying the scratch snapshot back. Check its hash and run the same method green. + Use a `finally` restoration in scripted checks. Never use `git checkout`, `git restore`, reset + or a cached Gradle result as restoration/evidence. +5. For UI mutations use `npx vitest run -t ''` from `spire-ui`; inspect the + executed case and failure count. Restored case must pass. Run the whole affected class/file + after its individual mutations to catch fixture leakage. +6. If a second guard masks the target, improve the discriminating fixture; do not delete several + guards together. A surviving/non-compiling mutant is not “killed.” Record failures honestly. + +Additional required isolated guards and their exact proposed witnesses: + +| Guard | Witness | Mutation | +|---|---|---| +| Same-origin account binding | `RepositoryAccountsTest.rejectsAnAccountFromAnotherOrigin` | Remove origin equality, retain matching kind/role. | +| Credential rotation/reference safety | `RepositoryAccountsTest.disabledAccountCannotServeAnExistingBinding` | Remove enabled filter after a valid binding was created. | +| Host-qualified item identity | `WorkItemIdsTest.samePathsOnDifferentOriginsHaveDifferentIds` | Omit origin from derived identity. | +| Per-source actor namespace | `WorkItemPolicyIT.sameIdOnAnotherSourceDoesNotAuthorize` | Resolve actor membership from the other source. | +| Lowest eligible label | `WorkItemPolicyIT.lowestEligibleLabelWins` | Select highest eligible label instead. | +| Pinned immutable profile | `WorkItemPolicyIT.profileEditCannotWidenAnAdmittedVersion` | Use latest version rather than pinned at transition. | +| Current label removal | `WorkItemPolicyIT.removedLabelStopsTheNextTransition` | Reuse intake labels for continuation. | +| Omitted phase refusal | `AutonomyProfileTest.omittedPhaseIsOff` | Default an absent phase to auto. | +| Gate deadline | `GateExpiryTest.exactDeadlineRefusesALateApproval` | Change `now >= expiresAt` to `now > expiresAt`. | +| Concurrent answer | `GateResourceTest.concurrentAnswersProduceOneResolution` | Remove expected-version/OPEN compare at the transaction write. | +| Dashboard authority | `GateResourceTest.viewerCannotResolveAGate` | Permit viewer on mutation resource. | +| Gate PR scope | `GateChannelsIT.prReviewCannotApproveAPlanGate` | Ignore required land phase when translating approval. | +| Artifact/head freshness | `GateChannelsIT.staleHeadAndDismissedReviewCannotApprove` | Accept approval without current-head verification; test stale-head branch alone. Use a second isolated method for dismissal mutation. | +| Retirement | `WorkItemRetirementIT.transferRetiresOldIdentityAndInvalidatesOpenGate` | Continue old item after confirmed identity transfer. | +| Machine identity in takeover | `HumanTakeoverIT.knownMachinePushDoesNotTakeOver` | Treat matching recorded factory actor as human. | +| Publication hold after restart | `PublicationHoldIT.orphanRecoveryKeepsTheDurablePublicationHold` | Omit durable hold read before orphan finalization. | +| Event/projection/outbox atomicity | `WorkItemStoreTest.rollbackLeavesNoGateEventOrOutboxEffect` | Append on a separate autocommit connection before the forced transaction failure. | +| Idempotent result continuation | `WorkItemRunBridgeTest.duplicateResultAdvancesTheItemOnlyOnce` | Remove consumed-result dedupe; preserve valid active state for both deliveries. | +| No early item PR | `WorkItemRunBridgeTest.itemRunCannotUseStandaloneAutomaticProposal` | Remove the item-association exclusion in automatic M2 proposal. | + +Tests with multiple scenarios should be split into individually selectable methods before their +mutations if one scenario would mask another. Every guard discovered during review is added to +this table with its discriminating witness before that slice is called complete. + +## Slice 10 — integrated proof and handoff + +- [ ] Run `testFast --rerun-tasks` and, after it exits, `testServices --rerun-tasks`. Verify every new + module is included in its proper tier. No parallel second Gradle invocation. +- [ ] From `spire-ui`, run `npm test` and `npx tsc --noEmit`. Check CSS contracts, routes/deep links, + keyboard form behavior and new unknown/refused/suspended status rendering. +- [ ] Run the whole criterion test classes after targeted mutation baselines; inspect JUnit/vitest + counts. No compilation/setup failure, skipped suite or empty fixture is an acceptance pass. +- [ ] Run the warm `spire-e2e` stack only if already available or separately provisioned without + touching `spire-dev`. Add `dev.codespire.e2e.WorkItemGateJourneyTest` for signed tracker event → + durable admission → gate → policy-controlled continuation against real GitLab. If the run unit + cannot reach its local GitLab, record the known network limitation; do not rebind it publicly + or claim this test proves run execution. Pair it with the real-container/local-origin worker + proof and an explicitly recorded live external-forge run when authorized and feasible. +- [ ] Demonstrate all seven acceptance criteria through actual APIs/UI with supported provider + configuration. Any live canary uses announced ids and exact local cleanup before insertion, + with explicit remote cleanup. Store only observed ids, dates and results in the evidence notes. + A live credential/permission probe is evidence about that account/token family, not all tokens. +- [ ] Record all mutation selectors and measured results, missing capabilities, unavailable live + credentials and exact remaining proof gaps. Do not mark acceptance complete if a required + criterion lacks evidence; bring that gap to the analyst. +- [ ] Update `docs/DECISIONS.md` with accepted ADRs; reconcile factory architecture/topic/data + catalogues and M2 live-proof statements; update smoke-test instructions, registry upgrade steps, + security/SCM mapping, factory roadmap and `docs/HISTORY.md`. Rewrite `CLAUDE.md` status/measured + counts from actual results, preserving unrelated UNVERIFIED entries. +- [ ] Submit each slice for analyst review and resolve findings with their tests/docs. The PR + remains draft until the analyst's review process says to mark it ready; this plan authorizes + no merge, deployment or privilege changes. + +## Round 1 report content + +Report the draft PR number/link, the two document paths, the commit and push verification, and +that this round ran documentation checks only. Explicitly call out the analyst decisions in design +§11: M3/M4 journey and phase-order boundary, ordering incomparable profiles, permission/handle +portability, legacy org/source cardinality, native drafts and takeover precedence. Those are +review inputs, not reasons to withhold the requested draft PR. diff --git a/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md new file mode 100644 index 00000000..c1dc9044 --- /dev/null +++ b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md @@ -0,0 +1,604 @@ +# Factory M3 — work items, labels and gates — design + +**Date:** 2026-09-12 + +**Status:** Round 1 proposal for analyst review; no implementation or test results claimed. + +**Issue:** [#114](https://github.com/artyomsv/code-spire/issues/114), re-read from GitHub, +updated `2026-09-12T21:47:45Z`. + +**Plan:** [Ordered slices and proof obligations](../plans/2026-09-12-factory-m3-work-items.md). + +## 1. Scope and evidence + +M3 makes a tracker ticket the entry point to the factory, with operator-owned policy, attributable +labels, durable approvals and human takeover. It also moves workspace ownership to repositories, +replaces `/fix`'s list-only authorization with repository push permission plus explicit overrides, +and lets people enter handles while authorization continues to use stable provider ids. + +The ticket records the live M2 loop on `artyomsv/spire-test#31`, runs `3987682681:1` and +`3987682176:1`: command, dispatch, push to the existing source branch, another review, resolved +thread and persisted verdict. That is the issue's reported evidence, not a measurement made in +this planning round. M3 is unblocked. The older contrary statements in `CLAUDE.md` and +`docs/UNVERIFIED.md` need reconciliation during implementation; the automated GitLab run-unit +network gap remains a separate claim and must not be deleted on the strength of a live GitHub run. + +Read alongside [AUTONOMY](../../factory/AUTONOMY.md), [factory PRD](../../factory/PRD.md), +[factory architecture](../../factory/ARCHITECTURE.md), [decisions](../../DECISIONS.md), +[unverified claims](../../UNVERIFIED.md), and the +[Accounts design](2026-09-07-accounts-and-roles-design.md) and +[Accounts plan](../plans/2026-09-07-accounts-and-roles.md). The September 7 documents establish +format and review depth; ADR-041 and the updated issue supersede their deferred account design. + +### What exists at branch base `27fe17b` + +| Observation | Implementation evidence | Consequence | +|---|---|---| +| Only review events implement `DomainEvent`. | `spire-contract/.../event/DomainEvent.java` | There is no run or work-item aggregate to extend by assumption. | +| Run results update the run projection, charges, credential feedback and PR proposal directly. | `spire-orchestrator/.../factory/RunResultSaga.java` | Keep delivered run durability; introduce workflow ownership deliberately. | +| PR proposal is now called after a finished run. | `factory/FactoryPullRequests.java` | Item delivery must intercept this path or a gated item will open a PR early. | +| Account lookup is by type, workspace and scalar role; a workspace-only fallback still exists. | `provider/ProviderRegistry.java`, `factory/MachineAccounts.java`, migration V44 | Changing only the form or UNIQUE key cannot move workspace ownership. Every resolver needs repository coordinates. | +| CONTEXT rows have null workspace; REVIEWER/FACTORY rows must have one. | Orchestrator V59, `scm_provider_workspace_by_role` | Both this CHECK and the old UNIQUE constraint need migration. Account ids and encrypted credentials can stay intact. | +| Gateway owns `webhook_repo`, including secrets and org/repo scope. | Gateway V1; `registry/WebhookRepoRegistry.java` | No cross-schema FK, SQL join, or orchestrator credential lookup at webhook ingress. | +| The keyed edge verifies signature, then every event's scope, then publishes. | Gateway `RegistryWebhookEdge.java` | Add tracker scope and event-kind validation here; Jira cannot be forced into `RepoRef` parsing. | +| `/fix` has two list barriers. | `IntegrationSaga.onManualCommand` and `requestFix` | Replacing only `allowedById` still denies a push-authorized person before the switch. | +| Context clients expose only `getJson`, sharing `PinnedJsonClient`. | `GitHubIssueClient`, `GitLabIssueClient`, `JiraClient` | Reuse transport and auth configuration, but keep writes off the context-reader API. | +| The existing sink API has no draft flag. | `PullRequestSink.NewPullRequest` | `draft_pr` is actual adapter work, not a label to paint on a regular PR. | + +Paths abbreviated with `...` above are under `src/main/java/dev/codespire//`. +The implementation plan names exact source roots for new files. + +## 2. Decisions and ADRs + +These are proposed decisions, not already accepted ADRs. Reserve the next available numbers at +implementation time; `042`–`045` are the expected sequence after ADR-041. + +| ADR | Decision to record | Existing decisions affected | +|---|---|---| +| ADR-042 — Repositories own workspace and account bindings | Account identity is its UUID; repository identity includes forge origin. Remove UNIQUE `(type, workspace, role)` and the workspace-by-role CHECK. Explicit repository-role bindings replace workspace resolution. Gateway owns webhook registrations independently. | Completes ADR-041 deferrals; preserves scalar roles and separate identities under ADR-038. | +| ADR-043 — A work item owns workflow milestones; a run remains a durable execution record | Add an event-sourced `WorkItemLifecycle`, transactional milestone/gate/outbox persistence, and separate work-item keys. Do not invent or backfill a run aggregate. | Clarifies ADR-034's aspirational milestone catalogue; generalizes ADR-010's single-writer rule to one writer per aggregate stream. | +| ADR-044 — Command authority uses stable identities and effective repository permission | `/fix`: explicit deny, explicit grant, then measured push permission. Handle resolution uses the selected account's credential; display names grant nothing. | Replaces the temporary M2 list-only guard; does not alter reviewer eligibility or tracker actor authority. | +| ADR-045 — Profiles have explicit precedence; authority is bounded per dimension | Versioned immutable profiles, operator-declared precedence, component-wise restriction, current labels/allowlist/ceiling at every boundary, version-bound gates and takeover precedence. | Makes ADR-033's “lowest” and “ceiling” implementable for vectors; records the M3/M4 boundary after analyst resolution. | + +### 2.1 The aggregate decision + +**A work-item aggregate will exist. A separate run aggregate will not.** A run remains the delivered +`factory_run` record plus `llm_charge`; `run_event` remains a bounded, encrypted transcript, never +state. Do not create synthetic historical `RunStarted` domain events or replay transcripts to +reconstruct runs. Amend ADR-034 to distinguish its intended milestones from the shipped run tier. + +The work item owns admission, the pinned policy version, phase cursor, gates, run associations, +retirement, takeover and completion. These decisions must survive restart, concurrent approvals, +duplicate deliveries and policy edits. A pure `WorkItemLifecycle.decide(state, command)` is their +single domain-event writer. Rehydration folds only recorded milestones and never calls a tracker, +checks today's policy, spends money or emits commands. A new decision receives current authorized +facts separately. Workers and webhooks still emit integration events; the saga converts them into +aggregate commands. + +Add work-item milestone records to `DomainEvent` and implement the lifecycle under +`spire-contract/.../lifecycle/`. `EventEnvelope` already has a generic stream id and payload. +Add typed work-item payload decoding and routing: today's `DomainEventSink` writes every envelope +to review history before its switch, so its default branch is not sufficient isolation. +`ReviewLifecycle` must reject work-item payloads and vice versa. Pure modules gain no framework +imports; extend the contract snapshots to the new nested wire types explicitly. + +**Why not just mutate `work_item` and emit an audit row?** That makes gate answers and phase changes +two sources of truth unless every writer implements the same concurrency discipline. A small pure +aggregate provides one decision table and replayable authority changes. Conversely, introducing a +second aggregate for every run would duplicate the delivered run lifecycle without solving a new +M3 invariant. Work-item events refer to run outcomes by id and result identity; the run record stays +authoritative for execution details and charges. + +### 2.2 Transactions and transport + +Use `event_log` for work-item streams, with a `work-item::` discriminator in the derived stream id. +Use separate `cs.work-integration` and `cs.work-commands` topics keyed by workItemId; `cs.events` +retains envelopes keyed by their own stream id. Run commands/control/results remain keyed by runId. +Update topic provisioning, serializers, retention and `spire-arch` checks together. Never put a +work-item command on the review worker's `ActionCommand` consumption path. + +One orchestrator transaction locks the item/version, checks the policy revision used to decide, +appends the expected event sequence, updates `work_item` and `work_item_gate`, records the input +deduplication key, and writes outgoing effects to a work-item outbox. A conflict reloads and +re-decides; it does not reuse a stale decision. The transaction must share one JDBC connection: +calling today's independently connected `JdbcEventStore.append` beside another repository write +does not make them atomic. Introduce a connection-scoped append implementation and retain the +existing `EventStore` adapter for review callers. Test actual rollback with PostgreSQL. + +Outbox delivery is at least once with stable effect ids. Mark sent after broker acknowledgement. +Consumers deduplicate by effect/delivery id, not receipt timestamp. A committed gate survives a +crash before publishing; a redelivery cannot open a second gate or dispatch a second run. Current +item and policy are checked again before releasing an unstarted effect. Disabled sources, unknown +policy, stale observations and transferred issues cannot authorize new effects. + +Repository registration snapshots need their own registry channel, keyed by gateway registration +id rather than workItemId. Add `cs.registry-integration` and its durable gateway outbox in slice 1; +do not force configuration snapshots through either a review or a work-item aggregate stream. +Gateway webhook success still waits for broker acknowledgement, as `IntegrationPublisher` does +today. A failed publish returns a retryable failure, never an accepted-but-lost delivery. + +Tracker comments and PR creation also need recoverable side-effect records. Use deterministic +comment markers and read-before-retry, with bounded retries and an explicit uncertain state when +the tracker cannot establish whether a timed-out write succeeded. Do not claim remote exactly-once +behavior. Existing M2 standalone run proposal behavior remains; item-linked runs are proposed only +by the item's delivery effect, not `FactoryPullRequests.propose`'s unconditional BUILD path. + +## 3. Repository and account ownership + +### 3.1 Registry model + +In the orchestrator schema introduce: + +| Table | Essential fields and constraints | +|---|---| +| `repository` | `id UUID`, `scm_type`, canonical `forge_origin`, `workspace`, `slug`, provider repository id when known, enabled, revision, timestamps. UNIQUE `(scm_type, forge_origin, workspace, slug)`. One workspace per repository now. | +| `repository_account` | repository id, account id, role; UNIQUE `(repository_id, role)` for REVIEWER and FACTORY. An account can serve many repositories. References block account deletion. | +| `repository_fix_actor` | repository id, stable actor id, effect `ALLOW` or `DENY`, observed handle and resolution time; one effect per actor. | + +`scm_provider.id` stays the credential identity and Tink AAD stays `provider:`. Do not copy or +re-encrypt account secrets for this key change. Do not replace the old uniqueness with +`UNIQUE(type, role)` or with a uniqueness on handle, bot id or token: multiple credentials at the +same host and role are legitimate. Keep immutable scalar roles. Account kind/origin changes with +references are refused pending reassignment; token rotation updates all consumers as in ADR-041. + +Binding checks enforce matching forge kind and normalized API origin, the selected account's role, +enabled state at use, and distinct resolved reviewer/factory identities when both are known. +No resolver silently selects the first account with working credentials. Repository views name +the configured account even if disabled or unreachable and distinguish selection from measured +reachability/permission. CONTEXT sources continue to use their explicit account references. + +Repository lookup must carry host as well as type and path; a self-managed forge and its cloud +counterpart may contain the same namespace. Existing ambiguous legacy review coordinates are +shown as needing repository assignment and cannot dispatch a factory run. Do not broaden this +milestone into rewriting every historical review id or charge reference. + +Replace `resolve(type, workspace, role)`, `registration(...)` and `resolveByWorkspace` on active +paths with `RepositoryAccounts.resolve(repositoryId, role)`. `ReviewProviderResolver`, manual +registration/rerun, prompts, `IntegrationSaga`, `MachineAccounts`, run resources, fix dispatch and +PR proposal must all use it. Serving chips call the same resolver's non-secret view. + +### 3.2 Migration and rollout + +Use an expand/bridge/contract migration, with independently numbered orchestrator and gateway +Flyway files (next orchestrator number is expected to be V60; verify before allocating). + +1. Create repository/binding tables while old workspace resolution still serves existing traffic. + Snapshot old `(account id, type, origin, workspace, role)` assignments into a migration-only + mapping table. Keep credentials and ids unchanged. Add nullable repository references to + existing review/run records; do not invent a host when historic records cannot establish it. +2. Bootstrap repositories from real known review/run coordinates and gateway registrations. + Gateway publishes a versioned, non-secret registration snapshot through a durable outbox; + orchestrator consumes it idempotently. No cross-schema SQL access. Bind a role only when the + old assignment and host identify exactly one account. Ambiguous rows remain pending with an + attention entry; count and report them. No placeholders that look like actual repositories. +3. Preserve org webhook coverage during the bridge. A verified event for a previously unseen + repository may materialize a real repository using the snapshotted legacy assignment, with + origin supplied by its registration. It must not inherit a different host or newer account by + workspace alone. An existing org scope is an explicit migration exception, not a new model + where an account owns every future repository. Whether to retain that convenience after the + bridge is an analyst question (§11). +4. Cut over every runtime resolver and the repository screen together after mapping checks pass. + Unresolved repositories fail closed for action with a named repair path. Remove account + workspace input and validation, drop the old UNIQUE and workspace-by-role CHECK, then drop + the workspace column when the bridge no longer needs it. The migration-only snapshot is + explicitly excluded from account selection after cutover. Preserve source credential recovery + columns from V59; their removal is unrelated to this migration. +5. Update both packaged compositions, local dev configuration and upgrade instructions for the + new wire fields. Gateway/orchestrator upgrades need a compatible overlap. A new consumer can + read legacy registrations during the bridge; after cutover an old payload with ambiguous + repository identity is refused. No claim of binary downgrade after contracting columns. + +Test migration from V59 with real PostgreSQL/Flyway, distinct hosts, nested GitLab namespaces, +multiple roles, disabled accounts, context references, an org registration and an interrupted +snapshot exchange. Prove identical credential decryption before and after, exact row mappings, +idempotent restart and refusal of ambiguous mappings. Do not exercise this on the running dev DB +in a test. The bridge must preserve its existing webhook keys and encrypted secrets. + +### 3.3 Repository screen and webhook model + +`#/settings/repositories` becomes a repository list and detail, not a list of webhook rows. Register +a repository by selecting forge/host, entering workspace and slug, and selecting accounts for the +roles that may act. A repository may exist with no webhook or no factory account; its state says so. +Its detail shows **Workspace**, **Accounts**, **Webhooks**, **Work sources** and **Autonomy**. + +Gateway retains webhook ownership and its own admin API. Extend registrations with a logical +repository id (no cross-service FK), source id where applicable, canonical origin, revision and +event kind. UNIQUE `(repository_id, event_kind)` means one active registration for each product +kind, not one hook per low-level forge action. Proposed kinds: + +| Kind | Accepted events and effect | +|---|---| +| `REVIEWER` | Existing review PR lifecycle and discussion commands, including `/fix`. | +| `FACTORY` | Branch pushes, PR human activity, PR approvals and delivery/merge observations for linked work items. | +| `ISSUE` | Tracker issue/label/comment/transfer events for the repository's work source; the future product-owner role is not introduced here. | + +Separate normalized event types prevent a duplicated PR comment delivered to both hooks from +dispatching twice: the reviewer route parses commands; the factory route observes activity. Keep +source delivery identity across both routes and deduplicate logical effects. A command or gate +answer recognized on one channel must not later be interpreted as unrelated takeover (§8). + +The gateway rejects a validly signed payload for the wrong repository, tracker project or event +kind before publishing anything. A Jira project is validated against its work-source scope and +mapping, not compared with an SCM workspace. Webhook keys are routing identifiers, never a +substitute for provider-supported signature/token verification. A Jira installation that cannot +authenticate deliveries safely requires polling; it does not get a secret-in-URL bypass. + +The UI composes the two authenticated APIs; no browser or orchestrator receives a webhook secret +on read. Creation shows its secret once. Because registration spans two services, save the repository +first, then create each hook idempotently. Partial failure leaves an honest “webhook setup pending” +row with Retry, rather than rolling back a repository that may already be referenced. Legacy routes +and `?edit=` continue to open the matching repository or a clearly labelled legacy org registration. + +## 4. People, handles and `/fix` + +### 4.1 A stable id with a readable label + +Add an account-scoped identity directory port alongside `IdentitySource`: exact handle lookup +and stable-id lookup return `{providerUserId, handle, displayName}` with typed not-found, +ambiguous, unavailable and unsupported outcomes. Composition belongs in `ProviderClients`; adapter +URLs, escaping and response parsing stay in the three SCM modules. Work-source identities use +their own adapter and source account. No email is persisted or logged. + +Existing account policy entries are edited through an authenticated admin endpoint such as +`POST /api/providers/{id}/actors/resolve {handle}`. New accounts can first be saved as credentials +and then have policy entries added. Repository fix overrides resolve using its selected reviewer's +account. The server performs the resolution again on save, or verifies an account/revision-bound +resolution token; it does not trust a submitted id/handle pair from the browser. Failed or ambiguous +lookup returns 422 and writes nothing; upstream unavailability is a retryable 503, never a text entry. + +Authorization stores and compares the stable id in the provider's actual namespace. An observed +handle and timestamp are display metadata, refreshed by id; a rename updates the display, a +reassigned handle never changes the stored id. If a refresh fails, show the last known handle as +stale or a labelled unresolved id, never `1` as a substitute for identity. Legacy raw numeric ids +remain valid ids; legacy raw handles are flagged for explicit re-resolution and do not acquire +`/fix` authority merely because the string now resolves to someone. + +For GitHub and GitLab, exact `@handle` input is meaningful. Bitbucket privacy-era nicknames and +Jira display names need not be unique, resolvable handles. Do not implement a first-search-hit +fallback. Those forms need a credential-backed, disambiguated person selection when exact +resolution is unavailable. This is an explicit portability qualification for criterion 6 (§11). + +### 4.2 Authorization decision + +Use a pure `FixAuthorization` decision over actor identity, explicit repository override and a +measured `RepositoryPermission` result. Decision order: + +1. Reject unknown actor, unresolved repository/account, self-command and ordinary existing + observe/archive/target precondition failures. No author-equals-PR-author shortcut. +2. Explicit `DENY` refuses even a repository owner. Explicit `ALLOW` authorizes even a reader. + Overrides grant the command only, never permission to bypass forks, trunk protection, caps, + observe-only mode, entitlement or invalid findings. +3. Without an override, authorize only `CAN_PUSH`; `CANNOT_PUSH` refuses and `UNKNOWN` refuses + with a permission-unavailable explanation. Use the repository's assigned reviewer credential, + without falling back to a stronger factory token or another host's token. + +`/fix` must leave the common legacy author-list path in `onManualCommand` and go through this +decision instead. The common self-loop and observe checks still apply. `/review`, `/finding`, +review eligibility and conversation policy retain their current behavior. Keep their old account +list separate from the new bidirectional fix overrides. Migrate verified stable-id entries as +explicit grants to repositories previously served by that reviewer; an empty list produces no +overrides, so actual push permission decides. Present those migrated grants in the repository UI. + +Check permission at dispatch time, not merely during account Check. Cache display observations +only; a prior success during an outage is not new authority. Bind lookup to repository origin, +account id/revision and actor stable id. If the provider accepts a handle in its permission URL, +verify that the returned user is the same stable actor before accepting its permission. + +### 4.3 Provider endpoints and limits + +Checked against official API documentation on 2026-09-12; contract tests and live probes are still +required. Repository push permission means general code-write access, not a promise that a +particular protected branch accepts a push. The existing target and publisher guards still apply. + +| Forge | Permission read | Interpretation | +|---|---|---| +| GitHub | `GET /repos/{owner}/{repo}/collaborators/{username}/permission` | Use the effective `permission` base role: `write` or `admin`; maintain maps to write, triage to read. Verify returned user id. [Official contract](https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user). | +| GitLab | `GET /projects/{id}/members/all/{user_id}` | Includes inherited/invited membership. Known active Developer/Maintainer/Owner levels allow general push; read roles refuse. Unknown/custom capabilities require evidence, not numeric guesswork. [Official contract](https://docs.gitlab.com/api/project_members/#retrieve-a-member-of-a-project). | +| Bitbucket Cloud | `GET /workspaces/{workspace}/permissions/repositories/{repo_slug}` | Read the matching stable user through all pages; `write`/`admin` are effective rights including groups. This endpoint requires repository-admin access from the caller. The `permissions-config/users` endpoint measures explicit grants only and is unsuitable. [Effective-permission contract](https://developer.atlassian.com/cloud/bitbucket/rest/api-group-workspaces/#api-workspaces-workspace-permissions-repositories-repo-slug-get). | + +403, timeout, rate limit, incomplete pagination and malformed/identity-mismatched responses cannot +grant the command. A 404 is not automatically proof that a person has no rights: adapters must +distinguish an unreadable repository from a known absent member where the API permits it. Both +refuse; the displayed reason differs. For Bitbucket a reviewer lacking the documented admin access +will report unknown; the analyst must settle whether that credential prerequisite is acceptable. +Do not increase any live account's rights as part of implementation. + +## 5. Work sources and label evidence + +Create pure SPI module `spire-worksource`, with arms `spire-worksource-github`, +`spire-worksource-gitlab` and `spire-worksource-jira`. Reuse each context adapter's client/auth +configuration and `spire-http` transport. Refactor common provider transport into a reusable +internal component and expose a distinct write-capable facade to the work-source arm. Context +providers must still be unable to comment or transition through their public read-only interface. +Use the existing module licensing split and add the new modules to build and architecture checks. + +`WorkSource` offers capabilities, paginated candidates, fetch, comment, transition and paginated +`labelEvents`. A fetch returns transient ticket content and canonical identity. It is never a row +to persist wholesale. A source registration owns type, origin, external project/repository scope, +target repository id, explicit account reference, enabled state, revision, scan cursor and stable +tracker actor allowlist. Matching kind, origin and auth are checked like context sources. A forge +source can use its factory account for writes; an Atlassian source references the existing +Atlassian account. No new “product owner” role is needed to call a work-source port. + +One work-source registration targets one repository in M3, matching one ISSUE webhook per +repository. GitHub/GitLab issue coordinates must agree with that source. Jira's project mapping +is operator-owned and may target an SCM repository on a different service; the tracker credential +never travels there. Multi-repository issue routing is a future schema extension, not a label trick. + +### 5.1 Identity and bookkeeping + +Derive workItemId from versioned, length-safe encoding of `(scm type, forge origin, workspace, +slug, work-source type, tracker origin, external project id, stable issue id)`. Do not use account +id, handle, mutable issue key, source display name or title in the key. Duplicate registrations +for the same source/target are refused. Re-admission of the same identity advances a generation; +run attempts never reset to an already used id. A bounded hash/encoded subject links through the +existing `RunIds` contract; add `factory_run.work_item_id`, generation and phase references, not +a second incompatible parser for legacy run ids. + +`work_item` contains only coordinates, generation, admitted profile/version, selected/effective +policy references, phase, workflow state/reason, revision and timestamps. It has **no issue title, +body or tracker status column**. Workflow state is named `workflow_status` to prevent that +confusion. Branch/PR/run links and human-supplied artifact references are workflow bookkeeping. +An optional live title/body on detail is fetched from the tracker for that request; a failed fetch +shows “tracker unavailable” while the durable workflow remains inspectable. + +Raw webhook bodies and fetched tickets must not leak into a generic durable inbox or timeline. +Persist only normalized control facts. Notes that may quote ticket/code text, gate notes and outbox +payloads carrying such text are Tink-encrypted with item/gate/effect AAD. Clear identifiers and +reason codes remain queryable. Existing run task storage retains its encryption boundary. + +### 5.2 Labels have authors, removals and provenance + +Proposed `LabelEvent`: stable source event id, issue ref, label, `ADD|REMOVE`, tracker actor id, +occurred time, provider ordering token when available and origin `WEBHOOK|AUDIT_TRAIL|UNATTRIBUTED`. +The earlier sketch omitted removal and event identity; both are needed to prevent a replayed old +addition from resurrecting authority. The **applier of the current addition** matters, not the +issue reporter, assignee, most recent issue editor or person who created the label definition. + +Reconcile current labels with paginated label audit and verified webhook evidence. A later remove +invalidates earlier attribution; a re-add needs its own actor. An audit gap or ambiguous ordering +produces `UNATTRIBUTED`, never attribution borrowed from an earlier incarnation of the label. +Polling after downtime and initial backlog scan run the same policy path as webhook intake. +Checkpoint only after admission/reconciliation commits; redelivery is safe. Avoid resetting the +scan cursor on every restart or persisting fetched ticket content to make polling easier. + +Adapter implementation must verify GitHub issue timeline label events, GitLab resource label +events, and Jira changelog label deltas (including pagination and attribution) against their +documented contracts. Source capabilities report where audit or transitions are unavailable. +Unsupported audit is an honest loss of automation: present labels without a proven applier select +nothing. Deletion/move is distinguished from token outage; inaccessible is suspended, not retired. +A confirmed transfer retires the old item, closes gates, cancels unstarted effects and requires +explicit admission under the new repository's policy; it never silently continues. + +## 6. Policy, versions and the phase boundary + +### 6.1 Named precedence and bounded vectors + +Store `autonomy_profile` and immutable `autonomy_profile_version` rows, repository ceiling and +label mappings in the operator registry. Profiles carry the eight phase modes, gate TTL, run/step/ +wall-clock/cost/call caps and protected paths. Omitted phases are `off`. Validate phase-specific +vocabulary: ordinary phases `off|approve|auto`, deliver `off|draft_pr|pr`, land +`off|approve|auto_if_green`. Reject unknown modes, invalid caps, absent referenced versions and +labels mapped to deleted profiles. Unknown wire modes render unknown/refused, never green. + +“Lowest wins” needs an order; names are not comparable and vectors can be incomparable. Even the +published examples cross: suggest has `plan:auto`, assisted has `plan:approve`. A globally monotone +chain would reject those examples. Propose an explicit unique precedence number per profile, +owned/versioned by the operator, for selecting among labels and identifying an above-ceiling label. +The three examples order suggest, assisted, autonomous; their names are not dispatch cases. + +Precedence never substitutes for a permission bound. Effective modes are the component-wise meet +of selected, pinned and ceiling vectors: `off < approve < auto` for ordinary phases, +`off < draft_pr < pr` for delivery and `off < approve < auto_if_green` for land. Numeric maxima +take the minimum; protected paths take the union plus the immutable CI floor. No glob containment +solver is required. Where the result is a composite vector, display the selected profile and the +limiting ceiling plus actual phase modes, not a claim that it equals an unmodified named profile. +Reject duplicate precedence, missing versions and invalid modes; accept cross-cutting vectors only +with this meet. Analyst acceptance of the ordering/composition rule is needed (§11). + +Initial examples explicitly declare `intake: auto`; copying the abbreviated AUTONOMY YAML without +that field would correctly default intake to off and admit nothing. Use these complete vectors: + +| Profile | intake | spec | plan | build | verify | review | deliver | land | +|---|---|---|---|---|---|---|---|---| +| suggest | auto | auto | auto | off | off | off | off | off | +| assisted | auto | auto | approve | auto | auto | auto | draft_pr | approve | +| autonomous | auto | auto | auto | auto | auto | auto | pr | auto_if_green | + +These are desired permissions, not claims that M4 executors exist. Missing verify/review/land +capabilities still block their transitions; the M3 journey proof must show that boundary honestly. + +At admission pin the chosen profile id/version and the mapping revision used. Compute the most +restrictive eligible label selection and the current ceiling; persist requested/effective selection +and why a clamp occurred. At later transitions re-read current labels, source allowlist, enabled +states, mappings and ceiling. Intersect the admitted version with current restrictions. A removed +label, disallowed applier or new lower label can narrow/stop an item. A higher label, raised ceiling +or edited version cannot widen its admitted authority. An operator explicitly re-admits to move to +a new version/generation. A lower ceiling's current version restricts the pinned vector; it never +silently replaces the admitted version's more restrictive fields. + +Store the applied policy revision and provenance at each decision so the screen can explain it. +Ceiling clamps produce a durable timeline entry and condition-based attention row while the +current selection is clamped. Deduplicate repeated observations of the same clamp. Invalid labels +each have an ignored reason; if another valid label remains, it can select a profile. If none +remain, `not_eligible` stops automation with the specific underlying reason visible. + +### 6.2 Every transition is a real check + +One `WorkItemTransitions` service is called for admission, phase completion, gate approval, +retry, run-result continuation, delivery, land, operator resume and explicit re-admission. +Expiry, takeover and retirement invalidate pending effects as well. Scheduled/outbox retries +cannot bypass this service. Fetch external evidence outside a DB lock, then compare its source/ +repository/policy revisions under lock; stale or failed reads cause waiting, not permission. +External revocation and a local dispatch cannot be globally atomic: the guarantee is a fresh +observation at each transition, not instantaneous revocation of a push already accepted remotely. + +A lowered ceiling stops advancement at the next phase. If its mode becomes `approve`, open a new +version-bound gate before proceeding; if `off`, record `not_eligible` and stop. It need not kill +the phase already running. A gate approved against old policy or an older artifact/head is stale +and cannot authorize the new transition. Failed verify is not item success; M4 owns retries and +step verification. Budget limits narrow existing SpendGate/FR-F32 checks and include call count +on unmetered deployments. Reserve a dispatch slot atomically and release it on refusal/expiry; +do not claim hard monetary reservations eliminate the documented in-flight spend softness. + +### 6.3 M3 journeys versus M4 execution — analyst decision required + +FR-F17 spans M3/M4. M4 explicitly owns generating specifications/plans, multi-step execution and +verification. M3 cannot label no-op phase handlers “complete” to manufacture three green journeys. +Proposed M3 boundary: implement the real phase state machine and manual tracker-artifact handoff, +then reuse M2 for **one already specified build task**. Humans can register references/digests to +a specification and a single-step plan actually present in the tracker. Those artifacts are fetched +and validated; they are not copied into `work_item`. Missing execution capabilities show +`awaiting_input` or `capability_unavailable`, never a fake successful phase. + +With the same prepared task and three profile labels the runnable control-plane proof is: + +| Profile | Visible journey in M3 | +|---|---| +| suggest | Admit; record the human-provided specification/plan references; stop before build (`off`), zero runs, no PR. | +| assisted | Admit; visibly wait on a durable plan gate with zero runs. Approval admits one build; then wait for any missing verification capability. Its eventual permitted delivery is a draft PR. | +| autonomous | Admit; plan proceeds without approval and starts one build immediately; then wait for any missing verification capability. Its eventual permitted delivery is a regular PR. | + +No `auto_if_green` implementation or automatic tracker closure is implied by a green unit test. +Criterion 1 is proved at the real plan/build boundary: suggest stops, assisted waits for approval, +autonomous builds. After approval, assisted's history still records its distinct human decision. +Draft/regular delivery tests use an explicitly identified test phase driver to supply verification +evidence; this is adapter/control-plane coverage, not proof of a shipped M4 verifier. Production +with no verifier remains waiting. If the analyst interprets criterion 1 as generated +specification through automatic merge, that moves named M4 work into M3 and must amend the plan +before slice 8. The proposal above is conditional, not an assertion that the ticket chose it. + +Delivery/review ordering also needs explicit correction: the published eight-phase diagram places +review before deliver, but the existing reviewer requires a pushed PR. Proposed execution records +PR opening as the delivery effect, then observes the existing reviewer before any land decision; +it does not report a review that could not have run. Preserve phase identifiers in the policy +vector, but settle execution order in ADR-045 rather than hiding the mismatch in a handler. + +**Publication is part of that decision too.** M2 builds already push before `RunFinished`; merely +gating the later PR API call cannot enforce a deliver mode of off. Proposed item-linked execution +starts with publication held, checkpoints local work and emits a durable `RunWorkReady` integration +result before push. The worker releases active compute while preserving the workspace and a +durable awaiting-delivery record. Only a current, item/generation-bound delivery permit resumes +trusted publisher finalization; it cannot rerun the build or accept a repository-authored permit. +Standalone M2 runs retain their existing automatic push. Expiry/retirement/takeover leave work +preserved without publishing, and orphan recovery honors the hold. This introduces a run state +and control/result messages, not a run aggregate. The design must establish charge reporting at +work-ready/final completion without double counting, and the artifact/verification boundary before +granting delivery. It is explicit additional work in slice 8, conditional on the analyst accepting +this execution boundary; the current M2 worker cannot be described as already supporting it. + +## 7. Durable approvals + +`work_item_gate` records id, item/generation, phase, expected item/policy version, artifact digest +or PR head, opened/expiry timestamps, status (`OPEN|APPROVED|REJECTED|EXPIRED|SUPERSEDED`), resolver +stable identity/channel, deduplication key and encrypted note. One current open gate per item, +generation and phase. It is a synchronous transactionally maintained query of aggregate state; +the event log remains rebuildable truth. Concurrent responses use expected version and a +conditional OPEN transition. One wins; replay of its idempotency key returns the stored outcome; +a conflicting answer returns 409. At `now >= expiresAt` expiry wins even if a scheduler is late. + +| Channel | Authority and binding | +|---|---| +| Dashboard | Existing authenticated operator authorization (`spire-admin` for gate mutations initially); server derives resolver from verified OIDC subject. Viewer can read only within existing access rules. | +| Tracker | Allowlisted actor in this work source; authenticated delivery; explicit command such as `/approve ` or `/reject ` binds the generation and artifact. Ordinary comments do not approve. | +| PR review | Current approval of this linked PR's current head by a human with measured repository push permission and no deny override. It can answer only a land gate, never a plan gate. Re-read review state; dismissed/stale approvals do not count. | + +All channels become the same `ResolveGate` command and `GateResolved` milestone. Tracker label +answers are deferred unless a gate-specific label can carry unambiguous generation and attributable +actor; “a comment or a label” does not require implementing an unsafe generic approve label. +If a forge cannot prove a PR approval, its capabilities disable that channel visibly; dashboard +and tracker remain usable. Expiry is a persisted `WorkItemRefused(gate_expired)` with reservation +release in the same transaction. A restarted sweeper expires overdue gates. Retrying a refused +item needs explicit re-admission, not reopening the old approval. + +## 8. Human takeover + +Signed push/PR activity on an item-linked branch/PR records `human_takeover` and suspends new +automation until an operator resumes. Compare the actor's stable id against the item's recorded +factory identity and assigned reviewer identity, not display names, author strings in commits or +the current factory account after it has been rotated. Unknown origin suspends conservatively. +Repo/head links must match; unrelated branches cannot suspend an item. A bot's observed push +does not count as a person, even after an account has been renamed. + +Gate answers and authorized `/fix` commands are deliberate workflow actions; classify and +deduplicate them before generic comment takeover. This is a proposed precedence rule resolving +FR-F22's literal “commenting” against FR-F25's tracker/PR answer channels; record it in ADR-045. +Normal human comments and pushes take over. A PR approval is processed as a gate response only +when it actually matches an open gate; it cannot accidentally resume a suspended item. + +Takeover cancels unstarted outbox effects, supersedes pending gates and requests active-run stop. +**Normal M1 cancel salvages and may push. It is insufficient for takeover.** Introduce a durable +publication hold for item-linked runs: preserve local work while suppressing further pushes and +PR creation after the hold is observed. Carry the hold through run control, worker durable state, +publisher finalization and orphan salvage; a restart must not restore publication authority. +Publication already in progress cannot be recalled; record its outcome and keep the item +suspended. Never claim atomic ordering between a remote human push and our webhook receipt. +The run-plane mechanics and the interaction with continuous checkpoints need explicit tests in +slice 9, not just a saga fake. Existing standalone cancellation retains its salvage contract. + +Resume is an authenticated operator action with expected version and a note. Re-fetch repository/ +issue/head, re-resolve policy and open any new gate before continuation. A retired item cannot +resume; it requires a new identity/admission. No automatic resume on a bot comment or on a new label. + +## 9. Screens, resources and observability + +| Route / API family | Behavior | +|---|---| +| `#/settings/repositories`, `/api/repositories` | Repository registration/detail, workspace, exact role bindings, per-kind hooks, source/policy setup and fix overrides. Non-secret account views. | +| `#/settings/accounts`, `/api/providers` | Identity, credential, scalar role and Used by; no workspace control. Handle entry renders people, not a count alone. | +| `/api/work-sources` | Admin source registration, actor allowlist, Check and bounded rescan. Uses an existing account, never a second token form. | +| `/api/autonomy-profiles`, repository policy subresource | Versioned profiles, mappings and ceiling; optimistic revisions on edits. | +| `#/work-items`, `/api/work-items` | Paged durable list by repository/source/workflow state/profile. Detail: tracker link, current phase, requested/effective profile, ignored labels, clamp reason, gates and links to real runs/PR/review. | +| `#/approvals`, `/api/approvals` | Open approvals with expiry, phase, artifact/head and decision note; authorized approve/reject. Separate history query for resolved gates. | +| Existing attention API | Current open gates, effective clamps, unknown permission/account mapping and failed source health. Resolving the condition removes its row. | + +Resources enqueue durable commands and return 202 plus a command/item id; they do not hold an HTTP +request open for a phase. Configuration writes retain ordinary synchronous registry semantics. +Read APIs expose a revision for bounded polling/live updates and explicit errors on source fetch +failure. UI statuses, labels, pipeline renderer, filters and unknown-state handling land together. +Keep new React components below 250 lines and eight state hooks, using existing controls/icons. +Never label a scheduled test fixture or an unsupported phase as a live completed item. + +## 10. Proof strategy and excluded work + +The [plan's acceptance matrix](../plans/2026-09-12-factory-m3-work-items.md#acceptance-proof-matrix) +names an executable test for each of the seven ticket criteria and an isolated, compiling mutation +that must kill exactly one discriminating test. It also covers migration, replay, concurrency, +expiry, transfer, channels, takeover and missing capabilities. Test names are proposed additions; +none are represented as tests that exist or have passed today. + +M3 excludes M4-generated specification/plan, multi-step continuity, repository verification runners, +automatic merge implementation without a separately accepted scope decision, model quality claims, +new runtime/harness arms, a product-owner role, context auto-discovery, account-role merging and +general remediation of historical review-id host collisions. It includes the seams and honest +unavailable states needed so those features can arrive without bypassing policy. + +No production code, migrations, dev data, Gradle execution or runtime restarts belong to Round 1. +Commit only this design and its plan, push the existing branch, open the requested draft PR. + +## 11. Questions the analyst must settle + +These do not block publishing a reviewable design. They do block silently choosing product +behavior in the dependent implementation slice. + +1. **Three journeys and phase ordering:** accept the prepared single-task/manual-artifact M3 + boundary in §6.3, or move named M4 capabilities into M3? Also reconcile review-before-deliver + with the existing PR-triggered reviewer. Fake completions are not an option. +2. **Profile ordering:** accept explicit operator-owned precedence plus a component-wise meet, or + specify another rule for incomparable vectors. Even suggest/assisted cross at the plan phase; + the ticket's “lowest” is undefined without a separate precedence rule today. +3. **Identity/permission portability:** Bitbucket's effective permission read requires an admin + caller; exact handles are not a universal Jira/Bitbucket identity primitive. Accept explicit + capability errors/disambiguated selection and that credential prerequisite, or narrow the + acceptance wording. Do not grant access by guessing or silently raise a token's authority. +4. **Repository registration compatibility:** confirm one source/ISSUE hook per repository and + REVIEWER/FACTORY/ISSUE as product event kinds. Decide whether legacy org auto-enrollment is + retained beyond migration; otherwise existing installations lose that convenience at cutover. +5. **Drafts and takeover:** require provider-native drafts wherever available and visibly refuse + unsupported `draft_pr` (never substitute a title prefix), and accept explicit command/approval + precedence over generic human-comment takeover. Native draft support and publication-hold + mechanics, including the proposed work-ready/delivery handshake in §6.3, must be established + in the relevant provider/runtime slices before claiming parity. + +The decisions above make the proposal concrete for review; unresolved alternatives are recorded +here instead of being disguised as ticket requirements. From ded7404e75149d34abb2b274c75bbe5840b63b6d Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Sun, 13 Sep 2026 00:20:35 +0200 Subject: [PATCH 02/41] Apply the Factory M3 planning review Retain workspace evidence until the final slice, require a validated dev backup and real credential continuity proof, and separate publication changes into slice 8b with a live standalone fix regression. Record the accepted policy, enrollment and journey decisions and move stale M2 evidence corrections into slice 1. --- .../plans/2026-09-12-factory-m3-work-items.md | 119 +++++++++++++++--- ...2026-09-12-factory-m3-work-items-design.md | 111 +++++++++------- 2 files changed, 166 insertions(+), 64 deletions(-) diff --git a/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md index b14cd17d..955122e2 100644 --- a/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md +++ b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md @@ -2,7 +2,7 @@ **Date:** 2026-09-12 -**Status:** Round 1, plan only. All implementation checkboxes and test outcomes below are pending. +**Status:** Accepted with Round 2 amendments; implementation checkboxes and test outcomes remain pending until measured. **Goal:** Start factory work from a tracker ticket with explicit, bounded autonomy; make repository ownership, command authority and approval state visible and durable. @@ -75,10 +75,13 @@ or interface-only commit can exist inside a slice, but is not its review exit. | 5 — First ticket, durable intake and suggest policy | 2, 3 | A signed issue label or rescan admits one durable item, visible on Work items; unknown/unlisted actors select nothing. | GitHub source, minimal profile registry, lifecycle/store/outbox and UI. | | 6 — GitLab/Jira sources and recovery | 5 | Each source can admit a real fetched ticket through webhook or polling with the same actor rule. | Adapter parity, safe tracker writes and restart-safe scanning. | | 7 — Full policy and dashboard approvals | 5 | Label changes and ceiling edits affect the next phase; a dashboard gate survives restart, resolves or expires. | Complete profile vector, transition checks, gates, Approvals and attention. | -| 8 — Prepared task to policy-controlled build/delivery | 4, 6, 7; design §11.1 resolved | Three labelled prepared tasks stop, await approval or build; missing later capabilities stay visibly waiting. | Artifact handoff, dispatch/result join and draft/regular delivery support. | -| 9 — External answers and human takeover | 8 | Tracker/PR answers resolve the same gate; human activity suspends automation and holds publication through restart. | Authenticated ingress, gate channel adapters, run control/publisher hold and resume. | +| 8a — Prepared task to policy-controlled build | 4, 6, 7 | Three labelled prepared tasks stop, await approval or build; missing later capabilities stay visibly waiting. | Artifact handoff and dispatch/result join. | +| 8b — Publication hold and draft delivery | 8a | Item-linked runs await a current delivery permit; standalone `/fix` still pushes automatically, re-proved live. | Worker/publisher/watchdog hold, native draft delivery and standalone regression proof. | +| 9 — External answers and human takeover | 8b | Tracker/PR answers resolve the same gate; human activity suspends automation and holds publication through restart. | Authenticated ingress, gate channel adapters, run control/publisher hold and resume. | | 10 — Integrated evidence and release documentation | 1–9 | Acceptance proofs and mutation evidence are recorded; supported journeys demonstrated and remaining limitations named. | Final integration tests, runbook and measured status updates. | +The §11.1 review dependency is discharged. Slices 9 and 10 retain their numbers; 8b sits between 8a and 9. + Slices are sequential review boundaries, not a request to launch parallel test runs or delegated work. Some dependencies are independent for scheduling, but this plan requires no additional pane. @@ -139,7 +142,50 @@ outbox; first repository UI; ADR-042 draft in `docs/DECISIONS.md`. **Produces:** `RepositoryAccounts.resolve(repositoryId, role)` and a non-secret serving view; `POST/GET /api/repositories`; versioned metadata-only registration snapshots. -- [ ] Settle the org-migration choice in design §11.4. Write failing migration/service tests: +- [ ] **Before any new migration reaches dev:** take a full `pg_dump`, validate the archive and + record its path/hash. Use the exact PowerShell commands below; the binary dump never passes + through PowerShell text redirection. The running stack holds real accounts, encrypted source + credentials and review/run history. Preserve its existing matching keyset securely outside git. + +```powershell +$m3Scratch = 'C:\Users\artjo\AppData\Local\Temp\claude\E--Projects-Stukans-code-spire-worktrees-feat-software-factory\5f317e7d-1b64-4305-bf1c-e3a370b07eb1\scratchpad' +$m3Dump = Join-Path $m3Scratch ('m3-before-slice1-' + (Get-Date -Format 'yyyyMMdd-HHmmss') + '.dump') +$m3Process = [Diagnostics.Process]::new() +$m3Process.StartInfo = [Diagnostics.ProcessStartInfo]::new('docker') +$m3Process.StartInfo.UseShellExecute = $false +$m3Process.StartInfo.RedirectStandardOutput = $true +@('exec','spire-postgres','sh','-c','exec pg_dump -U "$POSTGRES_USER" -d "$POSTGRES_DB" -Fc') | ForEach-Object { $m3Process.StartInfo.ArgumentList.Add($_) } +$m3File = [IO.File]::Create($m3Dump) +try { [void]$m3Process.Start(); $m3Process.StandardOutput.BaseStream.CopyTo($m3File); $m3Process.WaitForExit(); if ($m3Process.ExitCode -ne 0) { throw 'pg_dump failed' } } finally { $m3File.Dispose(); $m3Process.Dispose() } +Get-FileHash -LiteralPath $m3Dump -Algorithm SHA256 +``` + +Validate archive listing with `pg_restore --list` using a one-shot container with only this +scratchpad mounted read-only, without changing the running stack: + +```powershell +docker run --rm --mount "type=bind,source=$m3Scratch,target=/backup,readonly" postgres:18.4-alpine pg_restore --list "/backup/$([IO.Path]::GetFileName($m3Dump))" +if ($LASTEXITCODE -ne 0) { throw 'Backup archive validation failed' } +``` + +- [ ] Add `scripts/verify-dev-credential-continuity.ps1` as the read-only local operational probe. + It uses the actual dev keyset and `EncryptionService`, with `provider:` for account secrets + and `context-provider:` for remaining legacy context secrets. Capture encrypted baseline + evidence (including source→account references) into scratch; Compare re-decrypts actual rows + and checks equality in memory. Never write/log plaintext, keysets or unkeyed secret hashes. + Execute Capture before migration; slice 2 must execute Compare on the real dev rows: + +```powershell +.\scripts\verify-dev-credential-continuity.ps1 -Mode Capture -Snapshot (Join-Path $m3Scratch 'm3-real-credentials.bin') +.\scripts\verify-dev-credential-continuity.ps1 -Mode Compare -Snapshot (Join-Path $m3Scratch 'm3-real-credentials.bin') +``` + +- [ ] Reconcile `CLAUDE.md` and `docs/UNVERIFIED.md` **in this slice**: the live M2 chain on + `artyomsv/spire-test#31`, runs `3987682681:1` and `3987682176:1`, resolved threads and persisted + verdicts was measured on 2026-09-12. Keep the automated GitLab gap as its own open entry: + `RunUnitSpec` has no network field, so run units cannot reach that test stack's GitLab. + +- [ ] Apply the accepted bridge-only org enrollment decision. Write failing migration/service tests: `RepositorySchemaMigrationTest.preservesAccountIdsCredentialsAndContextReferences`, `RepositoryMigrationBridgeTest.replaysGatewaySnapshotWithoutDuplicateBindings`, `RepositoryMigrationBridgeTest.leavesConflictingOriginsPending`, and @@ -180,11 +226,15 @@ new API/form; gateway key plus scope plus event-kind validation. `providers.resolve`, `MachineAccounts.resolve` and serving API usages. Include manual/rerun, prompt, fix and result-time PR proposal paths, not only the HTTP run endpoint. - [ ] Drop the old UNIQUE and workspace-by-role CHECK; finish migration mappings, remove active - account workspace access and then its column per the accepted bridge schedule. Retain scalar + account workspace reads. Keep the populated column as rollback evidence until slice 10. Retain scalar role checks. Refuse origin/type edits on referenced accounts. Do not add global role uniqueness. - [ ] Upgrade all three keyed SCM edges to product event-kind filtering. Preserve keys, secrets, scope and rejection history during migration. Wire FACTORY activity separately from REVIEWER - commands; reserve ISSUE scope for source registration. Unknown kinds fail closed. + commands; reserve ISSUE scope for source registration. Unknown kinds fail closed. End org + auto-enrollment. Add `UnregisteredRepositoryAttentionTest.namesRepositoryOriginAndRegistration`: + a verified event for an unregistered repo raises attention and a Register action pre-filled + with repo, origin and incoming registration. Mutate its attention write, isolate the test, + expect exactly one failure, restore; a silent drop does not satisfy cutover. - [ ] Replace the webhook-row screen with repository detail and one hook control per kind. Support registration without hooks, retries after partial save and legacy org/deep-link navigation. Remove workspace from `ProviderInput`/`View` and `AccountCredentialFields`, not merely hide CSS. @@ -193,6 +243,12 @@ new API/form; gateway key plus scope plus event-kind validation. the same provider and a valid signature so a different guard cannot mask the mutation. - [ ] Run migration, gateway, orchestrator and UI verification in sequence. Search for remaining active workspace-only lookups; historical docs/bridge mappings are the only permitted matches. + Add `spire-arch/AccountWorkspaceIsUnusedTest.noProductionCodeReadsLegacyAccountWorkspace`: + inspect production source/SQL including SELECT-star mappings so retained workspace cannot silently + re-enter resolution. Mutation: restore a workspace read in the repository resolver; exactly that + targeted test fails. Restore from scratch. Confirm real dev-row credential continuity with the + Compare command below after cutover; compare all baseline account ids and context references, + report added/removed rows separately, and never accept fixture-only evidence. Update serving API/upgrade contracts and commit the runnable cutover. ## Slice 3 — resolve a person with the selected account @@ -203,7 +259,7 @@ override registry, person controls; ADR-044 identity half. **Produces:** exact identity resolution with typed errors; stable-id persisted allowlists and `ALLOW|DENY` repository fix overrides with readable display metadata. -- [ ] Settle capability wording for Bitbucket/Jira person lookup before claiming universal handles. +- [ ] Apply accepted capability errors and disambiguated selection for Bitbucket/Jira person lookup. Write criterion 6 tests and `ActorResolutionResourceTest.usesOnlyTheSelectedAccountsCredential`, `ActorResolutionResourceTest.refusesAnAmbiguousMatch`, `ActorResolutionResourceTest.rechecksSubmittedIdentityOnSave`, @@ -219,7 +275,7 @@ override registry, person controls; ADR-044 identity half. - [ ] Kill criterion 6 mutations, then separately kill account credential selection and returned-id verification with the corresponding targeted tests. Restore and rerun each case green. - [ ] Run adapter unit tests, resource persistence test and UI form round-trip; update SCM-MAPPING - identity capability notes and commit. At this exit overrides are editable; slice 4 activates + identity capability notes and per-forge UNVERIFIED entries (what was measured, which forge, and remaining proof), then commit. At this exit overrides are editable; slice 4 activates their new permission fallback without changing unrelated review policy. ## Slice 4 — measure repository push access for `/fix` @@ -246,7 +302,7 @@ target, identity, observe-mode, spending and fix-chain guards. validation. Known stale PR-state/shared-branch debt stays documented unless explicitly fixed. - [ ] Kill each criterion 5 mutation in its isolated method, then run the class green. Exercise all three adapter contracts and inherited-access cases; record live-token limitations without - changing the operator's account privileges. Commit the authorization slice. + changing the operator's account privileges. Add each per-forge permission behavior as its own UNVERIFIED entry with measurement and forge. Commit the authorization slice. ## Slice 5 — admit the first ticket and display durable bookkeeping @@ -317,7 +373,7 @@ Approvals screen; ADR-045 policy decision. **Produces:** checked profile vectors, visible clamps, current-policy phase decisions, durable gates, expiry and operator answers. -- [ ] Resolve the profile ordering choice with the analyst. Write criterion 2 and 4 tests plus +- [ ] Apply the accepted profile precedence and meet rule. Write criterion 2 and 4 tests plus `AutonomyProfileTest.requiresDistinctProfilePrecedence`, `AutonomyProfileTest.incomparableVectorsMeetWithoutWideningEither`, `AutonomyProfileTest.omittedPhaseIsOff`, @@ -325,7 +381,7 @@ expiry and operator answers. `WorkItemPolicyIT.profileEditCannotWidenAnAdmittedVersion`, `WorkItemPolicyIT.removedLabelStopsTheNextTransition`. - [ ] Implement versioned vector/precedence validation, current allowed label selection, pinned - admission version and component-wise restriction. Record selection/clamp/reason with policy revision. + admission version and component-wise restriction across EVERY eligible label, not only the lowest-precedence label. Record selection/clamp/reason with policy revision. Include source disabled/allowlist removed, stricter caps and protected-path floor cases. - [ ] Call the transition service from every entry point named in design §6.2. Re-read evidence outside the transaction and compare registry revision inside it; stale external data must not @@ -343,7 +399,7 @@ expiry and operator answers. - [ ] Run contract, orchestrator, UI tests sequentially; demonstrate restart and ceiling downgrade through real APIs in the test stack; update ADR-045 and commit. -## Slice 8 — connect policy-controlled work to the delivered run path +## Slice 8a — prepared task to policy-controlled build **Files:** artifact reference handoff, dispatcher, run-result bridge, item/run FK metadata, `FactoryPullRequests`, `PullRequestSink` and all three arms, work-item UI. @@ -351,9 +407,10 @@ expiry and operator answers. **Produces:** criterion 1's accepted M3 journeys; actual one-task build and policy-aware delivery boundaries. No production verifier is invented to reach the delivery test cases. -- [ ] **Before coding, resolve design §11.1/§11.5.** Confirm prepared manual artifacts versus M4 - scope, execution ordering of review/deliver, and draft capability behavior. Update this task's - exact journey expectations if the analyst changes the boundary; never use no-op phase success. +- [ ] Apply the accepted manual-artifact/plan-build proof boundary. Missing capabilities remain + waiting. Correct the eight-phase diagram in `docs/factory/AUTONOMY.md` and affected PRD/ + architecture diagrams in this slice: `intake → spec → plan → build → verify → deliver → review + → land`. Record the order in ADR-045; its acceptance dependency is discharged. - [ ] Write `WorkItemJourneyIT.threeProfilesProduceDifferentVisibleJourneys` and UI journey test from the acceptance matrix. Use real persisted policy/source/item data; scripted execution is permitted only in tests and identified as such. Also write @@ -366,6 +423,22 @@ boundaries. No production verifier is invented to reach the delivery test cases. - [ ] Persist an effect claim before dispatch, recheck current policy before publishing and make run/result association recoverable after crash. Do not reset attempts on re-admission or charge the same run result twice. Existing standalone runs remain outside work-item gates. +- [ ] Commit the artifact handoff and dispatch/result join for review. Before 8b provides a + trustworthy publication hold, item-linked real execution stays capability-unavailable; the + runnable 8a policy proof uses the explicit test execution boundary, never an auto-pushing M2 + run advertised as held. Slice 8b closes the real-container execution proof. + +## Slice 8b — publication hold and draft delivery + +**Depends on:** 8a. Slices 9 and 10 keep their numbers. + +**Two-part exit:** item-linked runs hold publication until a current delivery permit, through +restart and orphan recovery; **standalone `/fix` still pushes automatically**, re-proved live on +`artyomsv/spire-test`. Unit/fixture tests do not discharge the second half. + +**Files:** work-ready/control/results, worker durable state, publisher/runtime finalization, +orphan salvage, item delivery orchestration, sink draft support and UI states. + - [ ] Implement the accepted work-ready/delivery-permit handshake from design §6.3. An item-linked run starts with publication held, checkpoints without pushing, persists awaiting-delivery and releases active compute. Resume only the trusted publisher on a current delivery permit; keep @@ -390,7 +463,13 @@ boundaries. No production verifier is invented to reach the delivery test cases. - [ ] Kill criterion 1 mutations and the standalone-proposal bypass separately. Run relevant run, orchestrator, sink adapter and UI suites sequentially. Also remove the initial publication hold and isolate `WorkItemDeliveryIT.deliverOffNeverPushesTheBuiltBranch`: exactly one test must fail - on the real remote's changed head. Restore and rerun green. Commit the complete runnable journey. + on the real remote's changed head. Restore and rerun green. +- [ ] Re-prove a standalone `/fix` live on `artyomsv/spire-test`, using an actual open finding and + the same command → worker → publisher → next review → resolved thread/persisted verdict chain + proved by runs `3987682681:1` and `3987682176:1`. Record actual new run/PR ids, source head before/ + after and verdict observations. A synthetic fixture or unit test is not this proof. Announce + any TEST-/CANARY-prefixed setup and its exact cleanup first. Do not close 8b without this result. +- [ ] Commit 8b independently after both exit obligations pass. ## Slice 9 — answer outside the dashboard and take over safely @@ -438,7 +517,7 @@ Every integration proof has a visible UI assertion or a matching component test | # | Ticket exit criterion and exact proof | Production mutation and isolated expected failure | |---|---|---| -| 1 | **Three visibly different journeys.** `O-test/workitem/WorkItemJourneyIT.java#threeProfilesProduceDifferentVisibleJourneys`: label three otherwise identical `TEST-` prepared tasks under suggest/assisted/autonomous with ceiling autonomous, inspect persisted timeline/API: suggest stops before build; assisted waits at plan approval with zero runs; autonomous starts one build without approval. Then approve assisted and assert its recorded human decision and single dispatch. Assert exact phase/gate/effect fields; missing verify remains waiting. `U/components/workItems/WorkItemJourney.test.tsx` → `renders distinct suggest assisted and autonomous journeys` proves visible differences. Draft/regular PRs are separate delivery tests with a test-only prior-phase driver. Scope conditional on slice 8's explicit analyst decision. | First mutant: in the real transition policy branch change `approve` to proceed without opening its gate; run only the Java method, expect exactly one failed test. Restore. Second mutant: render all journey status labels as the same label; run only the named vitest case, expect one failure. Restore. A test that only compares three profile names is insufficient. | +| 1 | **Three visibly different journeys.** `O-test/workitem/WorkItemJourneyIT.java#threeProfilesProduceDifferentVisibleJourneys`: label three otherwise identical `TEST-` prepared tasks under suggest/assisted/autonomous with ceiling autonomous, inspect persisted timeline/API: suggest stops before build; assisted waits at plan approval with zero runs; autonomous starts one build without approval. Then approve assisted and assert its recorded human decision and single dispatch. Assert exact phase/gate/effect fields; missing verify remains waiting. `U/components/workItems/WorkItemJourney.test.tsx` → `renders distinct suggest assisted and autonomous journeys` proves visible differences. Draft/regular PRs are separate delivery tests with a test-only prior-phase driver. Scope accepted in Round 2; runtime publication proof belongs to 8b. | First mutant: in the real transition policy branch change `approve` to proceed without opening its gate; run only the Java method, expect exactly one failed test. Restore. Second mutant: render all journey status labels as the same label; run only the named vitest case, expect one failure. Restore. A test that only compares three profile names is insufficient. | | 2 | **Above-ceiling label clamped and says so.** `O-test/workitem/WorkItemPolicyIT.java#aboveCeilingLabelRecordsAndDisplaysClamp`: request autonomous at assisted ceiling; assert effective vector, durable clamp event after reload, attention API row and detail reason. `U/components/workItems/WorkItemPolicy.test.tsx` → `shows requested and effective profiles with the clamp reason`. | Mutate the effective-profile meet to retain requested authority; isolate Java method, one failure. Restore. Separately omit the clamp event/attention projection write; same isolated method must fail once. Restore. Mutate the UI clamp message to empty and run the named UI case for one failure. Each proves a different half of “and says so.” | | 3 | **Unlisted and unattributable appliers ignored.** `O-test/workitem/WorkItemIntakeIT.java#unlistedLabellerSelectsNoProfile` and `#unattributedCurrentLabelSelectsNoProfile`, each using a mapped label that would otherwise dispatch, valid source/account/ceiling and assertions of ignored reason plus no run effect. Add `#allowedAttributedLabellerCanSelect` as positive control. | Delete the actor-membership check; run only `unlistedLabellerSelectsNoProfile`, one failure. Restore. Delete the attribution check; run only `unattributedCurrentLabelSelectsNoProfile`, one failure. Fixture for the latter has an actor hint that would pass membership but origin UNATTRIBUTED, so the origin guard alone distinguishes it. Use additional no-id test for real missing actor. Never combine both negative cases into one count. | | 4 | **Lowering ceiling stops an in-flight item at next phase.** `O-test/workitem/WorkItemPolicyIT.java#loweredCeilingStopsAnInFlightItemAtTheNextPhase`: admit and start under higher policy, commit a lower ceiling with next phase off, then send the prior phase's result. Assert no next effect/PR and persisted stop/reason on detail. `#ceilingChangeBeforeGateAnswerRequiresANewDecision` covers waiting items. | Replace the transition's current ceiling lookup with admission-time ceiling, preserving all other checks; run only the first method, one failure. Restore. Separately bypass policy recheck in gate resolution and isolate the second method, one failure. The mutation must not be masked by also removing a label or disabling an account in the fixture. | @@ -507,6 +586,10 @@ this table with its discriminating witness before that slice is called complete. ## Slice 10 — integrated proof and handoff +- [ ] Only now drop `scm_provider.workspace` in an explicit Flyway migration. Confirm its + populated evidence survived slices 2–9 and the no-production-read guard remained green. + Retain the validated pre-migration dump/credential proof as rollback evidence outside git. + - [ ] Run `testFast --rerun-tasks` and, after it exits, `testServices --rerun-tasks`. Verify every new module is included in its proper tier. No parallel second Gradle invocation. - [ ] From `spire-ui`, run `npm test` and `npx tsc --noEmit`. Check CSS contracts, routes/deep links, @@ -527,7 +610,7 @@ this table with its discriminating witness before that slice is called complete. credentials and exact remaining proof gaps. Do not mark acceptance complete if a required criterion lacks evidence; bring that gap to the analyst. - [ ] Update `docs/DECISIONS.md` with accepted ADRs; reconcile factory architecture/topic/data - catalogues and M2 live-proof statements; update smoke-test instructions, registry upgrade steps, + catalogues; M2 live-proof reconciliation already landed in slice 1; update smoke-test instructions, registry upgrade steps, security/SCM mapping, factory roadmap and `docs/HISTORY.md`. Rewrite `CLAUDE.md` status/measured counts from actual results, preserving unrelated UNVERIFIED entries. - [ ] Submit each slice for analyst review and resolve findings with their tests/docs. The PR diff --git a/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md index c1dc9044..bd0834db 100644 --- a/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md +++ b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md @@ -2,7 +2,7 @@ **Date:** 2026-09-12 -**Status:** Round 1 proposal for analyst review; no implementation or test results claimed. +**Status:** Accepted with amendments in [Round 2 review](https://github.com/artyomsv/code-spire/pull/153#pullrequestreview-5188278775). Implementation and test evidence remain per-slice obligations. **Issue:** [#114](https://github.com/artyomsv/code-spire/issues/114), re-read from GitHub, updated `2026-09-12T21:47:45Z`. @@ -20,7 +20,7 @@ The ticket records the live M2 loop on `artyomsv/spire-test#31`, runs `398768268 `3987682176:1`: command, dispatch, push to the existing source branch, another review, resolved thread and persisted verdict. That is the issue's reported evidence, not a measurement made in this planning round. M3 is unblocked. The older contrary statements in `CLAUDE.md` and -`docs/UNVERIFIED.md` need reconciliation during implementation; the automated GitLab run-unit +`docs/UNVERIFIED.md` are reconciled in slice 1; the automated GitLab run-unit network gap remains a separate claim and must not be deleted on the strength of a live GitHub run. Read alongside [AUTONOMY](../../factory/AUTONOMY.md), [factory PRD](../../factory/PRD.md), @@ -50,7 +50,8 @@ The implementation plan names exact source roots for new files. ## 2. Decisions and ADRs -These are proposed decisions, not already accepted ADRs. Reserve the next available numbers at +These decisions are accepted by the review; ADR records land with the implementing slices. +Reserve the next available numbers at implementation time; `042`–`045` are the expected sequence after ADR-041. | ADR | Decision to record | Existing decisions affected | @@ -174,13 +175,17 @@ Flyway files (next orchestrator number is expected to be V60; verify before allo 3. Preserve org webhook coverage during the bridge. A verified event for a previously unseen repository may materialize a real repository using the snapshotted legacy assignment, with origin supplied by its registration. It must not inherit a different host or newer account by - workspace alone. An existing org scope is an explicit migration exception, not a new model - where an account owns every future repository. Whether to retain that convenience after the - bridge is an analyst question (§11). + workspace alone. Org auto-enrollment ends at cutover. Afterwards a verified event naming an + unregistered repository raises an attention row naming repository, forge origin and incoming + registration id, with a Register action pre-filled from those three. No silent drop and no + automatic inheritance of workspace accounts. One source/ISSUE hook per repository and the + REVIEWER/FACTORY/ISSUE kinds are confirmed. 4. Cut over every runtime resolver and the repository screen together after mapping checks pass. Unresolved repositories fail closed for action with a named repair path. Remove account - workspace input and validation, drop the old UNIQUE and workspace-by-role CHECK, then drop - the workspace column when the bridge no longer needs it. The migration-only snapshot is + workspace input and validation, drop the old UNIQUE and workspace-by-role CHECK, but keep + `scm_provider.workspace` populated with its existing values until slice 10 as rollback evidence. + No production code may read it after slice 2; a build guard enforces that rule. Slice 10 owns + the explicit column-drop migration. The migration-only snapshot is explicitly excluded from account selection after cutover. Preserve source credential recovery columns from V59; their removal is unrelated to this migration. 5. Update both packaged compositions, local dev configuration and upgrade instructions for the @@ -194,6 +199,13 @@ snapshot exchange. Prove identical credential decryption before and after, exact idempotent restart and refusal of ambiguous mappings. Do not exercise this on the running dev DB in a test. The bridge must preserve its existing webhook keys and encrypted secrets. +Before any new migration can reach the real dev stack, slice 1 takes and validates a full +`pg_dump` into the session scratchpad. The plan includes the exact binary-safe command. Preserve +the matching existing keyset outside git and capture a credential-continuity proof using real +rows and their actual Tink AADs. Slice 2 compares decrypted credentials against that baseline on +the real dev rows after cutover; matching fixture data or matching ciphertext alone is insufficient. +Only counts/ids and comparison outcomes are reported, never plaintext credentials or keysets. + ### 3.3 Repository screen and webhook model `#/settings/repositories` becomes a repository list and detail, not a list of webhook rows. Register @@ -300,7 +312,7 @@ particular protected branch accepts a push. The existing target and publisher gu grant the command. A 404 is not automatically proof that a person has no rights: adapters must distinguish an unreadable repository from a known absent member where the API permits it. Both refuse; the displayed reason differs. For Bitbucket a reviewer lacking the documented admin access -will report unknown; the analyst must settle whether that credential prerequisite is acceptable. +will report unknown; the accepted design requires an explicit capability error and an operator-facing credential prerequisite. Do not increase any live account's rights as part of implementation. ## 5. Work sources and label evidence @@ -394,7 +406,10 @@ take the minimum; protected paths take the union plus the immutable CI floor. No solver is required. Where the result is a composite vector, display the selected profile and the limiting ceiling plus actual phase modes, not a claim that it equals an unmodified named profile. Reject duplicate precedence, missing versions and invalid modes; accept cross-cutting vectors only -with this meet. Analyst acceptance of the ordering/composition rule is needed (§11). +with this meet. The review accepted this ordering/composition rule. **The effective vector is +never above any applied label in any component.** Here applied means current mapped labels with +proven, allowed appliers; ignored labels have no authority. Meet every eligible label's vector, +not just the lowest-precedence display selection, plus the pinned admission vector and ceiling. Initial examples explicitly declare `intake: auto`; copying the abbreviated AUTONOMY YAML without that field would correctly default intake to off and admit nothing. Use these complete vectors: @@ -441,11 +456,11 @@ step verification. Budget limits narrow existing SpendGate/FR-F32 checks and inc on unmetered deployments. Reserve a dispatch slot atomically and release it on refusal/expiry; do not claim hard monetary reservations eliminate the documented in-flight spend softness. -### 6.3 M3 journeys versus M4 execution — analyst decision required +### 6.3 M3 journeys versus M4 execution — accepted boundary FR-F17 spans M3/M4. M4 explicitly owns generating specifications/plans, multi-step execution and verification. M3 cannot label no-op phase handlers “complete” to manufacture three green journeys. -Proposed M3 boundary: implement the real phase state machine and manual tracker-artifact handoff, +Accepted M3 boundary: implement the real phase state machine and manual tracker-artifact handoff, then reuse M2 for **one already specified build task**. Humans can register references/digests to a specification and a single-step plan actually present in the tracker. Those artifacts are fetched and validated; they are not copied into `work_item`. Missing execution capabilities show @@ -464,15 +479,16 @@ Criterion 1 is proved at the real plan/build boundary: suggest stops, assisted w autonomous builds. After approval, assisted's history still records its distinct human decision. Draft/regular delivery tests use an explicitly identified test phase driver to supply verification evidence; this is adapter/control-plane coverage, not proof of a shipped M4 verifier. Production -with no verifier remains waiting. If the analyst interprets criterion 1 as generated -specification through automatic merge, that moves named M4 work into M3 and must amend the plan -before slice 8. The proposal above is conditional, not an assertion that the ticket chose it. +with no verifier remains waiting. The review accepted this plan/build-boundary proof; generated +specification, multi-step planning and verification executors remain M4 work. -Delivery/review ordering also needs explicit correction: the published eight-phase diagram places +Delivery/review ordering is corrected by the review: the published eight-phase diagram places review before deliver, but the existing reviewer requires a pushed PR. Proposed execution records PR opening as the delivery effect, then observes the existing reviewer before any land decision; it does not report a review that could not have run. Preserve phase identifiers in the policy -vector, but settle execution order in ADR-045 rather than hiding the mismatch in a handler. +vector; record `intake → spec → plan → build → verify → deliver → review → land` in ADR-045 and +fix the eight-phase diagram in `docs/factory/AUTONOMY.md` in slice 8a, together with affected +architecture/PRD diagrams. The diagram is wrong; the implemented reviewer is not changed to fit it. **Publication is part of that decision too.** M2 builds already push before `RunFinished`; merely gating the later PR API call cannot enforce a deliver mode of off. Proposed item-linked execution @@ -484,8 +500,10 @@ Standalone M2 runs retain their existing automatic push. Expiry/retirement/takeo preserved without publishing, and orphan recovery honors the hold. This introduces a run state and control/result messages, not a run aggregate. The design must establish charge reporting at work-ready/final completion without double counting, and the artifact/verification boundary before -granting delivery. It is explicit additional work in slice 8, conditional on the analyst accepting -this execution boundary; the current M2 worker cannot be described as already supporting it. +granting delivery. This is its own slice **8b**, following 8a; slices 9 and 10 keep their numbers. +Its two-part exit requires item-linked publication hold through restart **and** a standalone +`/fix` still pushing automatically, re-proved live on `artyomsv/spire-test`. Unit tests cannot +replace that second proof. The current M2 worker does not already support the hold. ## 7. Durable approvals @@ -522,7 +540,7 @@ does not count as a person, even after an account has been renamed. Gate answers and authorized `/fix` commands are deliberate workflow actions; classify and deduplicate them before generic comment takeover. This is a proposed precedence rule resolving -FR-F22's literal “commenting” against FR-F25's tracker/PR answer channels; record it in ADR-045. +FR-F22's literal “commenting” against FR-F25's tracker/PR answer channels; record it in ADR-045 with the FR-F22/FR-F25 conflict named. Normal human comments and pushes take over. A PR approval is processed as a gate response only when it actually matches an open gate; it cannot accidentally resume a suspended item. @@ -576,29 +594,30 @@ unavailable states needed so those features can arrive without bypassing policy. No production code, migrations, dev data, Gradle execution or runtime restarts belong to Round 1. Commit only this design and its plan, push the existing branch, open the requested draft PR. -## 11. Questions the analyst must settle - -These do not block publishing a reviewable design. They do block silently choosing product -behavior in the dependent implementation slice. - -1. **Three journeys and phase ordering:** accept the prepared single-task/manual-artifact M3 - boundary in §6.3, or move named M4 capabilities into M3? Also reconcile review-before-deliver - with the existing PR-triggered reviewer. Fake completions are not an option. -2. **Profile ordering:** accept explicit operator-owned precedence plus a component-wise meet, or - specify another rule for incomparable vectors. Even suggest/assisted cross at the plan phase; - the ticket's “lowest” is undefined without a separate precedence rule today. -3. **Identity/permission portability:** Bitbucket's effective permission read requires an admin - caller; exact handles are not a universal Jira/Bitbucket identity primitive. Accept explicit - capability errors/disambiguated selection and that credential prerequisite, or narrow the - acceptance wording. Do not grant access by guessing or silently raise a token's authority. -4. **Repository registration compatibility:** confirm one source/ISSUE hook per repository and - REVIEWER/FACTORY/ISSUE as product event kinds. Decide whether legacy org auto-enrollment is - retained beyond migration; otherwise existing installations lose that convenience at cutover. -5. **Drafts and takeover:** require provider-native drafts wherever available and visibly refuse - unsupported `draft_pr` (never substitute a title prefix), and accept explicit command/approval - precedence over generic human-comment takeover. Native draft support and publication-hold - mechanics, including the proposed work-ready/delivery handshake in §6.3, must be established - in the relevant provider/runtime slices before claiming parity. - -The decisions above make the proposal concrete for review; unresolved alternatives are recorded -here instead of being disguised as ticket requirements. +## 11. Review decisions — settled in Round 2 + +The [review summary and six inline comments](https://github.com/artyomsv/code-spire/pull/153#pullrequestreview-5188278775) +settled all five questions. These are requirements for implementation, not pending approvals. + +1. **Journeys/order:** accepted real state machine, manual tracker-artifact handoff and one + prepared M2 build, proved at plan/build. Missing later capabilities wait honestly. Correct + delivery-before-review in ADR-045 and the eight-phase diagram in slice 8a. +2. **Profiles:** accepted operator precedence plus the meet of every eligible applied label, + pinned vector and ceiling. ADR-045 states: **the effective vector is never above any applied + label in any component.** Precedence selects display/clamp wording, never authority by itself. +3. **Identity/permission:** explicit capability errors and disambiguated selection are accepted; + do not guess or raise token authority. State credential prerequisites in operator-facing text. + Add each per-forge identity behavior as its own UNVERIFIED entry in its introducing slice, + identifying the forge, measurement and remaining proof. This includes Bitbucket's admin-only + effective-permission query and provider-specific handle resolution behavior. +4. **Repositories:** one source/ISSUE hook per repository, with REVIEWER/FACTORY/ISSUE kinds. + Org auto-enrollment exists only during the bridge. At cutover an unregistered repository event + raises attention naming repo, origin and incoming registration, with all three pre-filled in + the Register action. Retain populated account workspace evidence, unused after slice 2, until + slice 10. Back up the actual dev database before migrations and compare actual credential + decryption after slice 2, using the exact commands in the plan. +5. **Drafts/takeover:** native drafts or explicit refusal, never a title prefix. Commands and + gate answers precede generic comment takeover; ADR-045 names the FR-F22/FR-F25 conflict. + Slice 8b separately owns publication hold and draft delivery after 8a. Its exit also requires + a live standalone `/fix` on `artyomsv/spire-test` still pushing automatically. Slices 9 and 10 + retain their numbers. From 9e797f70737e261465e791ec27826bcbedd6054e Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Sun, 13 Sep 2026 02:30:25 +0200 Subject: [PATCH 03/41] Add repository registration and the M3 migration bridge Register explicit repository coordinates and role bindings before cutting review and run resolution over from legacy account workspaces. Preserve account ids, encrypted credentials and context references, and reconcile versioned gateway snapshots without overwriting operator selections. Require origin evidence for automatic migration and expose unresolved mappings for repair. Add the registry API and settings view, retain the existing backup in .handoff, and record the credential and mutation proofs. Verify forced fast and service tiers sequentially, all 620 UI tests and the UI build. Record 3005 Java tests with zero failures and one Windows symlink skip, plus 40 distinct fail/restore/pass mutation checks. Refs #114 --- .claude/reviews/global/factory-m3-slice1.md | 124 ++++++++++++++ CLAUDE.md | 31 ++-- docs/CONTRACT.md | 16 ++ docs/DATA-MODEL.md | 20 +++ docs/DECISIONS.md | 45 +++++ docs/HISTORY.md | 25 +++ docs/SMOKE-TEST.md | 28 +++ docs/UNVERIFIED.md | 46 ++++- .../plans/2026-09-12-factory-m3-work-items.md | 83 +++++---- ...2026-09-12-factory-m3-work-items-design.md | 2 +- scripts/DevCredentialContinuity.java | 83 +++++++++ scripts/verify-dev-credential-continuity.ps1 | 39 +++++ .../event/RepositoryRegistration.java | 19 +++ .../codespire/contract/scm/ForgeOrigin.java | 27 +++ .../contract/ContractSchemaSnapshotTest.java | 3 + .../contract/RepositoryRegistrationTest.java | 23 +++ .../contract/scm/ForgeOriginTest.java | 18 ++ .../src/test/resources/contract-schema.txt | 3 + spire-gateway/build.gradle.kts | 1 + .../registry/RepositorySnapshotPublisher.java | 62 +++++++ .../gateway/registry/WebhookRepoInput.java | 11 +- .../gateway/registry/WebhookRepoRegistry.java | 13 +- .../gateway/registry/WebhookRepoView.java | 3 +- .../src/main/resources/application.yml | 10 ++ .../V3__repository_snapshot_outbox.sql | 30 ++++ .../RepositorySnapshotMigrationTest.java | 41 +++++ .../RepositorySnapshotPublisherTest.java | 124 ++++++++++++++ .../attention/AttentionQueries.java | 1 + .../codespire/orchestrator/dlq/DlqTopics.java | 1 + .../provider/ProviderClients.java | 8 + .../provider/ProviderRegistry.java | 23 +++ .../repository/RepositoryAccounts.java | 50 ++++++ .../repository/RepositoryAttentionRows.java | 30 ++++ .../repository/RepositoryBindings.java | 64 +++++++ .../repository/RepositoryHistoryBridge.java | 88 ++++++++++ .../repository/RepositoryInput.java | 7 + .../repository/RepositoryMappings.java | 56 ++++++ .../repository/RepositoryMigrationBridge.java | 151 +++++++++++++++++ .../repository/RepositoryRegistry.java | 140 +++++++++++++++ .../repository/RepositoryResource.java | 70 ++++++++ .../RepositorySnapshotConsumer.java | 21 +++ .../repository/RepositoryView.java | 9 + .../src/main/resources/application.yml | 14 ++ .../V60__repository_registration_bridge.sql | 41 +++++ .../orchestrator/dlq/DlqTopicsTest.java | 4 + .../factory/RunAgentStartedTest.java | 11 +- .../pipeline/ReviewRetryScheduleIT.java | 22 ++- .../provider/RepositoryForgeOriginTest.java | 14 ++ .../repository/RepositoryAccountsTest.java | 117 +++++++++++++ .../repository/RepositoryCoordinatesTest.java | 22 +++ .../repository/RepositoryFixture.java | 64 +++++++ .../RepositoryMigrationBridgeTest.java | 160 ++++++++++++++++++ .../repository/RepositoryResourceTest.java | 46 +++++ .../RepositorySchemaMigrationTest.java | 52 ++++++ .../RepositorySnapshotConsumerTest.java | 37 ++++ spire-ui/src/App.tsx | 2 + spire-ui/src/api.ts | 2 + spire-ui/src/components/ProviderFormModal.tsx | 2 + .../src/components/SettingsWebhookRepos.tsx | 1 + .../repositories/RepositoryForm.tsx | 64 +++++++ .../repositories/RepositoryPending.tsx | 27 +++ .../RepositoryRegistryPage.test.tsx | 85 ++++++++++ .../repositories/RepositoryRegistryPage.tsx | 55 ++++++ .../repositories/repositoriesApi.ts | 63 +++++++ 64 files changed, 2482 insertions(+), 72 deletions(-) create mode 100644 .claude/reviews/global/factory-m3-slice1.md create mode 100644 scripts/DevCredentialContinuity.java create mode 100644 scripts/verify-dev-credential-continuity.ps1 create mode 100644 spire-contract/src/main/java/dev/codespire/contract/event/RepositoryRegistration.java create mode 100644 spire-contract/src/main/java/dev/codespire/contract/scm/ForgeOrigin.java create mode 100644 spire-contract/src/test/java/dev/codespire/contract/RepositoryRegistrationTest.java create mode 100644 spire-contract/src/test/java/dev/codespire/contract/scm/ForgeOriginTest.java create mode 100644 spire-gateway/src/main/java/dev/codespire/gateway/registry/RepositorySnapshotPublisher.java create mode 100644 spire-gateway/src/main/resources/db/migration/V3__repository_snapshot_outbox.sql create mode 100644 spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotMigrationTest.java create mode 100644 spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotPublisherTest.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAccounts.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAttentionRows.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryBindings.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryHistoryBridge.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryInput.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMappings.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridge.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryRegistry.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryResource.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumer.java create mode 100644 spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryView.java create mode 100644 spire-orchestrator/src/main/resources/db/migration/V60__repository_registration_bridge.sql create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/provider/RepositoryForgeOriginTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryAccountsTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryCoordinatesTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryFixture.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridgeTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryResourceTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySchemaMigrationTest.java create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumerTest.java create mode 100644 spire-ui/src/components/repositories/RepositoryForm.tsx create mode 100644 spire-ui/src/components/repositories/RepositoryPending.tsx create mode 100644 spire-ui/src/components/repositories/RepositoryRegistryPage.test.tsx create mode 100644 spire-ui/src/components/repositories/RepositoryRegistryPage.tsx create mode 100644 spire-ui/src/components/repositories/repositoriesApi.ts diff --git a/.claude/reviews/global/factory-m3-slice1.md b/.claude/reviews/global/factory-m3-slice1.md new file mode 100644 index 00000000..5d820a2f --- /dev/null +++ b/.claude/reviews/global/factory-m3-slice1.md @@ -0,0 +1,124 @@ +# Factory M3 slice 1 — repository registry and migration bridge + +PR #153, issue #114. This slice expands the schema and makes explicit repository configuration +reviewable. Review/run callers still use their existing legacy resolvers until slice 2. + +## Review disposition + +- Round 2's remaining backup concern: the plan now uses worktree-relative `.handoff/`, already + ignored by git. No session GUID remains in the plan. The existing + `spire-dev-pre-m3-2026-09-13.dump` satisfies the prerequisite: 182 objects, 492,480 bytes, + SHA256 `7660D8EAE4110141887BD565747D45D1AA2621025A157D36741F70B6E65A1051`. + No second dump was taken in response to that review. +- Keep `scm_provider.workspace`, its values and old constraints in slice 1. Slice 2 removes the + key/check and runtime reads; slice 10 removes the retained workspace column. +- Reconcile the M2 live-proof claims now: `artyomsv/spire-test#31`, runs `3987682681:1` and + `3987682176:1`, resolved threads and persisted verdicts. Keep the automated GitLab network gap + separate. Slice 8b's standalone live `/fix` proof remains required; it was not attempted here. + +## Implemented behavior + +- V60 adds repositories, role bindings, immutable legacy mapping evidence, snapshot revisions and + nullable history references. V3 adds the gateway outbox and optional registration origin. +- Registry API and settings view expose workspace, canonical origin and explicit role accounts, + including missing, disabled and conflicting identities. Stale edits fail with a reload message. + Referenced account deletion names repositories; kind/origin changes require reassignment. +- Disabled repositories/accounts, wrong roles/kinds/origins and identity collisions after rotation + cannot resolve through the new registry. Binding validation preserves separate known identities. +- Gateway metadata has a typed wire discriminator and a stable registration key/revision. It + contains no webhook secret/routing key. Broker acknowledgement precedes marking the outbox sent; + failed processing has a registry-specific DLQ replay route. +- Replays preserve operator-selected accounts. Missing/conflicting legacy origins become pending + mappings and attention rows with explicit repair. Real historical coordinates, including nested + namespaces observed through legacy org coverage, receive nullable repository references. + Automatic binding requires explicit registration origin or persisted PR URL evidence matching + the legacy account origin. Workspace equality alone is insufficient; history without that + evidence remains pending. Both review and run history are linked after a mapping is established. + +## Verification scope + +All new database fixtures run in disposable Dev Services databases/private schemas and use +`TEST-` names. `RepositoryFixture.removeFixture` deletes owned history, bridge rows, bindings, +repositories, legacy evidence and accounts in FK order; gateway fixtures remove their registration +and outbox rows. No synthetic rows were inserted into the running dev database. + +The read-only credential probe captured and compared 9 real credential/reference entries. An +encrypted TEST-only baseline with an extra reference was rejected; bypassing the comparison +caused exactly one CLI assertion failure, and restoration made it pass again. The real baseline +then matched again. This is pre-upgrade evidence: the dev stack has not been rebuilt for slice 1. + +Two existing tests needed deterministic clocks: `RunAgentStartedTest` now compares a database +timestamp against the database clock; `ReviewRetryScheduleIT` uses an explicit future clock so +the background scheduler cannot claim its fixture. No production timing behavior changed. + +The user stopped four pre-existing `quarkusDev` run workers during verification. None was started +for this slice. Final Docker-driving verification passed with those workers stopped; there was +no missing-container failure. A read-only thread snapshot located the long runtime-test pause in +the existing publisher-drain wait, whose limit is five minutes. No runtime code was changed. + +## Final verification — 2026-09-13 + +| Gate | Measured result | +|---|---| +| `testFast --rerun-tasks` | 1053 tests / 125 suites; zero failures; one skip; 1m 29s | +| `testServices --rerun-tasks` | 1952 tests / 223 suites; zero failures or skips; 20m 10s | +| Java total | 3005 tests / 348 suites; zero failures; one skip | +| Full UI suite | 620 tests / 76 files passed | +| `npm run build` | TypeScript and Vite passed | +| Working diff | `git diff --check` passed | + +Every Java report counted above was freshly written by the final gates; nightly `spire-e2e` +reports were excluded. The existing skipped test is +`PublishRepoTest.theBundleIsOpenedWithoutFollowingASymlink`: this Windows session lacks the +privilege to create the symlink. The service gates ran sequentially after all production mutations +were restored. No live deployment or post-upgrade credential proof is claimed. +## Mutation verification + +Each Java row below changed a compiling production line, selected exactly one test, observed +exactly one failure and no skips, restored the working-file scratch snapshot, and observed one +passing test. No mutation was restored from git. Repeated checks after origin changes are counted +once. The two UI mutations and one CLI mutation used the same fail/restore/pass discipline. + +| Mutation | Targeted test | +|---|---| +| `aad_preservation` | `RepositorySchemaMigrationTest.preservesAccountIdsCredentialsAndContextReferences` | +| `account_enabled` | `RepositoryAccountsTest.disabledAccountCannotServeAnExistingBinding` | +| `account_kind` | `RepositoryAccountsTest.corruptBindingCannotUseAnotherKind` | +| `account_origin` | `RepositoryAccountsTest.rejectsAnAccountFromAnotherOrigin` | +| `account_role` | `RepositoryAccountsTest.corruptBindingCannotUseAnotherRole` | +| `admin_registration` | `RepositoryResourceTest.viewerCannotRegisterRepository` | +| `ambiguous_origins` | `RepositoryMigrationBridgeTest.leavesConflictingOriginsPending` | +| `binding_identity` | `RepositoryAccountsTest.rejectsSameResolvedIdentity` | +| `binding_kind` | `RepositoryAccountsTest.rejectsWrongKindBinding` | +| `binding_missing` | `RepositoryAccountsTest.missingAccountIsAnActionableConflict` | +| `binding_origin` | `RepositoryAccountsTest.rejectsCrossOriginBindingWithoutLeavingRepository` | +| `binding_role` | `RepositoryAccountsTest.rejectsWrongRoleBinding` | +| `broker_ack` | `RepositorySnapshotPublisherTest.failedBrokerAcknowledgementKeepsTheOutboxForRetry` | +| `dlq_destination` | `DlqTopicsTest.registrationReplaysOntoItsRegistryTopic` | +| `gateway_bootstrap` | `RepositorySnapshotMigrationTest.upgradeQueuesExistingRegistrationWithoutChangingKeyOrSecret` | +| `gateway_origin_retention` | `RepositorySnapshotPublisherTest.preservesConfiguredOriginWhenALegacyClientEditsRegistration` | +| `history_run_link` | `RepositoryMigrationBridgeTest.orgHistoryCreatesOnlyTheRepositoryActuallyObserved` | +| `mapping_attention` | `RepositoryMigrationBridgeTest.leavesConflictingOriginsPending` | +| `mapping_coordinates` | `RepositoryMigrationBridgeTest.mappingRefusesAnotherRepositoryPath` | +| `mapping_preservation` | `RepositoryMigrationBridgeTest.missingLegacyAccountStaysVisibleAndCanBeLinked` | +| `mapping_revision` | `RepositoryMigrationBridgeTest.mappingRefusesStaleRegistrationRevision` | +| `origin_url` | `ForgeOriginTest.rejectsAmbiguousOrSecretBearingUrls` | +| `public_web_origin_one` | `RepositoryForgeOriginTest.mapsPublicWebOriginsWithoutRewritingSelfHostedOrigins` | +| `public_web_origin_two` | `RepositoryForgeOriginTest.mapsPublicWebOriginsWithoutRewritingSelfHostedOrigins` | +| `referenced_delete` | `RepositoryAccountsTest.referencedDeleteNamesTheRepository` | +| `referenced_origin` | `RepositoryAccountsTest.referencedAccountCannotBeRepurposed` | +| `registration_origin` | `RepositoryMigrationBridgeTest.registrationFromAnotherHostCannotInheritWorkspaceCredentials` | +| `repository_enabled` | `RepositoryAccountsTest.disabledRepositoryCannotResolve` | +| `repository_revision` | `RepositoryResourceTest.staleUpdateCannotOverwriteBindings` | +| `repository_unique` | `RepositoryResourceTest.duplicateCoordinatesCannotCreateSecondRepository` | +| `role_selection` | `RepositoryAccountsTest.reviewerNeverReceivesTheFactoryCredential` | +| `rotated_identity` | `RepositoryAccountsTest.identityCollisionAfterRotationCannotServeEitherRole` | +| `slug_path` | `RepositoryCoordinatesTest.refusesNamespaceInsideSlug` | +| `snapshot_positive_revision` | `RepositoryRegistrationTest.rejectsInvalidSnapshotBeforeStorage` | +| `snapshot_revision` | `RepositoryMigrationBridgeTest.staleSnapshotCannotResurrectDeletedRegistration` | +| `unknown_origin` | `RepositoryMigrationBridgeTest.unknownRegistrationOriginCannotInheritWorkspaceCredentials` | +| `workspace_path` | `RepositoryCoordinatesTest.refusesTraversalAndEmptySegments` | +| `ui_host`, `ui_role` (separate mutations) | `RepositoryRegistryPage.test.tsx`: offers only same-origin accounts of the chosen role and saves explicit ids | +| `credential_comparison` | Read-only CLI Compare rejects an encrypted TEST baseline with one extra reference; bypassing equality fails that assertion; restoration rejects it and matches the real 9-entry baseline | + +Completed: 37 distinct Java mutations, 2 UI mutations and 1 CLI mutation (40 total). diff --git a/CLAUDE.md b/CLAUDE.md index ae180a07..2182a138 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,7 @@ The design is fully specified in `docs/` — **treat those files as the source o | `docs/SECURITY.md` | Trust boundaries, OIDC/RBAC, Tink encryption, LLM threat model, cost gaps | | `docs/TLS.md` | The five requirements a TLS terminator must satisfy, the identity-provider leg included, three worked topologies, and a symptom table. Code Spire terminates no TLS by design | | `docs/REPO-RULES.md` | The `.codespire` file: format, the target-branch rule and why, writing effective rules | -| `docs/DECISIONS.md` | ADR-001..041 — every locked decision with its why. ADR-029..040 are the software factory's; `docs/factory/` explains them in context | +| `docs/DECISIONS.md` | ADR-001..042 — every locked decision with its why. ADR-029..040 and ADR-042 are the software factory's; `docs/factory/` explains them in context | | `docs/UNVERIFIED.md` | **Read before claiming something works.** The register of claims the code or the docs make that no test establishes — known-broken-and-guarded, fixed-but-never-run-live, paths no test reaches, and claims needing a corpus or spend. Three milestones in a row shipped a feature that was green, documented, and did not work | | `docs/RESEARCH.md` | Market landscape + the PR-Agent code evaluation that justified greenfield | | `docs/ROADMAP.md` | Phases P0–P4 with exit criteria | @@ -45,7 +45,7 @@ The design is fully specified in `docs/` — **treat those files as the source o The per-milestone story — what shipped, what each review round found, the traps each one paid for — is in **`docs/HISTORY.md`**. A new milestone gets a new entry there; this section is rewritten to -describe the new current state. Everything below is true as of **2026-09-12**. +describe the new current state. Everything below is true as of **2026-09-13**. - **The reviewer (P0–P4) is delivered.** Three deployables over Kafka — `spire-gateway` (:34081), `spire-orchestrator` (:34080), `spire-review-worker` (:34082) — plus the `spire-ui` dashboard @@ -73,10 +73,13 @@ describe the new current state. Everything below is true as of **2026-09-12**. three adapters, so a run can end at a pull request rather than at a branch; `GET /api/runs`, the run↔review join and the `/runs` screen; and `spire-run-worker` in **both packaged stacks behind the `factory` compose profile** — opt-in because the Docker socket it mounts is root-equivalent - on the host. **The loop M2 exists to close has never been run end to end in one place**: the - dispatch, the push and the reconciliation are each proved separately, and a run unit cannot - reach the e2e stack's GitLab because `RunUnitSpec` has no network field (`docs/UNVERIFIED.md`). - **Next is M3** — `docs/factory/ROADMAP.md`. The two factory images are still not on GHCR. + on the host. **M2 was measured end to end on 2026-09-12:** runs `3987682681:1` and + `3987682176:1` on `artyomsv/spire-test#31` completed the finding → fix → push → reconciliation + chain, with resolved threads and persisted verdicts. The separate automated GitLab gap remains: + `RunUnitSpec` has no network field, so run units cannot reach that test stack's GitLab + (`docs/UNVERIFIED.md`). **M3 slice 1 adds the repository registry and migration bridge**; + existing review/run resolution remains on the legacy workspace until slice 2's cutover. + The two factory images are still not on GHCR. - **Accounts normalization (#148, ADR-041).** Machine accounts now own forge and Atlassian credentials in one registry. Context sources select a compatible account and retain their own URL, project keys and path allowlists. V59 plus an idempotent startup reconciler moves legacy @@ -90,14 +93,14 @@ describe the new current state. Everything below is true as of **2026-09-12**. remodeling, per-repository push checks and handle-to-id allowlist resolution. - **Known gaps** are in `docs/UNVERIFIED.md` (read before claiming something works) and `techdebt/` (one entry per item, per module). Review dispositions per round are in `.claude/reviews/`. -- **Measured, not estimated (2026-09-12):** 2954 Java tests across 336 suites, 0 failures, - 1 skipped; 613 UI tests across 75 files; TypeScript clean. Java verification uses JDK 25, - Docker and Git's shell on PATH. Nine intentional mutations fail their targeted tests, including - migration rollback, credential equality, disabled-account filtering, platform dispatch, - provider-neutrality, unknown scopes, account-picker compatibility, legacy wire degradation - and retaining scope observations through outages. UI table layout was - observed in headless Chrome at 1280/1440/1920 widths. The nightly testE2e tier and a production - credential migration were not run for this change. +- **Measured, not estimated (2026-09-13):** 3005 Java tests across 348 suites, 0 failures, + 1 skipped; 620 UI tests across 76 files; TypeScript and the UI build passed. Forced `testFast` + and `testServices` ran sequentially with JDK 25, Docker and Git's shell on PATH. The existing + symlink test skips because this Windows session lacks symlink privileges. Slice 1's 40 distinct + mutations each failed one targeted test and passed after scratch-snapshot restoration; see + `.claude/reviews/global/factory-m3-slice1.md`. The read-only encrypted probe matched 9 real + credential/reference entries before upgrade. The nightly testE2e tier and a live dev migration + were not run; no live run worker was started. ## Build & run diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md index 4ff41900..131b9bf7 100644 --- a/docs/CONTRACT.md +++ b/docs/CONTRACT.md @@ -1,5 +1,21 @@ # Domain Contract (`spire-contract`) +## Repository metadata channel (M3 slice 1, ADR-042) + +`cs.registry-integration` carries `RepositoryRegistration`, keyed by registration UUID (not a +review id). Its `type` discriminator is `RepositoryRegistration`; fields are `registrationId`, +positive monotonic `revision`, `providerType`, nullable `forgeOrigin`, `scope` (`repo`/`org`), +`target`, `enabled`, and `deleted`. The gateway outbox publishes only after its SQL transaction +commits and marks sent only after broker acknowledgement. The orchestrator accepts newer +revisions transactionally. Unknown legacy origins are reconciled only when unambiguous; +otherwise the snapshot becomes an operator-visible pending mapping. + +This is an integration snapshot, not a domain event or a new aggregate. Webhook keys and secrets +never cross the channel. Failed processing uses `cs.dlq`; the discriminator routes manual replay +back to `cs.registry-integration`. Existing review/run contracts and resolvers remain intact +during slice 1. Broker deployments with topic auto-creation disabled must provision this topic +and allow the gateway to write and the orchestrator to read it before upgrading. + > The shared kernel every service depends on: identifiers, the event envelope, the event & command > catalog, the `ReviewLifecycle` decider, the SPI ports, the context-aggregation policy, topics, and > the Bitbucket **Cloud** mapping. Companion to [EVENT-MODEL.md](EVENT-MODEL.md) (the narrative slices) diff --git a/docs/DATA-MODEL.md b/docs/DATA-MODEL.md index 4f0d4d57..4aeb5e7c 100644 --- a/docs/DATA-MODEL.md +++ b/docs/DATA-MODEL.md @@ -1,5 +1,25 @@ # Data Model +## M3 slice 1 expansion (ADR-042) + +V60 adds `repository` (UUID, kind, canonical forge origin, workspace, slug, enabled, revision) +with unique `(scm_type, forge_origin, workspace, slug)`, and `repository_account` with a repository +FK, account FK and one row per REVIEWER/FACTORY role. `review_status.repository_id` and +`factory_run.repository_id` are nullable during the bridge. Existing history keys stay intact. + +`repository_legacy_account` is immutable migration evidence: account UUID, kind, base URL, +workspace and role, without credentials or an FK that would erase evidence on deletion. +`repository_registration_bridge` stores the latest registration revision, metadata, selected +repository or named reconciliation problem. Duplicate/stale revisions cannot overwrite it. +Operators repair pending mappings through `/api/repositories/pending`. + +Gateway V3 adds nullable `webhook_repo.forge_origin` and `repository_snapshot_outbox`. Triggers +enqueue create/change/delete metadata atomically, including a bootstrap row for each existing +registration. The outbox stores a global monotonic revision, registration id, JSON and `sent_at`; +it contains neither `webhook_key` nor `webhook_secret`. Account/context ciphertext and UUID/AAD +are unchanged. The gateway API accepts/returns an optional canonical `forgeOrigin`; old clients +that omit it on update preserve the recorded origin. No old key or column is removed in this slice. + > Defines the actual data: (1) the **domain value types** that flow through events & ports, and (2) the > **persistence model** — the event store (the versioned source of truth), the blob store, and the > read-model projections, with relationships and encryption. Companion to [CONTRACT.md](CONTRACT.md) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index a7744f76..9081391e 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -4,6 +4,51 @@ Architecture decision records for Code Spire. Newest first. --- +## ADR-042 — Repositories own coordinates and explicitly bind role accounts + +**Status:** accepted design; M3 slice 1 implements the expansion and migration bridge. Slice 2 +performs the runtime cutover and retires the old key; slice 10 removes the legacy workspace column. + +**Decision.** A repository is identified by `(scm_type, forge_origin, workspace, slug)` and has +its own UUID. Forge origin is the canonical HTTP(S) scheme, host and non-default port, with no +API path suffix. Nested namespaces stay in workspace. `repository_account` binds at most one +REVIEWER and one FACTORY account. Configuration may leave either role empty or select a disabled +account; resolution requires an enabled repository and enabled account of the matching kind, +origin and role. Known reviewer/factory identities must differ at binding time and at resolution, +including after credential rotation. Account UUIDs, +ciphertexts, `provider:` AADs and context source references are preserved. + +**Why.** The former `(type, workspace, role)` account key conflates credentials with repository +selection and cannot distinguish hosts. Explicit references support credential rotation without +reassigning repositories. Repository edits use revision checks; account deletion names its +referencing repositories, and changing a referenced account's kind/origin is refused. + +**Bridge.** V60 snapshots legacy account assignments without changing existing serving resolvers. +The gateway retains ownership of webhooks and credentials. Its V3 transactional outbox publishes +typed, revisioned `RepositoryRegistration` metadata on `cs.registry-integration`, keyed by +registration UUID. Broker acknowledgement marks an outbox row sent; duplicate delivery and stale +revisions are harmless. Consumer failures use the existing DLQ with a registry-specific replay +route. No webhook secret or routing key appears in this event. + +An unambiguous legacy match with an evidenced origin creates bindings once; subsequent snapshots +preserve operator edits. A workspace alone never establishes a host. Conflicting or missing origins +become attention rows with explicit mapping repair. A bounded history sweep uses persisted review +URLs as origin evidence and links reviews/runs, including repositories observed through legacy org hooks. Org +auto-enrollment survives this bridge only: cutover must replace it with attention naming the +unregistered repository, origin and source registration plus a prefilled register action. + +**Rollback evidence.** Keep the old account key and populated workspace in slice 1. Slice 2 drops +the key/checks and all runtime reads, but retains workspace untouched until slice 10. Before any +dev upgrade, preserve a verified full database dump and the matching keysets. The repeatable +commands and real credential continuity probe are in the M3 plan; `.handoff/` survives sessions. + +**Proof.** `RepositorySchemaMigrationTest`, `RepositorySnapshotMigrationTest`, +`RepositoryMigrationBridgeTest`, `RepositoryAccountsTest`, `RepositoryResourceTest`, the broker +publisher/consumer tests, and `RepositoryRegistryPage.test.tsx`. These establish the expansion in +isolated PostgreSQL/Kafka; the production rollout and resolver cutover are separate claims. + +--- + ## ADR-041 — Credentials live on accounts; context sources reference an account **Context.** `context_provider` copied the credential shape of `llm_provider`: a key without diff --git a/docs/HISTORY.md b/docs/HISTORY.md index 2821c126..2dda63ce 100644 --- a/docs/HISTORY.md +++ b/docs/HISTORY.md @@ -1808,3 +1808,28 @@ lives in `docs/`, the locked decisions in `docs/DECISIONS.md`, and claims no tes scope-query wrapper and unconditional scope-write overload were removed so future call sites cannot silently choose the old path. Disabled migration rows intentionally remain in Attention because their legacy credential columns must be retired too; that intent is now commented. + +- **Factory M3 slice 1 (2026-09-13, PR #153; ADR-042) — repository registration before resolver cutover.** + Repositories own explicit forge origin, workspace and slug plus reviewer/factory account bindings. + The settings view and admin API expose those choices, disabled or missing accounts, identity + conflicts and stale edits. Existing review/run callers retain their legacy resolution until + slice 2; the account workspace and its constraints remain intact. + - V60 expands the orchestrator schema without changing account ids, ciphertext or context + references. Gateway V3 queues versioned metadata snapshots in a transactional outbox; broker + acknowledgement precedes completion, and failed snapshots have a registry DLQ replay route. + Replays preserve operator rebindings. Automatic migration requires explicit registration + origin or stored PR URL evidence; unknown/mismatched origins produce named attention rows + and explicit mapping repair. Historical review and run coordinates are linked together. + - The reviewed backup command now targets persistent, git-ignored `.handoff/` in the worktree. + The existing 182-object, 492,480-byte archive was reused; no second dump was taken for the + review correction. A read-only encrypted continuity probe matched 9 real credential/reference + entries. This remains pre-upgrade evidence: no dev deployment or live migration was performed. + - M2's live proof is recorded for `artyomsv/spire-test#31`, runs `3987682681:1` and + `3987682176:1`, resolved threads and persisted verdicts. The automated GitLab networking gap + remains separate. Slice 8b retains the standalone live `/fix` publication regression proof. + - Review evidence and the per-guard mutation ledger are in + `.claude/reviews/global/factory-m3-slice1.md`. + - Final sequential forced gates: 3005 Java tests across 348 suites, zero failures and one + existing Windows symlink privilege skip; 620 UI tests across 76 files and the UI build passed. + Forty distinct mutations each failed one targeted test and passed after scratch restoration. + Docker-driving tests passed with the live run workers stopped; none was started for this slice. diff --git a/docs/SMOKE-TEST.md b/docs/SMOKE-TEST.md index ea9c45dc..50f6b563 100644 --- a/docs/SMOKE-TEST.md +++ b/docs/SMOKE-TEST.md @@ -1,5 +1,33 @@ # Smoke Test Runbook +## M3 slice 1 registry upgrade + +Before rebuilding dev, follow slice 1 in +[`2026-09-12-factory-m3-work-items.md`](superpowers/plans/2026-09-12-factory-m3-work-items.md). +The verified `.handoff/spire-dev-pre-m3-2026-09-13.dump` already satisfies the backup prerequisite; +do not take a second dump. `.handoff/m3-real-credentials.bin` holds encrypted baseline evidence +for 9 real credential/reference entries. Keep the matching keysets outside git. Slice 2 must +rerun the probe's Compare mode on the real upgraded rows. + +The first upgraded gateway enqueues existing registrations in V3. Legacy registrations with no +recorded origin require explicit mapping; workspace alone is not host evidence. The gateway API +accepts an optional `forgeOrigin`, preserving it when an old client edits other fields. +The orchestrator applies V60, +consumes `cs.registry-integration`, and sweeps existing history. Topic auto-creation works in the +bundled broker; external brokers must provision the topic and ACLs first. Inspect pending broker +records in the existing DLQ screen and replay after fixing the reported cause. + +Open Settings → Repositories → Registered repositories and accounts. Confirm the real workspace, +forge origin and explicit reviewer/factory selections. Pending mappings name the registration and +target; verify the host, register the matching repository if needed, then explicitly link it. +No pipeline routing changes occur in slice 1. Do not invent accounts or runs in this live stack to +test registration: `RepositoryResourceTest` exercises the real HTTP API in Dev Services. + +The migration tests upgrade populated private schemas and prove ciphertext/UUID/reference/key +preservation. Broker tests separately exercise the gateway publisher and orchestrator consumer; +they do not claim a live multi-service rollout. The production account workspace remains populated +through slice 2 as rollback evidence and is removed only in slice 10. + **A** stub pipeline, zero external accounts; **B** real Bitbucket Cloud PR (webhook); **C** real GitHub PR via manual Register PR (no webhook); **D** real GitLab MR via manual Register PR (no webhook); **E** real GitHub PR via webhook (Tailscale Funnel); **F** real GitLab MR via diff --git a/docs/UNVERIFIED.md b/docs/UNVERIFIED.md index d6817e82..3fef497b 100644 --- a/docs/UNVERIFIED.md +++ b/docs/UNVERIFIED.md @@ -33,6 +33,35 @@ evidence would settle it**. --- +## Repository bridge — rollout evidence still needed (ADR-042, 2026-09-13) + +- **The actual dev upgrade is not yet measured.** Slice 1 tests upgrade populated private + PostgreSQL schemas, exercise the gateway outbox and real broker consumer, and compare legacy + and explicit role resolution with distinct encrypted credentials. The running dev services have + not been rebuilt for slice 1. The existing `.handoff/spire-dev-pre-m3-2026-09-13.dump` is the + verified backup (182 objects; 492,480 bytes). The read-only continuity probe captured and compared + 9 real credential/reference entries successfully before upgrade; that does not prove post-upgrade + continuity. Slice 2 must compare the same encrypted baseline after its migration on the real rows. +- **Historical forge origin can be unknowable.** Legacy history and gateway registrations do not + always record a host. The bridge uses the immutable legacy account snapshot only when there is + one matching origin backed by registration metadata or persisted review URLs; unknown origins, + conflicts and missing accounts become attention rows with explicit repair. Runs with no such + origin evidence remain pending even when only one legacy workspace account exists. + Tests establish that refusal and repair, not which host an old live row actually belonged to. + Inspect actual migrated mappings against the operator's known forge origins before cutover. +- **Native per-forge identity and permission behavior is unchanged in slice 1.** No new remote + identity lookup or permission API is introduced here. Each introducing slice must add separate + GitHub, GitLab and Bitbucket observations, stating the measured endpoint/token family and host; + fixtures are not live evidence. The automated GitLab M2 gap remains a separate entry below. + +Repository-origin normalization is local URL interpretation, not an identity API measurement: + +| Forge behavior | Measured in this slice | Still needed before relying on a live mapping | +|---|---|---| +| GitHub public web URLs map to `https://api.github.com`; self-hosted origins remain unchanged | `RepositoryForgeOriginTest`, public URL and `TEST-forge.example.test` fixtures; no remote call | Inspect the actual account API base and persisted PR URL, particularly custom API proxies. | +| GitLab web/API path suffixes share the same canonical origin | Nested-namespace PostgreSQL history fixture plus `RepositoryForgeOriginTest`; no live GitLab call | Compare the actual API and web origins on the target installation. | +| Bitbucket Cloud public web URLs map to `https://api.bitbucket.org` | `RepositoryForgeOriginTest` public URL fixture; no live Bitbucket call | Compare a live persisted PR URL with the selected account API origin. | + ## Accounts normalization — live evidence still needed (ADR-041, 2026-09-12) Sources below were retrieved **2026-09-11**. WireMock checks prove how the application handles @@ -271,14 +300,15 @@ Not work. Written down because each has been rediscovered at least once. matches, a no-diff run reports the forge's own error, which is honest; the status gate makes a wrong match much harder. One measurement against a live GitLab (SMOKE-TEST Mode G) settles it, and nothing should depend on this arm until then. -- **The M2 loop is covered in three places and joined in none.** Finding → fix run → push → - reconciliation is what M2 exists to close. `FixRunDispatcherTest` proves the dispatch, - `Adr040ExistingBranchTest` proves the push against a real remote with real containers, and - `ReviewChainTest` proves review and reconciliation against a real GitLab. **Nothing proves the - halves meet**, and it is not a matter of effort: a run unit lands on the default bridge and - cannot resolve the e2e stack's `gitlab` service, because `RunUnitSpec` has no network and - `DockerRunRuntime` never sets one. Rebinding GitLab off loopback would undo a deliberate - security control in `compose.e2e.yml`, so it is not the answer. +- **~~The M2 loop has no joined live proof~~ — CLOSED 2026-09-12.** On the live GitHub pull + request `artyomsv/spire-test#31`, runs `3987682681:1` and `3987682176:1` traversed finding → fix + run → push → reconciliation. The review threads were resolved and verdicts persisted. This + observation closes the live-chain claim; it does not establish an automated GitLab loop. +- **The automated GitLab M2 loop still cannot join dispatch, push and reconciliation.** + `FixRunDispatcherTest`, `Adr040ExistingBranchTest` and `ReviewChainTest` cover those legs + separately. A run unit cannot resolve the e2e stack's `gitlab` service: `RunUnitSpec` has no + network field and `DockerRunRuntime` never sets one. Rebinding GitLab off loopback would undo a + deliberate security control in `compose.e2e.yml`, so it is not the answer. — `techdebt/spire-runtime-docker/2-3-a-run-unit-has-no-network-so-it-is-neither-isolated-nor-reachable.md` - **~~The publisher's trunk floor is not exercised end to end~~ — CLOSED 2026-09-11.** It now is. This entry said the run died as `RUNTIME_UNAVAILABLE, init container failed with exit 1` diff --git a/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md index 955122e2..c455487f 100644 --- a/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md +++ b/docs/superpowers/plans/2026-09-12-factory-m3-work-items.md @@ -2,7 +2,7 @@ **Date:** 2026-09-12 -**Status:** Accepted with Round 2 amendments; implementation checkboxes and test outcomes remain pending until measured. +**Status:** Planning accepted; slice 1 implemented and verified for review. Slices 2–10 remain planned. **Goal:** Start factory work from a tracker ticket with explicit, bounded autonomy; make repository ownership, command authority and approval state visible and durable. @@ -35,10 +35,11 @@ Testing Library; existing Gradle split test tiers. Keep the versions already pin and a body explaining nontrivial changes. No authoring attribution, coauthor trailers, model names, vendor names or generated-by notices in commit/PR text. Requested PR title takes precedence over generic commit-style templates. -- The running `spire-dev` stack and `spire-run-worker` Gradle process on `:34083` are shared. - Do not stop them, run compose down or kill their processes. Service tests use their own - Testcontainers resources. If a Docker test cannot coexist with the dev worker, coordinate - a safe window with the analyst; do not stop the worker unilaterally. +- The running `spire-dev` stack is shared; do not run compose down. On 2026-09-13 the analyst + stopped all four competing `quarkusDev` run workers. Keep them stopped: the analyst will start + a worker for slice 8b's live proof. Service tests use their own Testcontainers resources. If a + Docker-driving test loses a container unexpectedly, report possible external deletion to the + analyst before investigating a production defect; the Gradle lock does not cover dev workers. - **Never run concurrent Gradle test invocations in this worktree.** Run `./gradlew testFast` then `./gradlew testServices`, sequentially, using `--rerun-tasks` for measured evidence. On PowerShell use `.\gradlew.bat`. Targeted runs use `--tests` plus `--rerun-tasks`. @@ -55,8 +56,9 @@ Testing Library; existing Gradle split test tiers. Keep the versions already pin requires announcing its actual ids and exact cleanup `DELETE` before insertion; track remote issue/branch cleanup too. This round creates no data. Do not include generic destructive SQL against user-owned rows in a runbook and call it cleanup. -- Temporary files belong in this session's scratchpad: - `C:\Users\artjo\AppData\Local\Temp\claude\E--Projects-Stukans-code-spire-worktrees-feat-software-factory\5f317e7d-1b64-4305-bf1c-e3a370b07eb1\scratchpad`. +- Temporary files belong in the active session's scratchpad. Do not reuse a previous session's + path. Persistent database backups and encrypted continuity evidence belong in the worktree's + git-ignored `.handoff/` directory. - Unknown capabilities, missing external evidence and test skips are visible outcomes. No stub phase reports success in production. Proposed tests below are not evidence until executed. @@ -118,20 +120,20 @@ the plan and its architecture allowlist together. **Files:** only this document and its linked design. -- [ ] Confirm branch and clean initial worktree, read the complete brief, re-fetch issue #114 and +- [x] Confirm branch and clean initial worktree, read the complete brief, re-fetch issue #114 and inspect ADR-041, factory requirements and the actual run/account/webhook implementation. -- [ ] Write the aggregate decision and why; repository migration; policy/identity/gate rules; +- [x] Write the aggregate decision and why; repository migration; policy/identity/gate rules; ordered runnable slices; exact proof and mutation obligations for criteria 1–7. -- [ ] Record under-specification in design §11 rather than choosing silent fallback behavior. -- [ ] Check Markdown links, `git diff --check`, exact two-file scope and absence of secrets/data. +- [x] Record under-specification in design §11 rather than choosing silent fallback behavior. +- [x] Check Markdown links, `git diff --check`, exact two-file scope and absence of secrets/data. No Gradle/UI suites are warranted for this documentation-only change. -- [ ] Commit `Plan Factory M3 work items, labels and gates`, with a body explaining the new +- [x] Commit `Plan Factory M3 work items, labels and gates`, with a body explaining the new registry and workflow decisions and the acceptance-proof plan. Push with `git push -u origin feat/factory-m3-work-items`. -- [ ] Write the PR body as a real UTF-8 scratchpad file and use `gh pr create --draft --base master +- [x] Write the PR body as a real UTF-8 scratchpad file and use `gh pr create --draft --base master --head feat/factory-m3-work-items --title "Factory M3 — work items, labels and gates" --body-file `. Link issue #114 without claiming to close implementation work. -- [ ] Verify the remote head equals the commit, PR is draft against master, and only these two +- [x] Verify the remote head equals the commit, PR is draft against master, and only these two documents are in its diff. Report the PR number and the design questions. **Stop Round 1.** ## Slice 1 — register a repository while preserving existing resolution @@ -142,14 +144,19 @@ outbox; first repository UI; ADR-042 draft in `docs/DECISIONS.md`. **Produces:** `RepositoryAccounts.resolve(repositoryId, role)` and a non-secret serving view; `POST/GET /api/repositories`; versioned metadata-only registration snapshots. -- [ ] **Before any new migration reaches dev:** take a full `pg_dump`, validate the archive and +- [x] **Before any new migration reaches dev:** take a full `pg_dump`, validate the archive and record its path/hash. Use the exact PowerShell commands below; the binary dump never passes through PowerShell text redirection. The running stack holds real accounts, encrypted source credentials and review/run history. Preserve its existing matching keyset securely outside git. +The existing `.handoff/spire-dev-pre-m3-2026-09-13.dump` satisfies this prerequisite (182 objects, +492,480 bytes, verified 2026-09-13). **Do not take another dump for slice 1.** For a future upgrade, +run this repeatable command from the worktree root; `.handoff/` is git-ignored and survives sessions. + ```powershell -$m3Scratch = 'C:\Users\artjo\AppData\Local\Temp\claude\E--Projects-Stukans-code-spire-worktrees-feat-software-factory\5f317e7d-1b64-4305-bf1c-e3a370b07eb1\scratchpad' -$m3Dump = Join-Path $m3Scratch ('m3-before-slice1-' + (Get-Date -Format 'yyyyMMdd-HHmmss') + '.dump') +$m3Handoff = Join-Path (Get-Location).Path '.handoff' +[void](New-Item -ItemType Directory -Force -Path $m3Handoff) +$m3Dump = Join-Path $m3Handoff ('m3-before-slice1-' + (Get-Date -Format 'yyyyMMdd-HHmmss') + '.dump') $m3Process = [Diagnostics.Process]::new() $m3Process.StartInfo = [Diagnostics.ProcessStartInfo]::new('docker') $m3Process.StartInfo.UseShellExecute = $false @@ -161,55 +168,69 @@ Get-FileHash -LiteralPath $m3Dump -Algorithm SHA256 ``` Validate archive listing with `pg_restore --list` using a one-shot container with only this -scratchpad mounted read-only, without changing the running stack: +handoff directory mounted read-only, without changing the running stack: ```powershell -docker run --rm --mount "type=bind,source=$m3Scratch,target=/backup,readonly" postgres:18.4-alpine pg_restore --list "/backup/$([IO.Path]::GetFileName($m3Dump))" +docker run --rm --mount "type=bind,source=$m3Handoff,target=/backup,readonly" postgres:18.4-alpine pg_restore --list "/backup/$([IO.Path]::GetFileName($m3Dump))" if ($LASTEXITCODE -ne 0) { throw 'Backup archive validation failed' } ``` -- [ ] Add `scripts/verify-dev-credential-continuity.ps1` as the read-only local operational probe. +- [x] Add `scripts/verify-dev-credential-continuity.ps1` as the read-only local operational probe. It uses the actual dev keyset and `EncryptionService`, with `provider:` for account secrets and `context-provider:` for remaining legacy context secrets. Capture encrypted baseline - evidence (including source→account references) into scratch; Compare re-decrypts actual rows + evidence (including source→account references) into .handoff/; Compare re-decrypts actual rows and checks equality in memory. Never write/log plaintext, keysets or unkeyed secret hashes. Execute Capture before migration; slice 2 must execute Compare on the real dev rows: ```powershell -.\scripts\verify-dev-credential-continuity.ps1 -Mode Capture -Snapshot (Join-Path $m3Scratch 'm3-real-credentials.bin') -.\scripts\verify-dev-credential-continuity.ps1 -Mode Compare -Snapshot (Join-Path $m3Scratch 'm3-real-credentials.bin') +$m3Handoff = Join-Path (Get-Location).Path '.handoff' +.\scripts\verify-dev-credential-continuity.ps1 -Mode Capture -Snapshot (Join-Path $m3Handoff 'm3-real-credentials.bin') +.\scripts\verify-dev-credential-continuity.ps1 -Mode Compare -Snapshot (Join-Path $m3Handoff 'm3-real-credentials.bin') ``` -- [ ] Reconcile `CLAUDE.md` and `docs/UNVERIFIED.md` **in this slice**: the live M2 chain on +Capture has already established the encrypted baseline for 9 real credential/reference entries. +Reuse it for Compare; Capture deliberately refuses to overwrite existing evidence. The measured +comparison in slice 1 is pre-upgrade only; slice 2's comparison after cutover remains required. + +- [x] Reconcile `CLAUDE.md` and `docs/UNVERIFIED.md` **in this slice**: the live M2 chain on `artyomsv/spire-test#31`, runs `3987682681:1` and `3987682176:1`, resolved threads and persisted verdicts was measured on 2026-09-12. Keep the automated GitLab gap as its own open entry: `RunUnitSpec` has no network field, so run units cannot reach that test stack's GitLab. -- [ ] Apply the accepted bridge-only org enrollment decision. Write failing migration/service tests: +- [x] Apply the accepted bridge-only org enrollment decision. Write failing migration/service tests: `RepositorySchemaMigrationTest.preservesAccountIdsCredentialsAndContextReferences`, `RepositoryMigrationBridgeTest.replaysGatewaySnapshotWithoutDuplicateBindings`, `RepositoryMigrationBridgeTest.leavesConflictingOriginsPending`, and `RepositoryResourceTest.registersARepositoryWithExplicitRoleBindings`. -- [ ] Add repository and binding tables, revision checks, referenced-delete protection and +- [x] Add repository and binding tables, revision checks, referenced-delete protection and migration snapshot storage. Preserve UUID/AAD and V59 source recovery. Do not relax the old account key before the bridge can preserve assignments. -- [ ] Publish/consume real gateway metadata with stable snapshot revision and outbox retries. +- [x] Publish/consume real gateway metadata with stable snapshot revision and outbox retries. Provision `cs.registry-integration`, keyed by registration id. Do not read gateway SQL from the orchestrator or send its webhook secret across this channel. -- [ ] Add repository registration/detail UI backed by the API, including empty/disabled/pending +- [x] Add repository registration/detail UI backed by the API, including empty/disabled/pending roles. Show workspace on the repository. During this slice the old account field is explicitly labelled legacy; it is removed at slice 2's cutover. -- [ ] Prove old review/run resolution equals new bindings for migrated fixtures, across restart, +- [x] Prove old review/run resolution equals new bindings for migrated fixtures, across restart, multiple roles, hosts and nested namespaces. Duplicate source snapshots must not recreate an account an operator has already rebound. -- [ ] Mutation: omit factory-role filtering in the binding resolver; run only + Migration requires origin evidence from explicit registration metadata or persisted PR URLs; + a workspace match alone cannot choose credentials. Unknown or conflicting origins remain + pending for explicit repair, including factory-only history without PR origin evidence. +- [x] Mutation: omit factory-role filtering in the binding resolver; run only `RepositoryAccountsTest.reviewerNeverReceivesTheFactoryCredential`. Expect one assertion failure with distinct `TEST-` credentials, restore snapshot, rerun green. Also kill the origin-match guard with `rejectsAnAccountFromAnotherOrigin` and migration AAD/id preservation with the migration test above, each as a separate mutation. -- [ ] Run relevant service/UI tests sequentially, demonstrate registration through the real API +- [x] Run relevant service/UI tests sequentially, demonstrate registration through the real API in the isolated test stack, update ADR-042/upgrade notes, commit the slice for review. +Measured 2026-09-13: forced `testFast` then `testServices`, 3005 Java tests / 348 suites with zero +failures and one existing Windows symlink skip; 620 UI tests and the UI build passed. Forty +distinct production mutations failed exactly one targeted test and passed after restoration. +Details: `.claude/reviews/global/factory-m3-slice1.md`. The dev stack was not rebuilt; slice 2's +post-cutover comparison against the real encrypted baseline remains required. + ## Slice 2 — cut over to repository ownership and per-kind hooks **Files:** all active account consumers, gateway registry/edge/resources, account DTOs/forms, diff --git a/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md index bd0834db..52b42305 100644 --- a/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md +++ b/docs/superpowers/specs/2026-09-12-factory-m3-work-items-design.md @@ -200,7 +200,7 @@ idempotent restart and refusal of ambiguous mappings. Do not exercise this on th in a test. The bridge must preserve its existing webhook keys and encrypted secrets. Before any new migration can reach the real dev stack, slice 1 takes and validates a full -`pg_dump` into the session scratchpad. The plan includes the exact binary-safe command. Preserve +`pg_dump` into the worktree's git-ignored `.handoff/` directory. The plan includes the exact binary-safe command. Preserve the matching existing keyset outside git and capture a credential-continuity proof using real rows and their actual Tink AADs. Slice 2 compares decrypted credentials against that baseline on the real dev rows after cutover; matching fixture data or matching ciphertext alone is insufficient. diff --git a/scripts/DevCredentialContinuity.java b/scripts/DevCredentialContinuity.java new file mode 100644 index 00000000..4441d2e6 --- /dev/null +++ b/scripts/DevCredentialContinuity.java @@ -0,0 +1,83 @@ +import dev.codespire.encryption.EncryptionService; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.Properties; + +/** Local read-only rollout proof. Plaintext exists only in memory; the baseline is Tink-encrypted. */ +class DevCredentialContinuity { + private static final String AAD = "m3-dev-credential-continuity"; + + public static void main(String[] args) throws Exception { + try { + compareOrCapture(args); + } catch (Exception failure) { + // JDBC/config exceptions can contain connection details. Never echo their messages here. + System.err.println("Credential continuity proof failed; no credentials were printed."); + System.exit(1); + } + } + + private static void compareOrCapture(String[] args) throws Exception { + EncryptionService encryption = new EncryptionService(System.getenv("SPIRE_ENCRYPTION_KEYSET")); + Properties current = read(encryption); + Path file = Path.of(args[1]); + if ("Capture".equals(args[0])) { + if (Files.exists(file)) throw new IllegalStateException("Baseline already exists"); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + current.store(bytes, "Real dev credential/reference continuity"); + Files.write(file, encryption.encrypt(bytes.toByteArray(), AAD), java.nio.file.StandardOpenOption.CREATE_NEW); + System.out.println("Captured " + current.size() + " real credential/reference entries in an encrypted baseline."); + return; + } + Properties baseline = new Properties(); + baseline.load(new ByteArrayInputStream(encryption.decrypt(Files.readAllBytes(file), AAD))); + if (!baseline.equals(current)) { + long missing = baseline.keySet().stream().filter(key -> !current.containsKey(key)).count(); + long added = current.keySet().stream().filter(key -> !baseline.containsKey(key)).count(); + long changed = baseline.keySet().stream().filter(current::containsKey) + .filter(key -> !baseline.get(key).equals(current.get(key))).count(); + System.err.printf("Continuity mismatch: %d missing, %d added, %d changed entries.%n", missing, added, changed); + throw new IllegalStateException("Mismatch"); + } + System.out.println("PASS: all " + current.size() + " real credential/reference entries are identical after decryption."); + } + + private static Properties read(EncryptionService encryption) throws Exception { + String url = "jdbc:postgresql://localhost:" + System.getenv().getOrDefault("POSTGRES_PORT", "34432") + + "/" + System.getenv("POSTGRES_DB"); + Properties entries = new Properties(); + try (Connection connection = DriverManager.getConnection(url, System.getenv("POSTGRES_USER"), + System.getenv("POSTGRES_PASSWORD"))) { + connection.setReadOnly(true); + connection.setAutoCommit(false); + connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ); + try (Statement statement = connection.createStatement(); ResultSet rows = statement.executeQuery( + "SELECT id, auth_secret FROM orchestrator.scm_provider ORDER BY id")) { + while (rows.next()) { + String id = rows.getString("id"); + entries.setProperty("account:" + id, encryption.decryptString(rows.getString("auth_secret"), "provider:" + id)); + } + } + try (Statement statement = connection.createStatement(); ResultSet rows = statement.executeQuery( + "SELECT id, account_id, auth_secret FROM orchestrator.context_provider ORDER BY id")) { + while (rows.next()) { + String id = rows.getString("id"); + String secret = rows.getString("auth_secret"); + String account = rows.getString("account_id"); + entries.setProperty("context-account:" + id, account == null ? "" : account); + if (secret != null) entries.setProperty("context-secret:" + id, + encryption.decryptString(secret, "context-provider:" + id)); + } + } + connection.commit(); + } + if (entries.isEmpty()) throw new IllegalStateException("An empty database is not continuity proof"); + return entries; + } +} diff --git a/scripts/verify-dev-credential-continuity.ps1 b/scripts/verify-dev-credential-continuity.ps1 new file mode 100644 index 00000000..ffac00df --- /dev/null +++ b/scripts/verify-dev-credential-continuity.ps1 @@ -0,0 +1,39 @@ +param( + [Parameter(Mandatory)][ValidateSet('Capture', 'Compare')][string]$Mode, + [Parameter(Mandatory)][string]$Snapshot +) +$ErrorActionPreference = 'Stop' +$repoRoot = Split-Path -Parent $PSScriptRoot +$gradleRoot = if ($env:GRADLE_USER_HOME) { $env:GRADLE_USER_HOME } else { Join-Path $env:USERPROFILE '.gradle' } +$cacheRoot = Join-Path $gradleRoot 'caches/modules-2/files-2.1' +$encryptionClasses = Join-Path $repoRoot 'spire-encryption/build/classes/java/main' +if (-not (Test-Path "$encryptionClasses/dev/codespire/encryption/EncryptionService.class")) { + throw 'Build :spire-encryption:classes first, using JDK 25.' +} +$classpath = @($encryptionClasses) +foreach ($artifact in @('com.google.crypto.tink/tink/1.23.0', 'com.google.protobuf/protobuf-java', 'com.google.code.gson/gson', 'org.postgresql/postgresql')) { + $jar = Get-ChildItem -LiteralPath (Join-Path $cacheRoot $artifact) -Recurse -Filter '*.jar' | + Where-Object { $_.Name -notmatch '-(sources|javadoc)\.jar$' } | Sort-Object LastWriteTime -Descending | Select-Object -First 1 + if (-not $jar) { throw "Missing cached runtime dependency: $artifact. Build the orchestrator first." } + $classpath += $jar.FullName +} +$java = if ($env:JAVA_HOME) { Join-Path $env:JAVA_HOME 'bin/java.exe' } else { (Get-Command java).Source } +$process = [Diagnostics.Process]::new() +$process.StartInfo = [Diagnostics.ProcessStartInfo]::new($java) +$process.StartInfo.UseShellExecute = $false +# Set only the child process environment. Never echo .env, secrets or the resulting command environment. +foreach ($line in [IO.File]::ReadAllLines((Join-Path $repoRoot '.env'))) { + if ($line -match '^\s*([A-Za-z_][A-Za-z0-9_]*)=(.*)$') { + $name = $Matches[1]; $value = $Matches[2].Trim() + if ($value.Length -ge 2 -and (($value.StartsWith('"') -and $value.EndsWith('"')) -or ($value.StartsWith("'") -and $value.EndsWith("'")))) { + $value = $value.Substring(1, $value.Length - 2) + } + $process.StartInfo.Environment[$name] = $value + } +} +@('--class-path', ($classpath -join ';'), (Join-Path $PSScriptRoot 'DevCredentialContinuity.java'), $Mode, [IO.Path]::GetFullPath($Snapshot)) | + ForEach-Object { $process.StartInfo.ArgumentList.Add($_) } +try { + [void]$process.Start(); $process.WaitForExit() + if ($process.ExitCode -ne 0) { throw 'Credential continuity proof failed.' } +} finally { $process.Dispose() } diff --git a/spire-contract/src/main/java/dev/codespire/contract/event/RepositoryRegistration.java b/spire-contract/src/main/java/dev/codespire/contract/event/RepositoryRegistration.java new file mode 100644 index 00000000..255d7c3b --- /dev/null +++ b/spire-contract/src/main/java/dev/codespire/contract/event/RepositoryRegistration.java @@ -0,0 +1,19 @@ +package dev.codespire.contract.event; + +import java.util.UUID; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonTypeName; + +/** Non-secret gateway snapshot on cs.registry-integration, keyed by registrationId, never reviewId. */ +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type") +@JsonTypeName("RepositoryRegistration") +public record RepositoryRegistration(UUID registrationId, long revision, String providerType, + String forgeOrigin, String scope, String target, + boolean enabled, boolean deleted) { + public RepositoryRegistration { + if (registrationId == null || revision < 1 || providerType == null || providerType.isBlank() + || (!"repo".equals(scope) && !"org".equals(scope)) || target == null || target.isBlank()) { + throw new IllegalArgumentException("Invalid repository registration snapshot"); + } + } +} diff --git a/spire-contract/src/main/java/dev/codespire/contract/scm/ForgeOrigin.java b/spire-contract/src/main/java/dev/codespire/contract/scm/ForgeOrigin.java new file mode 100644 index 00000000..7dce54cb --- /dev/null +++ b/spire-contract/src/main/java/dev/codespire/contract/scm/ForgeOrigin.java @@ -0,0 +1,27 @@ +package dev.codespire.contract.scm; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Locale; + +/** Host-qualified identity; API path suffixes and default ports do not create a second origin. */ +public final class ForgeOrigin { + private ForgeOrigin() { } + + public static String of(String value) { + if (value == null || value.isBlank()) throw new IllegalArgumentException("Forge origin is required"); + URI uri = URI.create(value.trim()); + String scheme = uri.getScheme() == null ? "" : uri.getScheme().toLowerCase(Locale.ROOT); + if ((!scheme.equals("https") && !scheme.equals("http")) || uri.getHost() == null + || uri.getUserInfo() != null || uri.getQuery() != null || uri.getFragment() != null) { + throw new IllegalArgumentException("Forge origin must be an HTTP(S) URL without credentials, query or fragment"); + } + int port = uri.getPort(); + if ((scheme.equals("https") && port == 443) || (scheme.equals("http") && port == 80)) port = -1; + try { + return new URI(scheme, null, uri.getHost().toLowerCase(Locale.ROOT), port, null, null, null).toString(); + } catch (URISyntaxException invalid) { + throw new IllegalArgumentException("Invalid forge origin", invalid); + } + } +} diff --git a/spire-contract/src/test/java/dev/codespire/contract/ContractSchemaSnapshotTest.java b/spire-contract/src/test/java/dev/codespire/contract/ContractSchemaSnapshotTest.java index acd78a73..6ee88644 100644 --- a/spire-contract/src/test/java/dev/codespire/contract/ContractSchemaSnapshotTest.java +++ b/spire-contract/src/test/java/dev/codespire/contract/ContractSchemaSnapshotTest.java @@ -143,6 +143,9 @@ private static String renderSchema() { lines.add("# ContextCredential"); lines.add(render(dev.codespire.contract.context.ContextCredential.class, "ContextCredential")); lines.add(""); + lines.add("# RepositoryRegistration"); + lines.add(render(dev.codespire.contract.event.RepositoryRegistration.class, "RepositoryRegistration")); + lines.add(""); return String.join("\n", lines); } diff --git a/spire-contract/src/test/java/dev/codespire/contract/RepositoryRegistrationTest.java b/spire-contract/src/test/java/dev/codespire/contract/RepositoryRegistrationTest.java new file mode 100644 index 00000000..5ea4a8f9 --- /dev/null +++ b/spire-contract/src/test/java/dev/codespire/contract/RepositoryRegistrationTest.java @@ -0,0 +1,23 @@ +package dev.codespire.contract; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.codespire.contract.event.RepositoryRegistration; +import org.junit.jupiter.api.Test; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.*; + +class RepositoryRegistrationTest { + @Test void metadataOnlyWireShapeRoundTrips() throws Exception { + String json = """ + {"type":"RepositoryRegistration","registrationId":"00000000-0000-0000-0000-000000000001","revision":17,"providerType":"TEST-forge", + "forgeOrigin":null,"scope":"repo","target":"TEST-group/nested/TEST-repo","enabled":true,"deleted":false} + """; + var mapper = new ObjectMapper(); + var snapshot = mapper.readValue(json, RepositoryRegistration.class); + assertEquals(17, snapshot.revision()); + assertEquals(mapper.readTree(json), mapper.readTree(mapper.writeValueAsString(snapshot))); + } + @Test void rejectsInvalidSnapshotBeforeStorage() { + assertThrows(IllegalArgumentException.class, () -> new RepositoryRegistration(UUID.randomUUID(), 0, "TEST-forge", null, "repo", "TEST/repo", true, false)); + } +} diff --git a/spire-contract/src/test/java/dev/codespire/contract/scm/ForgeOriginTest.java b/spire-contract/src/test/java/dev/codespire/contract/scm/ForgeOriginTest.java new file mode 100644 index 00000000..f4bff6c0 --- /dev/null +++ b/spire-contract/src/test/java/dev/codespire/contract/scm/ForgeOriginTest.java @@ -0,0 +1,18 @@ +package dev.codespire.contract.scm; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +class ForgeOriginTest { + @Test void canonicalizesApiPathsAndDefaultPorts() { + assertEquals("https://forge.example.test", ForgeOrigin.of("HTTPS://FORGE.example.test:443/api/v4")); + assertEquals("http://forge.example.test:8080", ForgeOrigin.of("http://forge.example.test:8080/api")); + assertNotEquals(ForgeOrigin.of("https://a.example.test"), ForgeOrigin.of("https://b.example.test")); + } + @Test void rejectsAmbiguousOrSecretBearingUrls() { + for (String value : new String[]{"", "forge.example.test", "ftp://forge.example.test", "https://user:secret@forge.example.test", + "https://forge.example.test?token=TEST-secret", "https://forge.example.test#fragment"}) { + assertThrows(IllegalArgumentException.class, () -> ForgeOrigin.of(value)); + } + } +} diff --git a/spire-contract/src/test/resources/contract-schema.txt b/spire-contract/src/test/resources/contract-schema.txt index d3e4e39c..6edf6869 100644 --- a/spire-contract/src/test/resources/contract-schema.txt +++ b/spire-contract/src/test/resources/contract-schema.txt @@ -52,3 +52,6 @@ RunStarted(runId: java.lang.String, providerRunId: java.lang.String) # ContextCredential ContextCredential(type: java.lang.String, platform: java.lang.String, baseUrl: java.lang.String, authKind: java.lang.String, username: java.lang.String, secret: java.lang.String, projectKeys: java.lang.String) + +# RepositoryRegistration +RepositoryRegistration(registrationId: java.util.UUID, revision: long, providerType: java.lang.String, forgeOrigin: java.lang.String, scope: java.lang.String, target: java.lang.String, enabled: boolean, deleted: boolean) diff --git a/spire-gateway/build.gradle.kts b/spire-gateway/build.gradle.kts index 960cbfa8..41f6e59c 100644 --- a/spire-gateway/build.gradle.kts +++ b/spire-gateway/build.gradle.kts @@ -38,6 +38,7 @@ dependencies { // other's — the alternative was a bus message, which would put a non-reviewId class on cs.*. implementation("io.quarkus:quarkus-websockets-next") implementation("io.quarkus:quarkus-messaging-kafka") + implementation("io.quarkus:quarkus-scheduler") // durable registration snapshot outbox implementation("io.quarkus:quarkus-config-yaml") implementation("io.quarkus:quarkus-smallrye-health") implementation("io.quarkus:quarkus-logging-json") // structured JSON logs in prod (plain console in dev/test) diff --git a/spire-gateway/src/main/java/dev/codespire/gateway/registry/RepositorySnapshotPublisher.java b/spire-gateway/src/main/java/dev/codespire/gateway/registry/RepositorySnapshotPublisher.java new file mode 100644 index 00000000..0e65986c --- /dev/null +++ b/spire-gateway/src/main/java/dev/codespire/gateway/registry/RepositorySnapshotPublisher.java @@ -0,0 +1,62 @@ +package dev.codespire.gateway.registry; + +import io.quarkus.scheduler.Scheduled; +import io.smallrye.reactive.messaging.kafka.api.OutgoingKafkaRecordMetadata; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import org.eclipse.microprofile.reactive.messaging.Channel; +import org.eclipse.microprofile.reactive.messaging.Emitter; +import org.eclipse.microprofile.reactive.messaging.Message; +import org.eclipse.microprofile.reactive.messaging.Metadata; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import javax.sql.DataSource; + +/** Broker acknowledgement precedes sent_at; a crash in between redelivers the same revision safely. */ +@ApplicationScoped +public class RepositorySnapshotPublisher { + @Inject DataSource dataSource; + @Inject @Channel("registry-out") Emitter emitter; + + @Scheduled(every = "${spire.repository-publish-interval:5s}", delayed = "15s", concurrentExecution = Scheduled.ConcurrentExecution.SKIP) + public void publishPending() throws Exception { + for (Snapshot snapshot : pending()) { + send(snapshot).get(10, TimeUnit.SECONDS); + markSent(snapshot.revision()); + } + } + + CompletableFuture send(Snapshot snapshot) { + CompletableFuture acknowledgement = new CompletableFuture<>(); + emitter.send(Message.of(snapshot.payload(), Metadata.of(OutgoingKafkaRecordMetadata.builder() + .withKey(snapshot.registrationId()).build()), + () -> { acknowledgement.complete(null); return CompletableFuture.completedFuture(null); }, + failure -> { acknowledgement.completeExceptionally(failure); return CompletableFuture.completedFuture(null); })); + return acknowledgement; + } + + List pending() throws SQLException { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement( + "SELECT revision,registration_id,payload FROM repository_snapshot_outbox WHERE sent_at IS NULL ORDER BY revision LIMIT 100"); + ResultSet rows = statement.executeQuery()) { + List snapshots = new ArrayList<>(); + while (rows.next()) snapshots.add(new Snapshot(rows.getLong(1), rows.getString(2), rows.getString(3))); + return snapshots; + } + } + + void markSent(long revision) throws SQLException { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement( + "UPDATE repository_snapshot_outbox SET sent_at=now() WHERE revision=? AND sent_at IS NULL")) { + statement.setLong(1, revision); statement.executeUpdate(); + } + } + + record Snapshot(long revision, String registrationId, String payload) { } +} diff --git a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoInput.java b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoInput.java index 83266c49..daa37a2a 100644 --- a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoInput.java +++ b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoInput.java @@ -14,5 +14,14 @@ public record WebhookRepoInput( String providerType, String scope, String target, - Boolean enabled) { + Boolean enabled, + String forgeOrigin) { + public WebhookRepoInput { + if (forgeOrigin != null) forgeOrigin = dev.codespire.contract.scm.ForgeOrigin.of(forgeOrigin); + } + + /** Legacy callers omit origin; updates preserve an origin already supplied by an operator. */ + public WebhookRepoInput(String providerType, String scope, String target, Boolean enabled) { + this(providerType, scope, target, enabled, null); + } } diff --git a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoRegistry.java b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoRegistry.java index 5ab4843b..b9484748 100644 --- a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoRegistry.java +++ b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoRegistry.java @@ -101,8 +101,8 @@ public WebhookRepoSecret create(WebhookRepoInput in) { String key = newWebhookKey(); try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(""" - INSERT INTO webhook_repo (id, provider_type, scope, target, webhook_key, webhook_secret, enabled) - VALUES (?, ?, ?, ?, ?, ?, ?) + INSERT INTO webhook_repo (id, provider_type, scope, target, webhook_key, webhook_secret, enabled, forge_origin) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) """)) { ps.setObject(1, id); ps.setString(2, in.providerType()); @@ -111,6 +111,7 @@ INSERT INTO webhook_repo (id, provider_type, scope, target, webhook_key, webhook ps.setString(5, key); ps.setString(6, encryption.encryptString(secret, aad(id))); ps.setBoolean(7, in.enabled() == null || in.enabled()); + ps.setString(8, in.forgeOrigin()); ps.executeUpdate(); } catch (SQLException e) { throw new IllegalStateException("Failed to create webhook repo", e); @@ -126,13 +127,14 @@ public Optional update(UUID id, WebhookRepoInput in) { return Optional.empty(); } try (PreparedStatement ps = c.prepareStatement( - "UPDATE webhook_repo SET provider_type=?, scope=?, target=?, enabled=?, updated_at=now() " + "UPDATE webhook_repo SET provider_type=?, scope=?, target=?, enabled=?, forge_origin=COALESCE(?,forge_origin), updated_at=now() " + "WHERE id=?")) { ps.setString(1, in.providerType()); ps.setString(2, in.scope()); ps.setString(3, in.target().trim()); ps.setBoolean(4, in.enabled() == null || in.enabled()); - ps.setObject(5, id); + ps.setString(5, in.forgeOrigin()); + ps.setObject(6, id); ps.executeUpdate(); } } catch (SQLException e) { @@ -283,7 +285,8 @@ private WebhookRepoView toView(ResultSet rs) throws SQLException { rs.getString("webhook_key"), secret != null && !secret.isBlank(), rs.getBoolean("enabled"), - rs.getTimestamp("created_at").toInstant()); + rs.getTimestamp("created_at").toInstant(), + rs.getString("forge_origin")); } private boolean exists(Connection c, UUID id) throws SQLException { diff --git a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoView.java b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoView.java index 4c89be74..f0f0542d 100644 --- a/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoView.java +++ b/spire-gateway/src/main/java/dev/codespire/gateway/registry/WebhookRepoView.java @@ -17,5 +17,6 @@ public record WebhookRepoView( String webhookKey, boolean hasSecret, boolean enabled, - Instant createdAt) { + Instant createdAt, + String forgeOrigin) { } diff --git a/spire-gateway/src/main/resources/application.yml b/spire-gateway/src/main/resources/application.yml index d44ef6b1..a00fd2cb 100644 --- a/spire-gateway/src/main/resources/application.yml +++ b/spire-gateway/src/main/resources/application.yml @@ -120,6 +120,14 @@ spire: mp: messaging: outgoing: + registry-out: + connector: smallrye-kafka + topic: cs.registry-integration + acks: all + key: + serializer: org.apache.kafka.common.serialization.StringSerializer + value: + serializer: org.apache.kafka.common.serialization.StringSerializer integration-out: connector: smallrye-kafka topic: cs.integration @@ -212,6 +220,8 @@ mp: json: enabled: false # plain console keeps test output readable spire: + # The snapshot scheduler is exercised explicitly by its outbox tests. + repository-publish-interval: "off" encryption: # Test-only webhook keyset (not a secret) — matches the orchestrator's so a # secret encrypted there decrypts here. Prod/dev read SPIRE_ENCRYPTION_WEBHOOK_KEYSET. diff --git a/spire-gateway/src/main/resources/db/migration/V3__repository_snapshot_outbox.sql b/spire-gateway/src/main/resources/db/migration/V3__repository_snapshot_outbox.sql new file mode 100644 index 00000000..43377c68 --- /dev/null +++ b/spire-gateway/src/main/resources/db/migration/V3__repository_snapshot_outbox.sql @@ -0,0 +1,30 @@ +-- Metadata only: signatures/credentials never cross to the orchestrator registry channel. +ALTER TABLE webhook_repo ADD COLUMN forge_origin TEXT; +CREATE TABLE repository_snapshot_outbox ( + revision BIGSERIAL PRIMARY KEY, + registration_id UUID NOT NULL, + payload JSONB NOT NULL, + sent_at TIMESTAMPTZ +); +CREATE FUNCTION queue_repository_snapshot() RETURNS trigger LANGUAGE plpgsql AS $$ +DECLARE row_data webhook_repo; next_revision BIGINT; +BEGIN + IF TG_OP = 'DELETE' THEN row_data := OLD; ELSE row_data := NEW; END IF; + next_revision := nextval('repository_snapshot_outbox_revision_seq'); + INSERT INTO repository_snapshot_outbox(revision,registration_id,payload) + VALUES (next_revision,row_data.id,jsonb_build_object( + 'type','RepositoryRegistration', + 'registrationId',row_data.id,'revision',next_revision,'providerType',row_data.provider_type, + 'forgeOrigin',row_data.forge_origin,'scope',row_data.scope,'target',row_data.target, + 'enabled',row_data.enabled,'deleted',TG_OP = 'DELETE')); + RETURN row_data; +END $$; +CREATE TRIGGER repository_snapshot_created AFTER INSERT ON webhook_repo + FOR EACH ROW EXECUTE FUNCTION queue_repository_snapshot(); +CREATE TRIGGER repository_snapshot_changed AFTER UPDATE OF provider_type,scope,target,enabled,forge_origin ON webhook_repo + FOR EACH ROW EXECUTE FUNCTION queue_repository_snapshot(); +CREATE TRIGGER repository_snapshot_deleted AFTER DELETE ON webhook_repo + FOR EACH ROW EXECUTE FUNCTION queue_repository_snapshot(); +-- Populate the outbox atomically for every existing registration without changing its key or secret. +UPDATE webhook_repo SET forge_origin=forge_origin; +CREATE INDEX repository_snapshot_pending ON repository_snapshot_outbox(revision) WHERE sent_at IS NULL; diff --git a/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotMigrationTest.java b/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotMigrationTest.java new file mode 100644 index 00000000..9a5de44c --- /dev/null +++ b/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotMigrationTest.java @@ -0,0 +1,41 @@ +package dev.codespire.gateway.registry; + +import dev.codespire.encryption.EncryptionService; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.flywaydb.core.Flyway; +import org.junit.jupiter.api.Test; +import javax.sql.DataSource; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +class RepositorySnapshotMigrationTest { + @Inject DataSource dataSource; + @Inject EncryptionService encryption; + + @Test void upgradeQueuesExistingRegistrationWithoutChangingKeyOrSecret() throws Exception { + String schema = "test_registry_upgrade_" + UUID.randomUUID().toString().replace("-", ""); + UUID id = UUID.randomUUID(); + String ciphertext = encryption.encryptString("TEST-webhook-secret", "webhook:" + id); + Flyway.configure().dataSource(dataSource).schemas(schema).defaultSchema(schema).target("2").load().migrate(); + try (var c = dataSource.getConnection()) { + String previous = c.getSchema(); + try { + c.setSchema(schema); + try (var ps = c.prepareStatement("INSERT INTO webhook_repo (id,provider_type,scope,target,webhook_key,webhook_secret) VALUES (?,'gitlab','repo','TEST-group/nested/TEST-repo','TEST-routing-key',?)")) { + ps.setObject(1, id); ps.setString(2, ciphertext); ps.executeUpdate(); + } + Flyway.configure().dataSource(dataSource).schemas(schema).defaultSchema(schema).target("3").load().migrate(); + try (var st = c.createStatement(); var rs = st.executeQuery("SELECT w.id,w.webhook_key,w.webhook_secret,o.payload::text FROM webhook_repo w JOIN repository_snapshot_outbox o ON o.registration_id=w.id")) { + assertTrue(rs.next()); assertEquals(id, rs.getObject(1, UUID.class)); assertEquals("TEST-routing-key", rs.getString(2)); + assertEquals(ciphertext, rs.getString(3)); assertEquals("TEST-webhook-secret", encryption.decryptString(rs.getString(3), "webhook:" + id)); + assertFalse(rs.getString(4).contains("TEST-webhook-secret")); assertFalse(rs.getString(4).contains(ciphertext)); assertFalse(rs.next()); + } + } finally { + c.setSchema(previous); + try (var st = c.createStatement()) { st.execute("DROP SCHEMA " + schema + " CASCADE"); } + } + } + } +} diff --git a/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotPublisherTest.java b/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotPublisherTest.java new file mode 100644 index 00000000..a96c640d --- /dev/null +++ b/spire-gateway/src/test/java/dev/codespire/gateway/registry/RepositorySnapshotPublisherTest.java @@ -0,0 +1,124 @@ +package dev.codespire.gateway.registry; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.codespire.contract.event.RepositoryRegistration; +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.kafka.InjectKafkaCompanion; +import io.quarkus.test.kafka.KafkaCompanionResource; +import io.smallrye.reactive.messaging.kafka.companion.KafkaCompanion; +import jakarta.inject.Inject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import javax.sql.DataSource; +import java.time.Duration; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +@QuarkusTestResource(KafkaCompanionResource.class) +class RepositorySnapshotPublisherTest { + @Inject DataSource dataSource; + @Inject WebhookRepoRegistry registry; + @Inject RepositorySnapshotPublisher publisher; + @Inject ObjectMapper mapper; + @InjectKafkaCompanion KafkaCompanion companion; + UUID registrationId; + + WebhookRepoSecret create() { + var created = registry.create(new WebhookRepoInput("gitlab", "repo", "TEST-group/" + UUID.randomUUID(), true)); + registrationId = UUID.fromString(created.repo().id()); + return created; + } + List ownPending() throws Exception { + try (var c = dataSource.getConnection(); var ps = c.prepareStatement( + "SELECT revision,payload FROM repository_snapshot_outbox WHERE registration_id=? AND sent_at IS NULL ORDER BY revision")) { + ps.setObject(1, registrationId); + try (var rows = ps.executeQuery()) { + var result = new java.util.ArrayList(); + while (rows.next()) result.add(new RepositorySnapshotPublisher.Snapshot(rows.getLong(1), registrationId.toString(), rows.getString(2))); + return result; + } + } + } + + @AfterEach void removeFixture() throws Exception { + if (registrationId == null) return; + registry.delete(registrationId); + try (var c = dataSource.getConnection(); var ps = c.prepareStatement("DELETE FROM repository_snapshot_outbox WHERE registration_id=?")) { + ps.setObject(1, registrationId); ps.executeUpdate(); + } + } + + @Test void sendsOnlyMetadataWithStableKeyThroughTheBroker() throws Exception { + var created = create(); + var pending = ownPending().getFirst(); + var snapshot = mapper.readValue(pending.payload(), RepositoryRegistration.class); + assertEquals(registrationId, snapshot.registrationId()); + assertFalse(pending.payload().contains(created.secret())); + assertFalse(pending.payload().contains(created.repo().webhookKey())); + assertEquals(9, mapper.readTree(pending.payload()).size()); + try (var consumed = companion.consumeStrings().withGroupId("TEST-registry-" + registrationId) + .fromTopics("cs.registry-integration", Duration.ofSeconds(5))) { + publisher.send(pending).get(15, java.util.concurrent.TimeUnit.SECONDS); + consumed.awaitCompletion(Duration.ofSeconds(15)); + assertTrue(consumed.getRecords().stream().anyMatch(record -> record.key().equals(registrationId.toString()) + && record.value().equals(pending.payload()))); + } + } + + @Test void failedBrokerAcknowledgementKeepsTheOutboxForRetry() throws Exception { + create(); + var original = ownPending().getFirst(); + var failed = new RepositorySnapshotPublisher() { + @Override List pending() { return List.of(original); } + @Override CompletableFuture send(Snapshot snapshot) { return CompletableFuture.failedFuture(new IllegalStateException("TEST-broker-down")); } + }; + failed.dataSource = dataSource; + assertThrows(ExecutionException.class, failed::publishPending); + assertEquals(List.of(original), ownPending()); + var restarted = new RepositorySnapshotPublisher() { + @Override List pending() { return List.of(original); } + @Override CompletableFuture send(Snapshot snapshot) { return CompletableFuture.completedFuture(null); } + }; + restarted.dataSource = dataSource; + restarted.publishPending(); + assertTrue(ownPending().isEmpty()); + } + + @Test void updateAndDeletionHaveIncreasingDurableRevisions() throws Exception { + var created = create(); + registry.update(registrationId, new WebhookRepoInput("gitlab", "repo", created.repo().target(), false)); + registry.delete(registrationId); + var rows = ownPending(); + assertEquals(3, rows.size()); + assertTrue(rows.get(0).revision() < rows.get(1).revision() && rows.get(1).revision() < rows.get(2).revision()); + assertTrue(mapper.readValue(rows.getLast().payload(), RepositoryRegistration.class).deleted()); + assertFalse(mapper.readValue(rows.get(1).payload(), RepositoryRegistration.class).enabled()); + } + + @Test void rollbackDoesNotPublishAnUncommittedRegistration() throws Exception { + registrationId = UUID.randomUUID(); + try (var c = dataSource.getConnection()) { + c.setAutoCommit(false); + try (var ps = c.prepareStatement("INSERT INTO webhook_repo (id,provider_type,scope,target,webhook_key,webhook_secret) VALUES (?,'gitlab','repo',? ,?,'TEST-cipher')")) { + ps.setObject(1, registrationId); ps.setString(2, "TEST/" + registrationId); ps.setString(3, "TEST-" + registrationId); ps.executeUpdate(); + } finally { c.rollback(); c.setAutoCommit(true); } + } + assertTrue(ownPending().isEmpty()); + } + + @Test void preservesConfiguredOriginWhenALegacyClientEditsRegistration() throws Exception { + var created = registry.create(new WebhookRepoInput("gitlab", "repo", "TEST-group/" + UUID.randomUUID(), true, + "https://TEST-forge.example.test:443/api/v4")); + registrationId = UUID.fromString(created.repo().id()); + registry.update(registrationId, new WebhookRepoInput("gitlab", "repo", created.repo().target(), false)); + assertEquals("https://test-forge.example.test", registry.get(registrationId).orElseThrow().forgeOrigin()); + for (var row : ownPending()) { + assertEquals("https://test-forge.example.test", mapper.readValue(row.payload(), RepositoryRegistration.class).forgeOrigin()); + } + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/attention/AttentionQueries.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/attention/AttentionQueries.java index 1d338b55..ea896c6a 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/attention/AttentionQueries.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/attention/AttentionQueries.java @@ -79,6 +79,7 @@ public List collect() { scmProviderRows(c, rows); accountScopeRows(c, rows); contextMigrationRows(c, rows); + dev.codespire.orchestrator.repository.RepositoryAttentionRows.collect(c, rows); reviewRows(c, rows); degradedReviewRows(c, rows); runRows.collect(c, rows); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/dlq/DlqTopics.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/dlq/DlqTopics.java index 2b07a22e..bf774ae7 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/dlq/DlqTopics.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/dlq/DlqTopics.java @@ -54,6 +54,7 @@ private DlqTopics() { /** Unknown/blank types fall back to cs.commands — the biggest DLQ source is AnswerFollowUp. */ static String forType(String type) { + if ("RepositoryRegistration".equals(type)) return "cs.registry-integration"; if (ACTION_COMMAND_TYPES.contains(type)) { return COMMANDS; } diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderClients.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderClients.java index 4179da7e..826ebf62 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderClients.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderClients.java @@ -47,6 +47,14 @@ public class ProviderClients { /** Account kinds also include credentials for the context-only adapters. */ public static final Set ACCOUNT_TYPES = Set.of("bitbucket-cloud", "github", "gitlab", "atlassian"); + /** Translate a persisted repository web URL to the account API origin; host aliases live here. */ + public static String repositoryForgeOrigin(String type, String webUrl) { + String origin = dev.codespire.contract.scm.ForgeOrigin.of(webUrl); + if ("github".equals(type) && "https://github.com".equals(origin)) return "https://api.github.com"; + if ("bitbucket-cloud".equals(type) && "https://bitbucket.org".equals(origin)) return "https://api.bitbucket.org"; + return origin; + } + public static boolean supportsContext(String source, String account) { return switch (source) { case "jira", "confluence" -> "atlassian".equals(account); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderRegistry.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderRegistry.java index e07127c9..111f4714 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderRegistry.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/provider/ProviderRegistry.java @@ -115,6 +115,7 @@ public Optional update(UUID id, ProviderInput in) { } } validateSourceReferences(c, id, in); + validateRepositoryReferences(c, id, in); boolean rotateSecret = in.secret() != null && !in.secret().isBlank(); // bot_username is refreshed only when the token was (re)validated; a token-less // update leaves the stored login intact (mirrors the rotateSecret conditional). @@ -418,9 +419,31 @@ private List usedBy(Connection c, UUID id, String role) throws SQLExcept ps.setObject(1, id); try (var rs = ps.executeQuery()) { while (rs.next()) uses.add(rs.getString(1)); } } + try (PreparedStatement ps = c.prepareStatement("SELECT r.forge_origin,r.workspace,r.slug FROM repository r " + + "JOIN repository_account a ON a.repository_id=r.id WHERE a.account_id=? ORDER BY r.id")) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { + while (rs.next()) uses.add(rs.getString(1) + "/" + rs.getString(2) + "/" + rs.getString(3)); + } + } return uses; } + private void validateRepositoryReferences(Connection c, UUID id, ProviderInput in) throws SQLException { + try (PreparedStatement ps = c.prepareStatement("SELECT r.scm_type,r.forge_origin FROM repository r " + + "JOIN repository_account a ON a.repository_id=r.id WHERE a.account_id=?")) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { + while (rs.next()) { + if (!rs.getString(1).equals(in.type()) || !rs.getString(2).equals( + dev.codespire.contract.scm.ForgeOrigin.of(in.baseUrl()))) { + throw new AccountConflict("Reassign the referencing repositories before changing account kind or origin"); + } + } + } + } + } + private void validateSourceReferences(Connection c, UUID id, ProviderInput in) throws SQLException { try (var ps = c.prepareStatement("SELECT name, type, base_url FROM context_provider WHERE account_id = ?")) { ps.setObject(1, id); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAccounts.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAccounts.java new file mode 100644 index 00000000..7a0d2ae6 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAccounts.java @@ -0,0 +1,50 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.scm.ForgeOrigin; +import dev.codespire.orchestrator.provider.ProviderRegistry; +import dev.codespire.orchestrator.provider.ProviderRole; +import dev.codespire.orchestrator.provider.ScmProvider; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.Optional; +import java.util.UUID; +import javax.sql.DataSource; + +/** Explicit repository resolution. Legacy pipeline callers remain unchanged during slice 1. */ +@ApplicationScoped +public class RepositoryAccounts { + @Inject DataSource dataSource; + @Inject ProviderRegistry providers; + + public Optional resolve(UUID repositoryId, ProviderRole role) { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(""" + SELECT a.account_id, r.scm_type, r.forge_origin, peer.bot_account_id peer_identity FROM repository r + JOIN repository_account a ON a.repository_id = r.id + LEFT JOIN repository_account other ON other.repository_id=r.id AND other.role<>a.role + LEFT JOIN scm_provider peer ON peer.id=other.account_id + WHERE r.id = ? AND a.role = ? AND r.enabled = TRUE + ORDER BY a.role + """)) { + statement.setObject(1, repositoryId); + statement.setString(2, role.name()); + try (ResultSet rows = statement.executeQuery()) { + if (!rows.next()) return Optional.empty(); + String type = rows.getString("scm_type"); + String origin = rows.getString("forge_origin"); + String peerIdentity = rows.getString("peer_identity"); + return providers.resolveById(rows.getObject("account_id", UUID.class)) + .filter(ScmProvider::enabled) + .filter(account -> account.role() == role) + .filter(account -> account.type().equals(type)) + .filter(account -> peerIdentity == null || peerIdentity.isBlank() || !peerIdentity.equals(account.botAccountId())) + .filter(account -> ForgeOrigin.of(account.baseUrl()).equals(origin)); + } + } catch (SQLException failure) { + throw new IllegalStateException("Cannot resolve repository account", failure); + } + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAttentionRows.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAttentionRows.java new file mode 100644 index 00000000..a1d9b094 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryAttentionRows.java @@ -0,0 +1,30 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.attention.AttentionView; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.List; + +/** Migration problems are current conditions; fixing the explicit mapping removes the row. */ +public final class RepositoryAttentionRows { + private RepositoryAttentionRows() { } + + public static void collect(Connection connection, List rows) throws SQLException { + try (PreparedStatement statement = connection.prepareStatement(""" + SELECT registration_id,provider_type,forge_origin,target,problem FROM repository_registration_bridge + WHERE problem IS NOT NULL AND deleted=FALSE ORDER BY registration_id + """); ResultSet results = statement.executeQuery()) { + while (results.next()) { + String id = results.getString("registration_id"); + String origin = results.getString("forge_origin"); + rows.add(new AttentionView("REPOSITORY_MAPPING_PENDING", AttentionView.Severity.WARNING, id, + results.getString("provider_type") + " " + results.getString("target") + " (" + + (origin == null ? "origin unresolved" : origin) + "): " + results.getString("problem") + + ". Select the repository and accounts for registration " + id + ".", + "/settings/repositories/registry?registration=" + id)); + } + } + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryBindings.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryBindings.java new file mode 100644 index 00000000..08549d95 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryBindings.java @@ -0,0 +1,64 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.scm.ForgeOrigin; +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import jakarta.enterprise.context.ApplicationScoped; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.UUID; + +/** Lock accounts before binding, sharing the provider edit/delete lock so validation cannot race edits. */ +@ApplicationScoped +public class RepositoryBindings { + public void replace(Connection connection, UUID repositoryId, RepositoryInput input) throws SQLException { + validate(connection, input); + try (PreparedStatement statement = connection.prepareStatement("DELETE FROM repository_account WHERE repository_id = ?")) { + statement.setObject(1, repositoryId); + statement.executeUpdate(); + } + insert(connection, repositoryId, new Binding(input.reviewerAccountId(), "REVIEWER")); + insert(connection, repositoryId, new Binding(input.factoryAccountId(), "FACTORY")); + } + + public void validate(Connection connection, RepositoryInput input) throws SQLException { + String reviewer = validate(connection, input.reviewerAccountId(), new Expected(input, "REVIEWER")); + String factory = validate(connection, input.factoryAccountId(), new Expected(input, "FACTORY")); + if (!reviewer.isBlank() && reviewer.equals(factory)) { + throw new AccountConflict("Reviewer and factory must use different resolved identities"); + } + } + + private String validate(Connection connection, UUID accountId, Expected expected) throws SQLException { + if (accountId == null) return ""; + try (PreparedStatement statement = connection.prepareStatement( + "SELECT type, base_url, role, bot_account_id FROM scm_provider WHERE id = ? FOR UPDATE")) { + statement.setObject(1, accountId); + try (ResultSet rows = statement.executeQuery()) { + if (!rows.next()) throw new AccountConflict("Selected account no longer exists"); + if (!rows.getString("role").equals(expected.role())) throw new AccountConflict("Selected account has the wrong role"); + if (!rows.getString("type").equals(expected.input().scmType())) throw new AccountConflict("Selected account has the wrong forge kind"); + if (!ForgeOrigin.of(rows.getString("base_url")).equals(expected.input().forgeOrigin())) { + throw new AccountConflict("Selected account belongs to another forge origin"); + } + String identity = rows.getString("bot_account_id"); + return identity == null ? "" : identity; + } + } + } + + private void insert(Connection connection, UUID repositoryId, Binding binding) throws SQLException { + if (binding.accountId() == null) return; + try (PreparedStatement statement = connection.prepareStatement( + "INSERT INTO repository_account (repository_id,account_id,role) VALUES (?,?,?)")) { + statement.setObject(1, repositoryId); + statement.setObject(2, binding.accountId()); + statement.setString(3, binding.role()); + statement.executeUpdate(); + } + } + + private record Expected(RepositoryInput input, String role) { } + private record Binding(UUID accountId, String role) { } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryHistoryBridge.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryHistoryBridge.java new file mode 100644 index 00000000..d94a80f2 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryHistoryBridge.java @@ -0,0 +1,88 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.event.RepositoryRegistration; +import io.quarkus.scheduler.Scheduled; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import java.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import javax.sql.DataSource; + +/** Bounded bridge sweep for actual history, including repositories first seen under a legacy org hook. */ +@ApplicationScoped +public class RepositoryHistoryBridge { + @Inject DataSource dataSource; + @Inject RepositoryMigrationBridge bridge; + + @Scheduled(every = "${spire.repository-history-interval:30s}", delayed = "15s", concurrentExecution = Scheduled.ConcurrentExecution.SKIP) + public void sweep() { + List histories = pending(); + for (History history : histories) importHistory(history); + } + + private List pending() { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(""" + SELECT DISTINCT provider_type,workspace,slug FROM ( + SELECT provider_type,workspace,slug FROM review_status WHERE repository_id IS NULL AND provider_type<>'' + UNION SELECT provider_type,workspace,slug FROM factory_run WHERE repository_id IS NULL + ) h WHERE NOT EXISTS (SELECT 1 FROM repository_registration_bridge b + WHERE b.provider_type=h.provider_type AND b.target=h.workspace || '/' || h.slug AND b.problem IS NOT NULL) + ORDER BY provider_type,workspace,slug LIMIT 100 + """); ResultSet rows = statement.executeQuery()) { + List histories = new ArrayList<>(); + while (rows.next()) histories.add(new History(rows.getString(1), rows.getString(2), rows.getString(3))); + return histories; + } catch (SQLException failure) { throw new IllegalStateException("Cannot enumerate repository history", failure); } + } + + @Transactional + public void importHistory(History history) { + UUID id = UUID.nameUUIDFromBytes(("repository-history:" + history.type() + ":" + history.workspace() + "/" + history.slug()) + .getBytes(StandardCharsets.UTF_8)); + bridge.apply(new RepositoryRegistration(id, 1, history.type(), origin(history), "repo", + history.workspace() + "/" + history.slug(), true, false)); + try (Connection connection = dataSource.getConnection()) { + link(connection, "review_status", new HistoryLink(id, history)); + link(connection, "factory_run", new HistoryLink(id, history)); + } catch (SQLException failure) { throw new IllegalStateException("Cannot link repository history", failure); } + } + + private String origin(History history) { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(""" + SELECT DISTINCT html_url FROM review_status WHERE provider_type=? AND workspace=? AND slug=? + """)) { + statement.setString(1, history.type()); statement.setString(2, history.workspace()); statement.setString(3, history.slug()); + java.util.Set origins = new java.util.HashSet<>(); + try (ResultSet rows = statement.executeQuery()) { + while (rows.next()) { + String url = rows.getString(1); + if (url == null || url.isBlank()) return null; + origins.add(dev.codespire.orchestrator.provider.ProviderClients.repositoryForgeOrigin(history.type(), url)); + } + } + return origins.size() == 1 ? origins.iterator().next() : null; + } catch (IllegalArgumentException invalidUrl) { return null; } + catch (SQLException failure) { throw new IllegalStateException("Cannot read repository origin evidence", failure); } + } + + private void link(Connection connection, String table, HistoryLink link) throws SQLException { + // Table comes only from the two literals above, never from registry or webhook input. + try (PreparedStatement statement = connection.prepareStatement("UPDATE " + table + " SET repository_id=" + + "(SELECT repository_id FROM repository_registration_bridge WHERE registration_id=?) " + + "WHERE repository_id IS NULL AND provider_type=? AND workspace=? AND slug=?")) { + statement.setObject(1, link.registrationId()); statement.setString(2, link.history().type()); + statement.setString(3, link.history().workspace()); statement.setString(4, link.history().slug()); + statement.executeUpdate(); + } + } + + public record History(String type, String workspace, String slug) { } + private record HistoryLink(UUID registrationId, History history) { } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryInput.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryInput.java new file mode 100644 index 00000000..53c92f3a --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryInput.java @@ -0,0 +1,7 @@ +package dev.codespire.orchestrator.repository; + +import java.util.UUID; + +/** Credentials stay on accounts; a repository input carries selected account ids only. */ +public record RepositoryInput(String scmType, String forgeOrigin, String workspace, String slug, + Boolean enabled, UUID reviewerAccountId, UUID factoryAccountId) { } diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMappings.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMappings.java new file mode 100644 index 00000000..372d6d3f --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMappings.java @@ -0,0 +1,56 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.scm.ForgeOrigin; +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import javax.sql.DataSource; + +/** Explicit repair of an ambiguous legacy registration; never guesses a host or grants a role. */ +@ApplicationScoped +public class RepositoryMappings { + @Inject DataSource dataSource; + @Inject RepositoryRegistry repositories; + + public List pending() { + try (var connection = dataSource.getConnection(); var statement = connection.prepareStatement(""" + SELECT registration_id,revision,provider_type,forge_origin,target,problem + FROM repository_registration_bridge WHERE problem IS NOT NULL AND NOT deleted ORDER BY target,registration_id + """); var rows = statement.executeQuery()) { + List result = new ArrayList<>(); + while (rows.next()) result.add(new Pending(rows.getObject(1, UUID.class), rows.getLong(2), + rows.getString(3), rows.getString(4), rows.getString(5), rows.getString(6))); + return result; + } catch (SQLException failure) { throw new IllegalStateException("Cannot read pending repository mappings", failure); } + } + + @Transactional + public void link(UUID registration, long revision, UUID repository) { + RepositoryView selected = repositories.get(repository).orElseThrow(() -> new AccountConflict("Register the repository first")); + try (var connection = dataSource.getConnection(); var statement = connection.prepareStatement(""" + SELECT provider_type,forge_origin,target FROM repository_registration_bridge + WHERE registration_id=? AND revision=? AND NOT deleted AND problem IS NOT NULL FOR UPDATE + """)) { + statement.setObject(1, registration); statement.setLong(2, revision); + try (var rows = statement.executeQuery()) { + if (!rows.next()) throw new AccountConflict("Registration changed; reload before linking"); + if (!rows.getString(1).equals(selected.scmType()) + || !rows.getString(3).equals(selected.workspace() + "/" + selected.slug()) + || rows.getString(2) != null && !ForgeOrigin.of(rows.getString(2)).equals(selected.forgeOrigin())) { + throw new AccountConflict("Selected repository does not match the registration coordinates"); + } + } + try (var update = connection.prepareStatement("UPDATE repository_registration_bridge SET repository_id=?,problem=NULL WHERE registration_id=?")) { + update.setObject(1, repository); update.setObject(2, registration); update.executeUpdate(); + } + } catch (SQLException failure) { throw new IllegalStateException("Cannot link repository registration", failure); } + } + + public record Pending(UUID registrationId, long revision, String scmType, String forgeOrigin, String target, String problem) { } + public record Selection(UUID repositoryId, long revision) { } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridge.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridge.java new file mode 100644 index 00000000..09fce367 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridge.java @@ -0,0 +1,151 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.event.RepositoryRegistration; +import dev.codespire.contract.scm.ForgeOrigin; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import javax.sql.DataSource; + +/** Replayable gateway metadata bridge. Existing/operator-edited bindings are never overwritten. */ +@ApplicationScoped +public class RepositoryMigrationBridge { + @Inject DataSource dataSource; + @Inject RepositoryRegistry repositories; + @Inject RepositoryBindings bindings; + + @Transactional + public void apply(RepositoryRegistration snapshot) { + try (Connection connection = dataSource.getConnection()) { + if (!accept(connection, snapshot)) return; + if (snapshot.deleted() || !"repo".equals(snapshot.scope())) return; + try (PreparedStatement mapped = connection.prepareStatement( + "SELECT repository_id FROM repository_registration_bridge WHERE registration_id=?")) { + mapped.setObject(1, snapshot.registrationId()); + try (ResultSet rows = mapped.executeQuery()) { if (rows.next() && rows.getObject(1) != null) return; } + } + String target = snapshot.target(); + int slash = target.lastIndexOf('/'); + if (slash < 1 || slash == target.length() - 1) { + finish(connection, snapshot.registrationId(), new Result(null, "invalid_repository_path")); + return; + } + Coordinates coordinates = new Coordinates(snapshot.providerType(), target.substring(0, slash), target.substring(slash + 1)); + finish(connection, snapshot.registrationId(), reconcile(connection, snapshot, coordinates)); + } catch (SQLException failure) { throw new IllegalStateException("Repository snapshot reconciliation failed", failure); } + } + + private boolean accept(Connection connection, RepositoryRegistration snapshot) throws SQLException { + try (PreparedStatement statement = connection.prepareStatement(""" + INSERT INTO repository_registration_bridge + (registration_id,revision,provider_type,forge_origin,scope,target,enabled,deleted) + VALUES (?,?,?,?,?,?,?,?) ON CONFLICT (registration_id) DO UPDATE SET + revision=EXCLUDED.revision,provider_type=EXCLUDED.provider_type,forge_origin=EXCLUDED.forge_origin, + scope=EXCLUDED.scope,target=EXCLUDED.target,enabled=EXCLUDED.enabled,deleted=EXCLUDED.deleted, + problem=NULL,repository_id=CASE WHEN repository_registration_bridge.provider_type=EXCLUDED.provider_type + AND repository_registration_bridge.target=EXCLUDED.target + AND repository_registration_bridge.scope=EXCLUDED.scope + AND repository_registration_bridge.forge_origin IS NOT DISTINCT FROM EXCLUDED.forge_origin + THEN repository_registration_bridge.repository_id ELSE NULL END + WHERE repository_registration_bridge.revision < EXCLUDED.revision + """)) { + statement.setObject(1, snapshot.registrationId()); statement.setLong(2, snapshot.revision()); + statement.setString(3, snapshot.providerType()); statement.setString(4, snapshot.forgeOrigin()); + statement.setString(5, snapshot.scope()); statement.setString(6, snapshot.target()); + statement.setBoolean(7, snapshot.enabled()); statement.setBoolean(8, snapshot.deleted()); + return statement.executeUpdate() == 1; + } + } + + private Result reconcile(Connection connection, RepositoryRegistration snapshot, Coordinates coordinates) throws SQLException { + List accounts = candidates(connection, coordinates); + List origins = accounts.stream().map(LegacyAccount::origin).distinct().toList(); + if (origins.size() != 1) return new Result(null, origins.isEmpty() ? "legacy_account_missing" : "conflicting_forge_origins"); + String origin = origins.getFirst(); + if (snapshot.forgeOrigin() == null) return new Result(null, "registration_origin_unknown"); + if (!ForgeOrigin.of(snapshot.forgeOrigin()).equals(origin)) { + return new Result(null, "registration_origin_mismatch"); + } + UUID existing = existing(connection, coordinates, origin); + if (existing != null) return new Result(existing, null); + UUID reviewer = account(accounts, "REVIEWER"); + UUID factory = account(accounts, "FACTORY"); + RepositoryInput input = new RepositoryInput(coordinates.type(), origin, coordinates.workspace(), coordinates.slug(), + true, reviewer, factory); + try { + input = repositories.normalize(input); + bindings.validate(connection, input); + } catch (IllegalArgumentException | dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict invalid) { + return new Result(null, "legacy_binding_invalid"); + } + UUID id = UUID.randomUUID(); + try (PreparedStatement statement = connection.prepareStatement(""" + INSERT INTO repository (id,scm_type,forge_origin,workspace,slug,enabled) VALUES (?,?,?,?,?,?) + ON CONFLICT (scm_type,forge_origin,workspace,slug) DO NOTHING + """)) { + statement.setObject(1, id); statement.setString(2, input.scmType()); + statement.setString(3, input.forgeOrigin()); statement.setString(4, input.workspace()); + statement.setString(5, input.slug()); statement.setBoolean(6, input.enabled()); + if (statement.executeUpdate() == 1) bindings.replace(connection, id, input); + else id = existing(connection, coordinates, origin); + } + return new Result(id, null); + } + + private List candidates(Connection connection, Coordinates coordinates) throws SQLException { + // Deleted/repurposed accounts cannot regain a binding from the immutable migration snapshot. + try (PreparedStatement statement = connection.prepareStatement(""" + SELECT l.account_id,l.base_url,l.role,p.base_url current_url,p.type current_type,p.role current_role + FROM repository_legacy_account l JOIN scm_provider p ON p.id=l.account_id + WHERE l.type=? AND l.workspace=? + """)) { + statement.setString(1, coordinates.type()); statement.setString(2, coordinates.workspace()); + List result = new ArrayList<>(); + try (ResultSet rows = statement.executeQuery()) { + while (rows.next()) { + String origin = ForgeOrigin.of(rows.getString("base_url")); + if (coordinates.type().equals(rows.getString("current_type")) + && rows.getString("role").equals(rows.getString("current_role")) + && origin.equals(ForgeOrigin.of(rows.getString("current_url")))) { + result.add(new LegacyAccount(rows.getObject("account_id", UUID.class), origin, rows.getString("role"))); + } + } + } + return result; + } + } + + private UUID existing(Connection connection, Coordinates coordinates, String origin) throws SQLException { + try (PreparedStatement statement = connection.prepareStatement( + "SELECT id FROM repository WHERE scm_type=? AND forge_origin=? AND workspace=? AND slug=?")) { + statement.setString(1, coordinates.type()); statement.setString(2, origin); + statement.setString(3, coordinates.workspace()); statement.setString(4, coordinates.slug()); + try (ResultSet rows = statement.executeQuery()) { return rows.next() ? rows.getObject(1, UUID.class) : null; } + } + } + + private UUID account(List accounts, String role) { + List matches = accounts.stream().filter(account -> account.role().equals(role)).map(LegacyAccount::id).toList(); + if (matches.size() > 1) throw new IllegalStateException("Ambiguous legacy role binding"); + return matches.isEmpty() ? null : matches.getFirst(); + } + + private void finish(Connection connection, UUID registrationId, Result result) throws SQLException { + try (PreparedStatement statement = connection.prepareStatement( + "UPDATE repository_registration_bridge SET repository_id=?, problem=? WHERE registration_id=?")) { + statement.setObject(1, result.repositoryId()); statement.setString(2, result.problem()); + statement.setObject(3, registrationId); statement.executeUpdate(); + } + } + + private record Coordinates(String type, String workspace, String slug) { } + private record LegacyAccount(UUID id, String origin, String role) { } + private record Result(UUID repositoryId, String problem) { } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryRegistry.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryRegistry.java new file mode 100644 index 00000000..5a566501 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryRegistry.java @@ -0,0 +1,140 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.scm.ForgeOrigin; +import dev.codespire.orchestrator.provider.ProviderClients; +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import javax.sql.DataSource; + +/** Repository configuration, separate from the gateway's registration and credential stores. */ +@ApplicationScoped +public class RepositoryRegistry { + private static final String SELECT = """ + SELECT r.*, a.id reviewer_id, a.name reviewer_name, a.bot_username reviewer_handle, + a.enabled reviewer_enabled, a.bot_account_id reviewer_identity, + b.id factory_id, b.name factory_name, b.bot_username factory_handle, + b.enabled factory_enabled, b.bot_account_id factory_identity + FROM repository r + LEFT JOIN repository_account ra ON ra.repository_id=r.id AND ra.role='REVIEWER' + LEFT JOIN scm_provider a ON a.id=ra.account_id + LEFT JOIN repository_account rb ON rb.repository_id=r.id AND rb.role='FACTORY' + LEFT JOIN scm_provider b ON b.id=rb.account_id + """; + @Inject DataSource dataSource; + @Inject RepositoryBindings bindings; + + public List list() { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement( + SELECT + " ORDER BY r.forge_origin,r.workspace,r.slug"); ResultSet rows = statement.executeQuery()) { + List result = new ArrayList<>(); + while (rows.next()) result.add(view(rows)); + return result; + } catch (SQLException failure) { throw database(failure); } + } + + public Optional get(UUID id) { + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(SELECT + " WHERE r.id=?")) { + statement.setObject(1, id); + try (ResultSet rows = statement.executeQuery()) { return rows.next() ? Optional.of(view(rows)) : Optional.empty(); } + } catch (SQLException failure) { throw database(failure); } + } + + @Transactional + public RepositoryView create(RepositoryInput raw) { + RepositoryInput input = normalize(raw); + UUID id = UUID.randomUUID(); + try (Connection connection = dataSource.getConnection()) { + insert(connection, id, input); + bindings.replace(connection, id, input); + } catch (SQLException failure) { + if ("23505".equals(failure.getSQLState())) throw new AccountConflict("Repository is already registered at this forge origin"); + throw database(failure); + } + return get(id).orElseThrow(); + } + + @Transactional + public RepositoryView update(UUID id, long expectedRevision, RepositoryInput raw) { + RepositoryInput input = normalize(raw); + try (Connection connection = dataSource.getConnection(); PreparedStatement statement = connection.prepareStatement(""" + UPDATE repository SET enabled=?, revision=revision+1 + WHERE id=? AND revision=? AND scm_type=? AND forge_origin=? AND workspace=? AND slug=? + """)) { + statement.setBoolean(1, input.enabled()); + statement.setObject(2, id); + statement.setLong(3, expectedRevision); + statement.setString(4, input.scmType()); + statement.setString(5, input.forgeOrigin()); + statement.setString(6, input.workspace()); + statement.setString(7, input.slug()); + if (statement.executeUpdate() != 1) throw new AccountConflict("Repository changed or its coordinates differ; reload before saving"); + bindings.replace(connection, id, input); + } catch (SQLException failure) { throw database(failure); } + return get(id).orElseThrow(); + } + + private void insert(Connection connection, UUID id, RepositoryInput input) throws SQLException { + try (PreparedStatement statement = connection.prepareStatement( + "INSERT INTO repository (id,scm_type,forge_origin,workspace,slug,enabled) VALUES (?,?,?,?,?,?)")) { + statement.setObject(1, id); + statement.setString(2, input.scmType()); + statement.setString(3, input.forgeOrigin()); + statement.setString(4, input.workspace()); + statement.setString(5, input.slug()); + statement.setBoolean(6, input.enabled()); + statement.executeUpdate(); + } + } + + public RepositoryInput normalize(RepositoryInput input) { + if (input == null || input.scmType() == null || !ProviderClients.SUPPORTED_TYPES.contains(input.scmType())) throw new IllegalArgumentException("Select a supported forge kind"); + String workspace = path(input.workspace()); + String slug = path(input.slug()); + if (slug.contains("/")) throw new IllegalArgumentException("Repository slug cannot contain a slash"); + return new RepositoryInput(input.scmType(), ForgeOrigin.of(input.forgeOrigin()), workspace, slug, + input.enabled() == null || input.enabled(), input.reviewerAccountId(), input.factoryAccountId()); + } + + private String path(String value) { + if (value == null || value.isBlank()) throw new IllegalArgumentException("Workspace and repository slug are required"); + String result = value.trim(); + for (String part : result.split("/", -1)) { + if (part.isBlank() || part.equals(".") || part.equals("..") || part.contains("\\") + || part.chars().anyMatch(Character::isISOControl)) throw new IllegalArgumentException("Invalid repository path"); + } + return result; + } + + private RepositoryView view(ResultSet rows) throws SQLException { + return new RepositoryView(rows.getObject("id", UUID.class), rows.getString("scm_type"), rows.getString("forge_origin"), + rows.getString("workspace"), rows.getString("slug"), rows.getBoolean("enabled"), rows.getLong("revision"), + account(rows, "reviewer"), account(rows, "factory")); + } + + private RepositoryView.Account account(ResultSet rows, String role) throws SQLException { + UUID id = rows.getObject(role + "_id", UUID.class); + if (id == null) return null; + String handle = rows.getString(role + "_handle"); + String identity = rows.getString(role + "_identity"); + String peerIdentity = rows.getString(("reviewer".equals(role) ? "factory" : "reviewer") + "_identity"); + String state = !rows.getBoolean(role + "_enabled") ? "disabled" + : identity != null && !identity.isBlank() && identity.equals(peerIdentity) ? "identity-conflict" + : "factory".equals(role) && (handle == null || handle.isBlank()) ? "no-login" + : "reviewer".equals(role) && (identity == null || identity.isBlank()) ? "no-identity" : "configured"; + return new RepositoryView.Account(id, rows.getString(role + "_name"), role.toUpperCase(java.util.Locale.ROOT), handle, state); + } + + private IllegalStateException database(SQLException failure) { + return new IllegalStateException("Repository registry operation failed", failure); + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryResource.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryResource.java new file mode 100644 index 00000000..28603527 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryResource.java @@ -0,0 +1,70 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import jakarta.annotation.security.RolesAllowed; +import jakarta.inject.Inject; +import jakarta.ws.rs.BadRequestException; +import jakarta.ws.rs.ClientErrorException; +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.NotFoundException; +import jakarta.ws.rs.POST; +import jakarta.ws.rs.PUT; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.PathParam; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import java.util.List; +import java.util.UUID; + +/** Admin-only registry operations; registration grants no new runtime authority during the bridge. */ +@Path("/api/repositories") +@RolesAllowed("spire-admin") +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +public class RepositoryResource { + @Inject RepositoryRegistry registry; + @Inject RepositoryMappings mappings; + + @GET @Path("/kinds") + public List kinds() { return dev.codespire.orchestrator.provider.ProviderClients.SUPPORTED_TYPES.stream().sorted().toList(); } + + @GET @Path("/pending") + public List pending() { return mappings.pending(); } + + @PUT @Path("/pending/{id}") + public Response link(@PathParam("id") UUID id, RepositoryMappings.Selection selection) { + if (selection == null || selection.repositoryId() == null) throw new BadRequestException("Select a repository"); + try { mappings.link(id, selection.revision(), selection.repositoryId()); return Response.noContent().build(); } + catch (AccountConflict conflict) { throw conflict(conflict); } + } + + @GET + public List list() { return registry.list(); } + + @GET @Path("/{id}") + public RepositoryView get(@PathParam("id") UUID id) { + return registry.get(id).orElseThrow(() -> new NotFoundException("Repository not registered")); + } + + @POST + public Response create(RepositoryInput input) { + try { return Response.status(201).entity(registry.create(input)).build(); } + catch (IllegalArgumentException invalid) { throw new BadRequestException(invalid.getMessage()); } + catch (AccountConflict conflict) { throw conflict(conflict); } + } + + @PUT @Path("/{id}") + public RepositoryView update(@PathParam("id") UUID id, @QueryParam("revision") long revision, RepositoryInput input) { + get(id); + try { return registry.update(id, revision, input); } + catch (IllegalArgumentException invalid) { throw new BadRequestException(invalid.getMessage()); } + catch (AccountConflict conflict) { throw conflict(conflict); } + } + + private ClientErrorException conflict(AccountConflict failure) { + return new ClientErrorException(Response.status(409).type(MediaType.TEXT_PLAIN).entity(failure.getMessage()).build()); + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumer.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumer.java new file mode 100644 index 00000000..b14c1c9d --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumer.java @@ -0,0 +1,21 @@ +package dev.codespire.orchestrator.repository; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.codespire.contract.event.RepositoryRegistration; +import io.smallrye.reactive.messaging.annotations.Blocking; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import org.eclipse.microprofile.reactive.messaging.Incoming; + +/** A failed reconciliation is nacked; it must not acknowledge a snapshot whose mapping was lost. */ +@ApplicationScoped +public class RepositorySnapshotConsumer { + @Inject ObjectMapper mapper; + @Inject RepositoryMigrationBridge bridge; + + @Incoming("registry-in") @Blocking + public void on(String payload) throws JsonProcessingException { + bridge.apply(mapper.readValue(payload, RepositoryRegistration.class)); + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryView.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryView.java new file mode 100644 index 00000000..424a7bb8 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/repository/RepositoryView.java @@ -0,0 +1,9 @@ +package dev.codespire.orchestrator.repository; + +import java.util.UUID; + +/** Selected accounts are visible even when disabled; this view never decrypts a credential. */ +public record RepositoryView(UUID id, String scmType, String forgeOrigin, String workspace, + String slug, boolean enabled, long revision, Account reviewer, Account factory) { + public record Account(UUID id, String name, String role, String handle, String state) { } +} diff --git a/spire-orchestrator/src/main/resources/application.yml b/spire-orchestrator/src/main/resources/application.yml index c8da20f5..1b5ffb5a 100644 --- a/spire-orchestrator/src/main/resources/application.yml +++ b/spire-orchestrator/src/main/resources/application.yml @@ -151,6 +151,19 @@ mp: serializer: dev.codespire.orchestrator.factory.RunCommandSerializer waitForWriteCompletion: true incoming: + registry-in: + connector: smallrye-kafka + topic: cs.registry-integration + group: + id: spire-orchestrator-registry + value: + deserializer: org.apache.kafka.common.serialization.StringDeserializer + auto: + offset: + reset: earliest + failure-strategy: dead-letter-queue + dead-letter-queue: + topic: cs.dlq integration-in: connector: smallrye-kafka topic: cs.integration @@ -354,6 +367,7 @@ spire: # Test: DevServices boot Postgres + a broker automatically. "%test": spire: + repository-history-interval: "off" review: # Tests exercise the active pipeline — seed active (prod/dev seed observe in code). mode: active diff --git a/spire-orchestrator/src/main/resources/db/migration/V60__repository_registration_bridge.sql b/spire-orchestrator/src/main/resources/db/migration/V60__repository_registration_bridge.sql new file mode 100644 index 00000000..898a0ec8 --- /dev/null +++ b/spire-orchestrator/src/main/resources/db/migration/V60__repository_registration_bridge.sql @@ -0,0 +1,41 @@ +-- Expand only. Workspace and its old constraints stay intact until the resolver cutover. +-- Account ids and provider: encryption AADs are never rewritten by this bridge. +CREATE TABLE repository ( + id UUID PRIMARY KEY, + scm_type TEXT NOT NULL, + forge_origin TEXT NOT NULL, + workspace TEXT NOT NULL CHECK (workspace <> ''), + slug TEXT NOT NULL CHECK (slug <> ''), + enabled BOOLEAN NOT NULL DEFAULT TRUE, + revision BIGINT NOT NULL DEFAULT 1, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + UNIQUE (scm_type, forge_origin, workspace, slug) +); +CREATE TABLE repository_account ( + repository_id UUID NOT NULL REFERENCES repository(id), + account_id UUID NOT NULL REFERENCES scm_provider(id), + role TEXT NOT NULL CHECK (role IN ('REVIEWER', 'FACTORY')), + PRIMARY KEY (repository_id, role) +); +CREATE INDEX repository_account_account ON repository_account(account_id); + +-- Immutable migration evidence, deliberately without an account FK: evidence survives re-assignment. +CREATE TABLE repository_legacy_account AS + SELECT id AS account_id, type, base_url, workspace, role FROM scm_provider + WHERE role IN ('REVIEWER', 'FACTORY'); +ALTER TABLE repository_legacy_account ADD PRIMARY KEY (account_id); + +CREATE TABLE repository_registration_bridge ( + registration_id UUID PRIMARY KEY, + revision BIGINT NOT NULL, + provider_type TEXT NOT NULL, + forge_origin TEXT, + scope TEXT NOT NULL, + target TEXT NOT NULL, + enabled BOOLEAN NOT NULL, + deleted BOOLEAN NOT NULL, + repository_id UUID REFERENCES repository(id), + problem TEXT +); +ALTER TABLE review_status ADD COLUMN repository_id UUID REFERENCES repository(id); +ALTER TABLE factory_run ADD COLUMN repository_id UUID REFERENCES repository(id); diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/dlq/DlqTopicsTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/dlq/DlqTopicsTest.java index 6bc7583c..f6e371d4 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/dlq/DlqTopicsTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/dlq/DlqTopicsTest.java @@ -7,6 +7,10 @@ /** Plain unit test (no Quarkus) — the type -> original-topic map must be deterministic. */ class DlqTopicsTest { + @Test void registrationReplaysOntoItsRegistryTopic() { + assertEquals("cs.registry-integration", DlqTopics.forType("RepositoryRegistration")); + } + @Test void aRunRecordReplaysOntoTheFactorysOwnTopics() { // Falling through to cs.commands republished a token-bearing record onto a topic whose diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunAgentStartedTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunAgentStartedTest.java index dcce6082..9c1e1c15 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunAgentStartedTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunAgentStartedTest.java @@ -44,13 +44,13 @@ void queuedRunsHaveNoAgentStart() { void theFirstStartRecordsAgentTimeWithoutMovingQueueTime() { String runId = queuedRun(); exec("UPDATE factory_run SET started_at = TIMESTAMPTZ '2026-01-01 00:00:00Z' WHERE run_id = ?", runId); - Instant before = Instant.now(); + Instant before = databaseNow(); projection.apply(new RunResult.RunStarted(runId, "TEST-unit")); Instant at = projection.find(runId).orElseThrow().agentStartedAt(); assertFalse(at.isBefore(before.minusMillis(1)), "the start is observed now, not copied from queue time"); - assertFalse(at.isAfter(Instant.now())); + assertFalse(at.isAfter(databaseNow())); assertEquals(at, listed(runId).agentStartedAt()); assertEquals(Instant.parse("2026-01-01T00:00:00Z"), listed(runId).startedAt()); } @@ -115,6 +115,13 @@ private String queuedRun() { return runId; } + private Instant databaseNow() { + // The timestamp is generated in Postgres. Host and container clocks can differ under load. + try (var c = dataSource.getConnection(); var st = c.createStatement(); var rows = st.executeQuery("SELECT clock_timestamp()")) { + assertTrue(rows.next()); return rows.getTimestamp(1).toInstant(); + } catch (SQLException failure) { throw new IllegalStateException(failure); } + } + private FactoryRunProjection.RunListEntry listed(String runId) { // One test pins queue time in the past; unrelated suites may have more than 200 newer rows. return projection.list(new FactoryRunProjection.RunFilter(null, null, null, Integer.MAX_VALUE)).stream() diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/pipeline/ReviewRetryScheduleIT.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/pipeline/ReviewRetryScheduleIT.java index 809a025c..a483d756 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/pipeline/ReviewRetryScheduleIT.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/pipeline/ReviewRetryScheduleIT.java @@ -25,6 +25,10 @@ @TestSecurity(user = "test-admin", roles = {"spire-viewer", "spire-admin"}) class ReviewRetryScheduleIT { + // Exercise the explicit clock parameter beyond the live scheduler's horizon. It must not steal + // a fixture between schedule and claim, which made these cases fail once every five seconds. + private final Instant testClock = Instant.now().plusSeconds(86400); + @Inject ReviewProjection projection; @@ -51,10 +55,10 @@ void aRetryIsInvisibleUntilItComesDue() { @Test void onlyOneClaimWinsSoAnAttemptCannotBeDispatchedTwice() { String reviewId = seedReview(9002L); - projection.scheduleRetry(reviewId, 2, "waiting", Instant.now().minusSeconds(1)); + projection.scheduleRetry(reviewId, 2, "waiting", testClock.minusSeconds(1)); - List first = projection.claimDueRetries(Instant.now()); - List second = projection.claimDueRetries(Instant.now()); + List first = projection.claimDueRetries(testClock); + List second = projection.claimDueRetries(testClock); assertTrue(first.contains(reviewId), "the first sweep claims it"); assertFalse(second.contains(reviewId), "the second finds nothing — the claim cleared the due time"); @@ -63,23 +67,23 @@ void onlyOneClaimWinsSoAnAttemptCannotBeDispatchedTwice() { @Test void aCancelledRetryIsNeverClaimed() { String reviewId = seedReview(9003L); - projection.scheduleRetry(reviewId, 2, "waiting", Instant.now().minusSeconds(1)); + projection.scheduleRetry(reviewId, 2, "waiting", testClock.minusSeconds(1)); projection.clearScheduledRetry(reviewId); - assertFalse(projection.claimDueRetries(Instant.now()).contains(reviewId), + assertFalse(projection.claimDueRetries(testClock).contains(reviewId), "a run that went terminal before its retry came due must not be resurrected"); } @Test void aFailedDispatchCanBePutBackOnTheClock() { String reviewId = seedReview(9004L); - projection.scheduleRetry(reviewId, 2, "waiting", Instant.now().minusSeconds(1)); - assertTrue(projection.claimDueRetries(Instant.now()).contains(reviewId)); + projection.scheduleRetry(reviewId, 2, "waiting", testClock.minusSeconds(1)); + assertTrue(projection.claimDueRetries(testClock).contains(reviewId)); // The claim already cleared the due time, so a dispatch failing afterwards would otherwise leave // the review waiting on a retry nobody sends. - projection.rescheduleRetry(reviewId, Instant.now().minusSeconds(1)); - assertTrue(projection.claimDueRetries(Instant.now()).contains(reviewId), "claimable again"); + projection.rescheduleRetry(reviewId, testClock.minusSeconds(1)); + assertTrue(projection.claimDueRetries(testClock).contains(reviewId), "claimable again"); } @Test diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/provider/RepositoryForgeOriginTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/provider/RepositoryForgeOriginTest.java new file mode 100644 index 00000000..3b8d3dd8 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/provider/RepositoryForgeOriginTest.java @@ -0,0 +1,14 @@ +package dev.codespire.orchestrator.provider; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +class RepositoryForgeOriginTest { + @Test void mapsPublicWebOriginsWithoutRewritingSelfHostedOrigins() { + assertEquals("https://api.github.com", ProviderClients.repositoryForgeOrigin("github", "https://github.com/TEST/repo/pull/1")); + assertEquals("https://api.bitbucket.org", ProviderClients.repositoryForgeOrigin("bitbucket-cloud", "https://bitbucket.org/TEST/repo/pull-requests/1")); + assertEquals("https://test-forge.example.test", ProviderClients.repositoryForgeOrigin("gitlab", "https://TEST-forge.example.test/TEST/nested/repo/-/merge_requests/1")); + assertEquals("https://test-forge.example.test", ProviderClients.repositoryForgeOrigin("github", "https://TEST-forge.example.test/TEST/repo/pull/1")); + assertEquals("https://github.com", ProviderClients.repositoryForgeOrigin("gitlab", "https://github.com/TEST/repo")); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryAccountsTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryAccountsTest.java new file mode 100644 index 00000000..5e5c1956 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryAccountsTest.java @@ -0,0 +1,117 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.orchestrator.provider.ProviderRole; +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import io.quarkus.test.junit.QuarkusTest; +import org.junit.jupiter.api.Test; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +class RepositoryAccountsTest extends RepositoryFixture { + @Test void reviewerNeverReceivesTheFactoryCredential() { + UUID factory = account("FACTORY"), reviewer = account("REVIEWER"); + var repository = repositories.create(repository(reviewer, factory)); + assertEquals("TEST-secret-REVIEWER", accounts.resolve(repository.id(), ProviderRole.REVIEWER).orElseThrow().secret()); + assertEquals("TEST-secret-FACTORY", accounts.resolve(repository.id(), ProviderRole.FACTORY).orElseThrow().secret()); + assertEquals(providers.resolve("gitlab", workspace, ProviderRole.REVIEWER), accounts.resolve(repository.id(), ProviderRole.REVIEWER)); + assertEquals(providers.resolve("gitlab", workspace, ProviderRole.FACTORY), accounts.resolve(repository.id(), ProviderRole.FACTORY)); + } + + @Test void rejectsAnAccountFromAnotherOrigin() throws Exception { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + execute("UPDATE scm_provider SET base_url='https://TEST-other.example.test' WHERE id=?", reviewer); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + } + + @Test void disabledAccountCannotServeAnExistingBinding() { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + providers.update(reviewer, input("REVIEWER", origin, false, "TEST-id-REVIEWER")); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + assertEquals("disabled", repositories.get(repo.id()).orElseThrow().reviewer().state()); + } + + @Test void disabledRepositoryCannotResolve() { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + repositories.update(repo.id(), repo.revision(), new RepositoryInput("gitlab", origin, workspace, "TEST-repo", false, reviewer, null)); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + } + + @Test void corruptBindingCannotUseAnotherRole() throws Exception { + UUID factory = account("FACTORY"); + var repo = repositories.create(repository(null, factory)); + execute("UPDATE repository_account SET role='REVIEWER' WHERE repository_id=?", repo.id()); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + } + + @Test void corruptBindingCannotUseAnotherKind() throws Exception { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + execute("UPDATE repository SET scm_type='github' WHERE id=?", repo.id()); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + } + + @Test void referencedAccountCannotBeRepurposed() { + UUID reviewer = account("REVIEWER"); + repositories.create(repository(reviewer, null)); + assertThrows(AccountConflict.class, () -> providers.update(reviewer, + input("REVIEWER", "https://TEST-other.example.test", true, "TEST-id-REVIEWER"))); + assertEquals(origin, providers.resolveById(reviewer).orElseThrow().baseUrl()); + } + + @Test void referencedDeleteNamesTheRepository() { + UUID reviewer = account("REVIEWER"); + repositories.create(repository(reviewer, null)); + var failure = assertThrows(AccountConflict.class, () -> providers.delete(reviewer)); + assertTrue(failure.getMessage().contains(origin + "/" + workspace + "/TEST-repo")); + assertTrue(providers.resolveById(reviewer).isPresent()); + } + + @Test void rejectsCrossOriginBindingWithoutLeavingRepository() { + UUID reviewer = account("REVIEWER", "https://TEST-other.example.test"); + assertThrows(AccountConflict.class, () -> repositories.create(repository(reviewer, null))); + assertTrue(repositories.list().stream().noneMatch(repo -> repo.workspace().equals(workspace))); + } + + @Test void rejectsWrongRoleBinding() { + UUID factory = account("FACTORY"); + assertThrows(AccountConflict.class, () -> repositories.create(repository(factory, null))); + } + + @Test void missingAccountIsAnActionableConflict() { + assertThrows(AccountConflict.class, () -> repositories.create(repository(UUID.randomUUID(), null))); + } + + @Test void rejectsWrongKindBinding() throws Exception { + UUID reviewer = account("REVIEWER"); + execute("UPDATE scm_provider SET type='github' WHERE id=?", reviewer); + assertThrows(AccountConflict.class, () -> repositories.create(repository(reviewer, null))); + } + + @Test void rejectsSameResolvedIdentity() { + UUID reviewer = account("REVIEWER"), factory = account("FACTORY"); + providers.update(factory, input("FACTORY", origin, true, "TEST-id-REVIEWER")); + assertThrows(AccountConflict.class, () -> repositories.create(repository(reviewer, factory))); + } + + @Test void rotationReachesBindingWithoutReassignment() { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + var rotated = input("REVIEWER", origin, true, "TEST-id-REVIEWER"); + providers.update(reviewer, new dev.codespire.orchestrator.provider.ProviderInput(rotated.name(), rotated.type(), origin, + workspace, "bearer", null, "TEST-rotated", rotated.botAccountId(), true, rotated.authors(), rotated.botUsername(), null, "REVIEWER")); + assertEquals("TEST-rotated", accounts.resolve(repo.id(), ProviderRole.REVIEWER).orElseThrow().secret()); + } + + @Test void identityCollisionAfterRotationCannotServeEitherRole() { + UUID reviewer = account("REVIEWER"), factory = account("FACTORY"); + var repo = repositories.create(repository(reviewer, factory)); + providers.update(factory, input("FACTORY", origin, true, "TEST-id-REVIEWER")); + assertTrue(accounts.resolve(repo.id(), ProviderRole.REVIEWER).isEmpty()); + assertTrue(accounts.resolve(repo.id(), ProviderRole.FACTORY).isEmpty()); + assertEquals("identity-conflict", repositories.get(repo.id()).orElseThrow().factory().state()); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryCoordinatesTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryCoordinatesTest.java new file mode 100644 index 00000000..e08a01ac --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryCoordinatesTest.java @@ -0,0 +1,22 @@ +package dev.codespire.orchestrator.repository; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +class RepositoryCoordinatesTest { + RepositoryRegistry registry = new RepositoryRegistry(); + RepositoryInput input(String workspace, String slug) { + return new RepositoryInput("gitlab", "https://TEST-forge.example.test", workspace, slug, true, null, null); + } + @Test void refusesTraversalAndEmptySegments() { + for (String path : new String[]{"TEST/../other", "TEST//other", "TEST/./other", "TEST\\other", "TEST/\u0001other"}) { + assertThrows(IllegalArgumentException.class, () -> registry.normalize(input(path, "TEST-repo"))); + } + } + @Test void refusesNamespaceInsideSlug() { + assertThrows(IllegalArgumentException.class, () -> registry.normalize(input("TEST/group", "TEST/repo"))); + } + @Test void requiresCoordinates() { + assertThrows(IllegalArgumentException.class, () -> registry.normalize(input("", "TEST-repo"))); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryFixture.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryFixture.java new file mode 100644 index 00000000..6cd4b208 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryFixture.java @@ -0,0 +1,64 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.orchestrator.provider.*; +import jakarta.inject.Inject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import javax.sql.DataSource; +import java.util.List; +import java.util.UUID; + +/** TEST-only fixtures in Quarkus Dev Services; no live database or forge calls. */ +abstract class RepositoryFixture { + @Inject DataSource dataSource; + @Inject ProviderRegistry providers; + @Inject RepositoryRegistry repositories; + @Inject RepositoryAccounts accounts; + @Inject RepositoryMigrationBridge bridge; + @Inject RepositoryMappings mappings; + String workspace; + final java.util.Set createdAccounts = new java.util.HashSet<>(); + final java.util.Set createdRegistrations = new java.util.HashSet<>(); + final String origin = "https://TEST-forge.example.test".toLowerCase(); + + @BeforeEach void nameFixture() { workspace = "TEST-" + UUID.randomUUID() + "/nested"; } + + ProviderInput input(String role, String host, boolean enabled, String identity) { + return new ProviderInput("TEST-" + role, "gitlab", host, workspace, "bearer", null, + "TEST-secret-" + role, identity, enabled, List.of(), "TEST-login-" + role, null, role); + } + + UUID account(String role) { return account(role, origin); } + UUID account(String role, String host) { + UUID id = UUID.fromString(providers.create(input(role, host, true, "TEST-id-" + role)).id()); + createdAccounts.add(id); + return id; + } + + RepositoryInput repository(UUID reviewer, UUID factory) { + return new RepositoryInput("gitlab", origin, workspace, "TEST-repo", true, reviewer, factory); + } + + void snapshotAccounts() throws Exception { + execute("INSERT INTO repository_legacy_account SELECT id,type,base_url,workspace,role FROM scm_provider WHERE workspace=? ON CONFLICT DO NOTHING", workspace); + } + + void execute(String sql, Object... parameters) throws Exception { + try (var c = dataSource.getConnection(); var ps = c.prepareStatement(sql)) { + for (int i = 0; i < parameters.length; i++) ps.setObject(i + 1, parameters[i]); + ps.executeUpdate(); + } + } + + @AfterEach void removeFixture() throws Exception { + execute("DELETE FROM review_status WHERE workspace=?", workspace); + execute("DELETE FROM factory_run WHERE workspace=?", workspace); + execute("DELETE FROM repository_registration_bridge WHERE target=?", workspace + "/TEST-repo"); + for (UUID id : createdRegistrations) execute("DELETE FROM repository_registration_bridge WHERE registration_id=?", id); + execute("DELETE FROM repository_account WHERE repository_id IN (SELECT id FROM repository WHERE workspace=?)", workspace); + execute("DELETE FROM repository WHERE workspace=?", workspace); + execute("DELETE FROM repository_legacy_account WHERE workspace=?", workspace); + execute("DELETE FROM scm_provider WHERE workspace=?", workspace); + for (UUID id : createdAccounts) execute("DELETE FROM scm_provider WHERE id=?", id); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridgeTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridgeTest.java new file mode 100644 index 00000000..8643304a --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryMigrationBridgeTest.java @@ -0,0 +1,160 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.contract.event.RepositoryRegistration; +import dev.codespire.orchestrator.provider.ProviderRole; +import dev.codespire.orchestrator.provider.ProviderRegistry.AccountConflict; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.junit.jupiter.api.Test; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +class RepositoryMigrationBridgeTest extends RepositoryFixture { + @Inject RepositoryHistoryBridge histories; + @Inject dev.codespire.orchestrator.attention.AttentionQueries attention; + RepositoryRegistration registration(UUID id, long revision) { + return new RepositoryRegistration(id, revision, "gitlab", origin, "repo", workspace + "/TEST-repo", true, false); + } + RepositoryView migrated() { + return repositories.list().stream().filter(repo -> repo.workspace().equals(workspace)).findFirst().orElseThrow(); + } + + @Test void replaysGatewaySnapshotWithoutDuplicateBindings() throws Exception { + UUID reviewer = account("REVIEWER"), factory = account("FACTORY"); + snapshotAccounts(); + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(snapshot); + var repo = migrated(); + assertEquals(reviewer, repo.reviewer().id()); assertEquals(factory, repo.factory().id()); + assertEquals(providers.resolve("gitlab", workspace, ProviderRole.REVIEWER), accounts.resolve(repo.id(), ProviderRole.REVIEWER)); + assertEquals(providers.resolve("gitlab", workspace, ProviderRole.FACTORY), accounts.resolve(repo.id(), ProviderRole.FACTORY)); + // State is in SQL: a fresh CDI-free instance simulates recreation after restart. + var restarted = new RepositoryMigrationBridge(); + restarted.dataSource = dataSource; restarted.repositories = repositories; restarted.bindings = new RepositoryBindings(); + restarted.apply(snapshot); + String originalWorkspace = workspace; + UUID rebound; + try { workspace += "/TEST-replacement"; rebound = account("REVIEWER"); } + finally { workspace = originalWorkspace; } + repositories.update(repo.id(), repo.revision(), repository(rebound, factory)); + bridge.apply(registration(snapshot.registrationId(), 2)); + bridge.apply(registration(UUID.randomUUID(), 1)); + assertEquals(rebound, migrated().reviewer().id(), "gateway replay must not restore the operator-replaced binding"); + assertEquals(1, repositories.list().stream().filter(row -> row.workspace().equals(workspace)).count()); + } + + @Test void leavesConflictingOriginsPending() throws Exception { + account("REVIEWER"); account("FACTORY", "https://TEST-other.example.test"); snapshotAccounts(); + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(snapshot); + var pending = mappings.pending().stream().filter(row -> row.registrationId().equals(snapshot.registrationId())).findFirst().orElseThrow(); + assertEquals("conflicting_forge_origins", pending.problem()); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + var row = attention.collect().stream().filter(value -> snapshot.registrationId().toString().equals(value.subject())).findFirst(); + assertTrue(row.isPresent(), "the pending mapping must reach the operator attention panel"); + assertTrue(row.orElseThrow().message().contains(snapshot.target())); + assertTrue(row.orElseThrow().message().contains(origin)); + assertTrue(row.orElseThrow().action().contains(snapshot.registrationId().toString())); + } + + @Test void missingLegacyAccountStaysVisibleAndCanBeLinked() { + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(snapshot); + var repo = repositories.create(repository(null, null)); + mappings.link(snapshot.registrationId(), 1, repo.id()); + bridge.apply(registration(snapshot.registrationId(), 2)); + assertTrue(mappings.pending().stream().noneMatch(row -> row.registrationId().equals(snapshot.registrationId()))); + assertEquals(repo.id(), migrated().id()); + } + + @Test void staleSnapshotCannotResurrectDeletedRegistration() { + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(new RepositoryRegistration(snapshot.registrationId(), 2, "gitlab", null, "repo", snapshot.target(), false, true)); + bridge.apply(snapshot); + assertTrue(mappings.pending().stream().noneMatch(row -> row.registrationId().equals(snapshot.registrationId()))); + } + + @Test void mappingRefusesAnotherRepositoryPath() { + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(snapshot); + var wrong = repositories.create(new RepositoryInput("gitlab", origin, workspace, "TEST-wrong", true, null, null)); + assertThrows(AccountConflict.class, () -> mappings.link(snapshot.registrationId(), 1, wrong.id())); + } + + @Test void mappingRefusesStaleRegistrationRevision() { + var snapshot = registration(UUID.randomUUID(), 2); + bridge.apply(snapshot); + var repo = repositories.create(repository(null, null)); + assertThrows(AccountConflict.class, () -> mappings.link(snapshot.registrationId(), 1, repo.id())); + } + + @Test void sameBotIdentityBecomesRepairableInsteadOfPoisoningTheConsumer() throws Exception { + account("REVIEWER"); UUID factory = account("FACTORY"); + providers.update(factory, input("FACTORY", origin, true, "TEST-id-REVIEWER")); snapshotAccounts(); + var snapshot = registration(UUID.randomUUID(), 1); + bridge.apply(snapshot); + assertEquals("legacy_binding_invalid", mappings.pending().stream().filter(row -> row.registrationId().equals(snapshot.registrationId())) + .findFirst().orElseThrow().problem()); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + } + + @Test void orgHistoryCreatesOnlyTheRepositoryActuallyObserved() throws Exception { + account("REVIEWER"); account("FACTORY"); snapshotAccounts(); + UUID orgRegistration = UUID.randomUUID(); createdRegistrations.add(orgRegistration); + bridge.apply(new RepositoryRegistration(orgRegistration, 1, "gitlab", origin, "org", workspace.split("/")[0], true, false)); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + var history = new RepositoryHistoryBridge.History("gitlab", workspace, "TEST-repo"); + UUID review = UUID.randomUUID(); + execute("INSERT INTO review_status (review_id,workspace,slug,pr_id,status,provider_type,html_url) VALUES (?,?,?,1,'completed','gitlab',?)", + review, workspace, "TEST-repo", origin + "/" + workspace + "/TEST-repo/-/merge_requests/1"); + String run = "run::gitlab:" + workspace + "/TEST-repo:TEST-history:1"; + execute(""" + INSERT INTO factory_run (run_id,provider_type,workspace,slug,subject,attempt,status,harness,model, + base_branch,base_commit,branch,ended_at) VALUES (?,'gitlab',?,?,'TEST-history',1,'succeeded', + 'TEST-harness','TEST-model','TEST-main','TEST-sha','TEST-branch',now()) + """, run, workspace, "TEST-repo"); + histories.importHistory(history); + try (var c = dataSource.getConnection(); var ps = c.prepareStatement("SELECT repository_id FROM review_status WHERE review_id=?")) { + ps.setString(1, review.toString()); + try (var rows = ps.executeQuery()) { assertTrue(rows.next()); assertEquals(migrated().id(), rows.getObject(1, UUID.class)); } + } + try (var c = dataSource.getConnection(); var ps = c.prepareStatement("SELECT repository_id FROM factory_run WHERE run_id=?")) { + ps.setString(1, run); + try (var rows = ps.executeQuery()) { assertTrue(rows.next()); assertEquals(migrated().id(), rows.getObject(1, UUID.class)); } + } + histories.importHistory(history); + assertEquals(1, repositories.list().stream().filter(row -> row.workspace().equals(workspace)).count()); + } + + @Test void unknownRegistrationOriginCannotInheritWorkspaceCredentials() throws Exception { + account("REVIEWER"); snapshotAccounts(); + UUID id = UUID.randomUUID(); + bridge.apply(new RepositoryRegistration(id, 1, "gitlab", null, "repo", workspace + "/TEST-repo", true, false)); + var pending = mappings.pending().stream().filter(row -> row.registrationId().equals(id)).findFirst(); + assertTrue(pending.isPresent(), "unknown origin must remain explicitly pending"); + assertEquals("registration_origin_unknown", pending.orElseThrow().problem()); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + } + + @Test void registrationFromAnotherHostCannotInheritWorkspaceCredentials() throws Exception { + account("REVIEWER"); snapshotAccounts(); + UUID id = UUID.randomUUID(); + bridge.apply(new RepositoryRegistration(id, 1, "gitlab", "https://TEST-other.example.test", "repo", workspace + "/TEST-repo", true, false)); + var pending = mappings.pending().stream().filter(row -> row.registrationId().equals(id)).findFirst(); + assertTrue(pending.isPresent(), "mismatched origin must remain explicitly pending"); + assertEquals("registration_origin_mismatch", pending.orElseThrow().problem()); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + } + + @Test void historyFromAnotherHostCannotInheritWorkspaceCredentials() throws Exception { + account("REVIEWER"); snapshotAccounts(); + execute("INSERT INTO review_status (review_id,workspace,slug,pr_id,status,provider_type,html_url) VALUES (?,?,?,1,'completed','gitlab',?)", + UUID.randomUUID().toString(), workspace, "TEST-repo", "https://TEST-other.example.test/" + workspace + "/TEST-repo/-/merge_requests/1"); + histories.importHistory(new RepositoryHistoryBridge.History("gitlab", workspace, "TEST-repo")); + var pending = mappings.pending().stream().filter(row -> row.target().equals(workspace + "/TEST-repo")).findFirst(); + assertTrue(pending.isPresent()); + assertEquals("registration_origin_mismatch", pending.orElseThrow().problem()); + assertTrue(repositories.list().stream().noneMatch(row -> row.workspace().equals(workspace))); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryResourceTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryResourceTest.java new file mode 100644 index 00000000..59cc29d9 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositoryResourceTest.java @@ -0,0 +1,46 @@ +package dev.codespire.orchestrator.repository; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.security.TestSecurity; +import org.junit.jupiter.api.Test; +import java.util.UUID; +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +@TestSecurity(user = "TEST-admin", roles = {"spire-viewer", "spire-admin"}) +class RepositoryResourceTest extends RepositoryFixture { + @Test void registersARepositoryWithExplicitRoleBindings() { + UUID reviewer = account("REVIEWER"), factory = account("FACTORY"); + String id = given().contentType("application/json").body(repository(reviewer, factory)).post("/api/repositories") + .then().statusCode(201).body("workspace", equalTo(workspace)) + .body("reviewer.id", equalTo(reviewer.toString())).body("factory.id", equalTo(factory.toString())) + .body(not(containsString("TEST-secret"))).extract().path("id"); + given().get("/api/repositories/" + id).then().statusCode(200) + .body("forgeOrigin", equalTo(origin)).body("factory.handle", equalTo("TEST-login-FACTORY")); + } + + @Test void staleUpdateCannotOverwriteBindings() { + UUID reviewer = account("REVIEWER"); + var repo = repositories.create(repository(reviewer, null)); + given().contentType("application/json").body(repository(null, null)).put("/api/repositories/" + repo.id() + "?revision=1").then().statusCode(200); + given().contentType("application/json").body(repository(reviewer, null)).put("/api/repositories/" + repo.id() + "?revision=1") + .then().statusCode(409).body(containsString("reload")); + assertNull(repositories.get(repo.id()).orElseThrow().reviewer()); + } + + @Test void duplicateCoordinatesCannotCreateSecondRepository() { + repositories.create(repository(null, null)); + given().contentType("application/json").body(repository(null, null)).post("/api/repositories").then().statusCode(409); + } + + @Test void missingKindIsBadRequest() { + given().contentType("application/json").body("{}").post("/api/repositories").then().statusCode(400); + } + + @Test @TestSecurity(user = "TEST-viewer", roles = "spire-viewer") + void viewerCannotRegisterRepository() { + given().contentType("application/json").body(repository(null, null)).post("/api/repositories").then().statusCode(403); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySchemaMigrationTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySchemaMigrationTest.java new file mode 100644 index 00000000..0bdab86f --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySchemaMigrationTest.java @@ -0,0 +1,52 @@ +package dev.codespire.orchestrator.repository; + +import dev.codespire.encryption.EncryptionService; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.flywaydb.core.Flyway; +import org.junit.jupiter.api.Test; +import javax.sql.DataSource; +import java.util.UUID; +import static org.junit.jupiter.api.Assertions.*; + +/** Actual populated V59 -> V60 migration in a private Dev Services schema. */ +@QuarkusTest +class RepositorySchemaMigrationTest { + @Inject DataSource dataSource; + @Inject EncryptionService encryption; + + @Test void preservesAccountIdsCredentialsAndContextReferences() throws Exception { + String schema = "test_repository_upgrade_" + UUID.randomUUID().toString().replace("-", ""); + Flyway.configure().dataSource(dataSource).schemas(schema).defaultSchema(schema).target("59").load().migrate(); + UUID reviewer = UUID.randomUUID(), factory = UUID.randomUUID(), source = UUID.randomUUID(); + String reviewerCipher = encryption.encryptString("TEST-review-token", "provider:" + reviewer); + String factoryCipher = encryption.encryptString("TEST-factory-token", "provider:" + factory); + try (var c = dataSource.getConnection()) { + String previous = c.getSchema(); + try { + c.setSchema(schema); + try (var ps = c.prepareStatement("INSERT INTO scm_provider (id,name,type,base_url,workspace,auth_kind,auth_secret,role) VALUES (?,?,'gitlab','https://TEST-forge.example.test','TEST-group/nested','bearer',?,?)")) { + ps.setObject(1, reviewer); ps.setString(2, "TEST-reviewer"); ps.setString(3, reviewerCipher); ps.setString(4, "REVIEWER"); ps.executeUpdate(); + ps.setObject(1, factory); ps.setString(2, "TEST-factory"); ps.setString(3, factoryCipher); ps.setString(4, "FACTORY"); ps.executeUpdate(); + } + try (var ps = c.prepareStatement("INSERT INTO context_provider (id,name,type,base_url,account_id) VALUES (?,'TEST-context','gitlab-issues','https://TEST-forge.example.test',?)")) { + ps.setObject(1, source); ps.setObject(2, reviewer); ps.executeUpdate(); + } + Flyway.configure().dataSource(dataSource).schemas(schema).defaultSchema(schema).target("60").load().migrate(); + try (var st = c.createStatement(); var rs = st.executeQuery("SELECT p.id,p.auth_secret,l.account_id,l.workspace,p.role FROM scm_provider p JOIN repository_legacy_account l ON l.account_id=p.id ORDER BY p.role")) { + assertTrue(rs.next()); assertEquals(factory, rs.getObject(1, UUID.class)); assertEquals(factoryCipher, rs.getString(2)); + assertEquals("TEST-factory-token", encryption.decryptString(rs.getString(2), "provider:" + rs.getObject(1))); + assertTrue(rs.next()); assertEquals(reviewer, rs.getObject(1, UUID.class)); assertEquals(reviewerCipher, rs.getString(2)); + assertEquals("TEST-review-token", encryption.decryptString(rs.getString(2), "provider:" + rs.getObject(1))); + assertEquals("TEST-group/nested", rs.getString(4)); assertFalse(rs.next()); + } + try (var st = c.createStatement(); var rs = st.executeQuery("SELECT id,account_id,auth_secret FROM context_provider")) { + assertTrue(rs.next()); assertEquals(source, rs.getObject(1, UUID.class)); assertEquals(reviewer, rs.getObject(2, UUID.class)); assertNull(rs.getString(3)); + } + } finally { + c.setSchema(previous); + try (var st = c.createStatement()) { st.execute("DROP SCHEMA " + schema + " CASCADE"); } + } + } + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumerTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumerTest.java new file mode 100644 index 00000000..2cf64e36 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/repository/RepositorySnapshotConsumerTest.java @@ -0,0 +1,37 @@ +package dev.codespire.orchestrator.repository; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.codespire.contract.event.RepositoryRegistration; +import io.quarkus.test.common.QuarkusTestResource; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.kafka.InjectKafkaCompanion; +import io.quarkus.test.kafka.KafkaCompanionResource; +import io.smallrye.reactive.messaging.kafka.companion.KafkaCompanion; +import jakarta.inject.Inject; +import org.apache.kafka.clients.producer.ProducerRecord; +import org.junit.jupiter.api.Test; +import java.time.Duration; +import java.util.UUID; +import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +@QuarkusTestResource(KafkaCompanionResource.class) +class RepositorySnapshotConsumerTest extends RepositoryFixture { + @InjectKafkaCompanion KafkaCompanion companion; + @Inject ObjectMapper mapper; + + @Test void brokerDeliveryReconcilesTheActualConsumerAndRegistry() throws Exception { + UUID reviewer = account("REVIEWER"); snapshotAccounts(); + UUID registration = UUID.randomUUID(); + var snapshot = new RepositoryRegistration(registration, 1, "gitlab", origin, "repo", workspace + "/TEST-repo", true, false); + companion.produceStrings().fromRecords(new ProducerRecord<>("cs.registry-integration", registration.toString(), mapper.writeValueAsString(snapshot))) + .awaitCompletion(Duration.ofSeconds(15)); + await().atMost(Duration.ofSeconds(15)).untilAsserted(() -> { + var match = repositories.list().stream().filter(row -> row.workspace().equals(workspace)).findFirst(); + assertTrue(match.isPresent(), "the actual consumer must create the registry row"); + var repo = match.orElseThrow(); + assertEquals(reviewer, repo.reviewer().id()); assertNull(repo.factory()); + }); + } +} diff --git a/spire-ui/src/App.tsx b/spire-ui/src/App.tsx index e32105dc..076085cc 100644 --- a/spire-ui/src/App.tsx +++ b/spire-ui/src/App.tsx @@ -13,6 +13,7 @@ import SettingsProviders from './components/SettingsProviders'; import SettingsLlmProviders from './components/SettingsLlmProviders'; import SettingsContextProviders from './components/SettingsContextProviders'; import SettingsWebhookRepos from './components/SettingsWebhookRepos'; +import RepositoryRegistryPage from './components/repositories/RepositoryRegistryPage'; import SettingsDlq from './components/SettingsDlq'; import PromptsSettings from './components/PromptsSettings'; import PromptDetail from './components/PromptDetail'; @@ -378,6 +379,7 @@ export default function App() { )} /> )} /> )} /> + )} /> {/* The three screens moved on 2026-09-07. Old addresses live in bookmarks and in attention rows emitted by a service not yet upgraded; the query rides along because ?edit= is what opens the named record. */} diff --git a/spire-ui/src/api.ts b/spire-ui/src/api.ts index 03d34da7..e18021f2 100644 --- a/spire-ui/src/api.ts +++ b/spire-ui/src/api.ts @@ -477,6 +477,7 @@ export type WebhookScope = 'repo' | 'org'; export interface WebhookRepoView { id: string; + forgeOrigin?: string | null; // absent on older gateways; explicit registration evidence when known providerType: string; // 'github' | 'gitlab' | 'bitbucket-cloud' scope: WebhookScope; // 'repo' (target = owner/repo) | 'org' (target = owner) target: string; // owner/repo (repo scope) | owner (org scope) @@ -487,6 +488,7 @@ export interface WebhookRepoView { } export interface WebhookRepoInput { + forgeOrigin?: string | null; // omitted legacy edits preserve the stored origin providerType: string; // 'github' | 'gitlab' | 'bitbucket-cloud' scope: WebhookScope; target: string; // owner/repo (repo scope) | owner (org scope) diff --git a/spire-ui/src/components/ProviderFormModal.tsx b/spire-ui/src/components/ProviderFormModal.tsx index 590bd78f..a36d0aae 100644 --- a/spire-ui/src/components/ProviderFormModal.tsx +++ b/spire-ui/src/components/ProviderFormModal.tsx @@ -215,9 +215,11 @@ export default function ProviderFormModal({ {role !== 'CONTEXT' &&