diff --git a/.specify/functional/target-integration.md b/.specify/functional/target-integration.md new file mode 100644 index 00000000..c82d3a54 --- /dev/null +++ b/.specify/functional/target-integration.md @@ -0,0 +1,85 @@ +--- +id: FUNC-AC-TARGET-INTEGRATION +type: functional +domain: runforge +status: draft +version: 1 +layer: 1 +--- + +# FUNC-AC-TARGET-INTEGRATION — Target Repository Integration Contract + +## Problem Statement + +The platform can automate only a repository whose working conventions happen to match its own: the exact label names that mark work as ready, the fixed wording of proposal titles, one governance layout for governing specifications, and one built-in patience window for a repository's own quality checks. A maintainer whose repository names, workflows, or review conventions differ cannot adopt the platform without forking it, and cannot find out in advance whether adoption would be safe — the first sign of incompatibility is a wrong or destructive action in their repository. + +The integration boundary between the platform and a target repository must therefore express semantic roles ("this label means work is ready") rather than one repository's naming conventions, must be declared explicitly rather than guessed, and must stop autonomous work when the declaration is missing, ambiguous, or rejected by the repository host — never improvising a convention and never reporting an action as successful when the host did not confirm it. + +## Actors + +- **Maintainer** — owns a target repository and decides whether and how the platform may work in it +- **Operator** — configures the platform for a target repository and answers escalations when the integration contract blocks work + +## Behavior + +**Scenario: Conventions are declared, not assumed** +- Given a target repository whose workflow label names differ from the platform's native ones +- When the Operator declares the repository's conventions as an integration profile that maps each semantic role to the repository's own names +- Then work discovery, state transitions, and status reporting follow the declared names, without any change to the platform itself + +**Scenario: Native conventions remain a built-in profile** +- Given a repository that already follows the platform's native conventions +- When the Operator selects the built-in profile +- Then the platform behaves exactly as it does today, and existing installations keep working through a documented migration path + +**Scenario: Compatibility preflight before any work** +- Given a configured target repository +- When the Operator runs the compatibility check +- Then they receive a deterministic verdict — compatible, configuration missing, or unsafe — with each missing or conflicting item named, and the check changes nothing in the repository + +**Scenario: Incomplete contract stops autonomous work** +- Given an integration declaration that is invalid, ambiguous, or incomplete +- When the platform would otherwise claim available work in that repository +- Then no work is claimed and the blocking reason is visible to the Operator + +**Scenario: Proposal wording follows the profile** +- Given a repository whose contribution rules require particular proposal titles or descriptions +- When the platform submits a change proposal +- Then the proposal's title and description follow the wording declared in the integration profile, with the platform's current wording used by the built-in profile + +**Scenario: Repository check patience is declared per repository** +- Given a repository whose own quality checks regularly take longer than the platform's default patience window +- When the Operator declares a longer waiting policy for that repository +- Then the platform waits accordingly before judging the checks, without any change to the platform itself + +**Scenario: Governance layout is a selectable profile** +- Given a repository that organizes its governing documents differently from the platform's native methodology +- When the Operator selects a governance profile for that repository +- Then the platform reads governing documents and enforces protected areas according to that profile, with the native methodology remaining available as a built-in profile + +**Scenario: Repository instructions may only tighten protection** +- Given a target repository that ships its own working instructions +- When the platform works in that repository +- Then those instructions may narrow what the platform is allowed to touch but can never widen it beyond the declared profile, unless the Operator has explicitly marked the repository as trusted for that purpose + +**Scenario: Unconfirmed actions are never reported as success** +- Given the repository host rejects or does not confirm a requested transition +- When the platform records the outcome +- Then the work item is parked with the host's stated reason rather than recorded as completed + +## Success Criteria + +- A repository whose label names differ from the platform's native conventions can be onboarded by declaration alone — demonstrated without forking or patching the platform +- The compatibility check gives the same verdict for the same repository state on every run, names every missing item, and performs no action a repository audit trail would record as a change +- With an incomplete or invalid integration declaration, the number of autonomous actions taken in the target repository is zero +- An existing installation continues to operate unchanged after upgrading, following the documented migration path +- A repository whose checks conclude after the platform's default patience window can still be integrated by declaration alone + +## Constraints + +- The integration declaration is explicit and versioned; unrecognized or contradictory declarations are rejected as a whole rather than partially honored +- Declared wording for proposals is fixed-text substitution of named values only; a declaration can never cause the platform to execute instructions of any kind +- The compatibility check is strictly read-only toward the target repository +- Records produced during configuration and preflight never reveal credentials or other secrets +- Repository-supplied instructions can only narrow the platform's permitted working area; widening requires the Operator's explicit trust decision +- Protections that isolate held-back verification material and the platform's methodology remain in force under every profile diff --git a/.specify/traceability.yml b/.specify/traceability.yml index 347e2256..29f771d7 100644 --- a/.specify/traceability.yml +++ b/.specify/traceability.yml @@ -1755,3 +1755,9 @@ STACK-AC-RELEASE: - packages/release-ledger/test/ledger.test.ts - packages/release-ledger/test/marker.test.ts status: draft + +# Target-repository integration contract (GitHub issue #2). L1 drafted from the +# Operator-authored issue; status: draft pending Operator approval of the L1 +# content (the Operator's gate per L0). L2/L3 follow after approval. +FUNC-AC-TARGET-INTEGRATION: + status: draft diff --git a/packages/daemon/src/control-plane/deployment-registry/schema.test.ts b/packages/daemon/src/control-plane/deployment-registry/schema.test.ts index bf2c9b0f..f01ed858 100644 --- a/packages/daemon/src/control-plane/deployment-registry/schema.test.ts +++ b/packages/daemon/src/control-plane/deployment-registry/schema.test.ts @@ -85,6 +85,29 @@ describe('parseProfile — fail-closed rejections (real zod / composed parsers)' if (!r.ok) expect(r.offenders.join()).toContain('deploymentName'); }); + it('landing accepts an explicit required-check wait policy (checkBudgetMs / checkPollMs)', () => { + const withWaitPolicy = structuredClone(validProfile) as Record; + withWaitPolicy.landing = { + ...validProfile.landing, + requiredChecks: ['daemon / test'], + checkBudgetMs: 900_000, + checkPollMs: 15_000, + }; + const r = parseProfile('dep-a', withWaitPolicy); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.profile.landing.checkBudgetMs).toBe(900_000); + expect(r.profile.landing.checkPollMs).toBe(15_000); + } + }); + + it('a non-positive checkBudgetMs is rejected', () => { + const bad = structuredClone(validProfile) as Record; + bad.landing = { ...validProfile.landing, checkBudgetMs: 0 }; + const r = parseProfile('dep-a', bad); + expect(r.ok).toBe(false); + }); + it('a malformed lane set (duplicate lane name) rejects the whole profile', () => { const bad = structuredClone(validProfile); bad.laneSet.lanes[0]!.name = 'standard'; // duplicates lanes[1] diff --git a/packages/daemon/src/control-plane/deployment-registry/schema.ts b/packages/daemon/src/control-plane/deployment-registry/schema.ts index 7d30861a..b58b0512 100644 --- a/packages/daemon/src/control-plane/deployment-registry/schema.ts +++ b/packages/daemon/src/control-plane/deployment-registry/schema.ts @@ -107,6 +107,12 @@ const LandingTargetSchema = z * merge path. Absent or empty for a governed deployment fails closed. */ requiredChecks: z.array(z.string().min(1)).optional(), + /** + * OPTIONAL required-check wait policy: total budget and poll interval in + * milliseconds. Absent falls back to the await-checks defaults. + */ + checkBudgetMs: z.number().int().positive().optional(), + checkPollMs: z.number().int().positive().optional(), }) .strict(); diff --git a/packages/daemon/src/control-plane/deployment-registry/types.ts b/packages/daemon/src/control-plane/deployment-registry/types.ts index 59369323..d5734bfd 100644 --- a/packages/daemon/src/control-plane/deployment-registry/types.ts +++ b/packages/daemon/src/control-plane/deployment-registry/types.ts @@ -93,6 +93,12 @@ export interface LandingTarget { * treated as fail-closed (escalate) — never as an implicit green. */ requiredChecks?: string[]; + /** + * OPTIONAL required-check wait policy in milliseconds. Absent falls back to + * the await-checks defaults (60s budget / 5s poll). + */ + checkBudgetMs?: number; + checkPollMs?: number; } /** For each shared capability, the identified version this deployment is bound to. */ diff --git a/packages/daemon/src/control-plane/p1-pr-delivery.gate.test.ts b/packages/daemon/src/control-plane/p1-pr-delivery.gate.test.ts index 4e60204d..0762cf02 100644 --- a/packages/daemon/src/control-plane/p1-pr-delivery.gate.test.ts +++ b/packages/daemon/src/control-plane/p1-pr-delivery.gate.test.ts @@ -76,6 +76,8 @@ interface DeliveryArgs { awaitRequiredChecks: (args: AwaitRequiredChecksArgs) => Promise; pushFeatureBranch: (args: PushFeatureBranchArgs) => Promise; trigger: { kind: 'auto-merge' | 'operator-approved-epoch'; detail: string }; + checkBudgetMs?: number; + checkPollMs?: number; } type DeliverCodeChangeViaPR = (args: DeliveryArgs) => Promise; @@ -102,7 +104,14 @@ async function loadDeliverCodeChangeViaPR(): Promise { return deliver!; } -function makeOctokit(options: { existingPulls?: PullRequestSummary[]; prNumber?: number; mergeSha?: string } = {}) { +function makeOctokit( + options: { + existingPulls?: PullRequestSummary[]; + prNumber?: number; + mergeSha?: string; + mergeResponse?: { merged: boolean; sha: string; message?: string }; + } = {}, +) { const existingPulls = options.existingPulls ?? []; const prNumber = options.prNumber ?? 101; const mergeSha = options.mergeSha ?? 'squash-merge-sha-101'; @@ -120,9 +129,14 @@ function makeOctokit(options: { existingPulls?: PullRequestSummary[]; prNumber?: base: { ref: 'staging' }, }, })), - merge: vi.fn(async (_params: PullsMergeParams): Promise<{ data: { merged: true; sha: string } }> => ({ - data: { merged: true, sha: mergeSha }, - })), + merge: vi.fn( + async ( + _params: PullsMergeParams, + ): Promise<{ data: { merged: boolean; sha: string; message?: string } }> => + options.mergeResponse !== undefined + ? { data: options.mergeResponse } + : { data: { merged: true, sha: mergeSha } }, + ), }, }; } @@ -254,6 +268,42 @@ describe('G2 deliverCodeChangeViaPR', () => { }); }); + it('records merged only when the provider confirms it — merged:false parks with the provider reason', async () => { + const deliver = await loadDeliverCodeChangeViaPR(); + const octokit = makeOctokit({ + prNumber: 404, + mergeResponse: { merged: false, sha: '', message: 'Base branch was modified. Review and try the merge again.' }, + }); + const args = makeArgs({ octokit }); + + const result = await deliver(args); + + expect(octokit.pulls.merge).toHaveBeenCalledWith( + expect.objectContaining({ pull_number: 404, merge_method: 'squash' }), + ); + expect(result).toMatchObject({ merged: false, prNumber: 404 }); + expect((result as { reason?: string }).reason).toContain('Base branch was modified'); + expect((result as { mergeSha?: string }).mergeSha).toBeUndefined(); + expect(args.phaseArtifact.status).not.toBe('joined'); + }); + + it('forwards the configured required-check budget and poll interval to awaitRequiredChecks', async () => { + const deliver = await loadDeliverCodeChangeViaPR(); + const awaitRequiredChecks = vi.fn(async (): Promise => ({ status: 'green' })); + + await deliver( + makeArgs({ + awaitRequiredChecks, + checkBudgetMs: 300_000, + checkPollMs: 10_000, + }), + ); + + expect(awaitRequiredChecks).toHaveBeenCalledWith( + expect.objectContaining({ budgetMs: 300_000, pollMs: 10_000 }), + ); + }); + it('uses the same PR delivery lane for auto-merge decisions and operator-approved epoch re-entry', async () => { const deliver = await loadDeliverCodeChangeViaPR(); diff --git a/packages/daemon/src/control-plane/phases.ts b/packages/daemon/src/control-plane/phases.ts index 915f01e6..53b0ec21 100644 --- a/packages/daemon/src/control-plane/phases.ts +++ b/packages/daemon/src/control-plane/phases.ts @@ -361,6 +361,8 @@ export function createPhaseHandlers( function isLandingTarget(value: unknown): value is { landsOn: string; requiredChecks?: string[]; + checkBudgetMs?: number; + checkPollMs?: number; } { return ( typeof value === 'object' && @@ -373,7 +375,14 @@ export function createPhaseHandlers( function readLandingTarget( deploymentId: string, - ): { landsOn: string; requiredChecks: string[] } | undefined { + ): + | { + landsOn: string; + requiredChecks: string[]; + checkBudgetMs?: number; + checkPollMs?: number; + } + | undefined { if (registry === undefined) return undefined; const declared = registry.readDeclaredData(deploymentId, 'landing'); if (declared.kind !== 'found' || !isLandingTarget(declared.value)) { @@ -382,6 +391,8 @@ export function createPhaseHandlers( return { landsOn: declared.value.landsOn, requiredChecks: declared.value.requiredChecks ?? [], + checkBudgetMs: declared.value.checkBudgetMs, + checkPollMs: declared.value.checkPollMs, }; } @@ -2732,6 +2743,8 @@ export function createPhaseHandlers( phaseArtifact: integrateArtifact, awaitRequiredChecks: (args) => awaitRequiredChecks({ octokit, ...args }), + checkBudgetMs: landing?.checkBudgetMs, + checkPollMs: landing?.checkPollMs, pushFeatureBranch, trigger: isApprovedReEntry ? { kind: 'operator-approved-epoch', detail: 'mergeDecisionApprovedEpoch re-entry' } diff --git a/packages/daemon/src/control-plane/pr-delivery.ts b/packages/daemon/src/control-plane/pr-delivery.ts index d7b8a562..01d3570c 100644 --- a/packages/daemon/src/control-plane/pr-delivery.ts +++ b/packages/daemon/src/control-plane/pr-delivery.ts @@ -46,7 +46,7 @@ export interface DeliverCodeChangeViaPRArgs { repo: string; pull_number: number; merge_method: 'squash'; - }) => Promise<{ data: { merged: boolean; sha: string } }>; + }) => Promise<{ data: { merged: boolean; sha: string; message?: string } }>; }; }; owner: string; @@ -70,6 +70,10 @@ export interface DeliverCodeChangeViaPRArgs { trigger: { kind: 'auto-merge' | 'operator-approved-epoch'; detail: string }; /** Internal seam: when true the PR is opened/adopted but never merged. */ skipMerge?: boolean; + /** Total wait budget for required checks; falls back to awaitRequiredChecks' default. */ + checkBudgetMs?: number; + /** Poll interval for required checks; falls back to awaitRequiredChecks' default. */ + checkPollMs?: number; } export interface DeliverCodeChangeViaPRResult { @@ -118,6 +122,8 @@ export async function deliverCodeChangeViaPR({ awaitRequiredChecks, pushFeatureBranch, skipMerge = false, + checkBudgetMs, + checkPollMs, }: DeliverCodeChangeViaPRArgs): Promise { try { const now = new Date().toISOString(); @@ -228,6 +234,8 @@ export async function deliverCodeChangeViaPR({ repo, ref: featureBranch, requiredChecks, + ...(checkBudgetMs !== undefined ? { budgetMs: checkBudgetMs } : {}), + ...(checkPollMs !== undefined ? { pollMs: checkPollMs } : {}), }); if (checkResult.status !== 'green') { @@ -238,7 +246,8 @@ export async function deliverCodeChangeViaPR({ }; } - // 3. Squash-merge. + // 3. Squash-merge. The join transition is recorded ONLY on provider-confirmed + // `merged: true`; a 200 with `merged: false` parks with GitHub's reason. const mergeResponse = await octokit.pulls.merge({ owner, repo, @@ -246,6 +255,14 @@ export async function deliverCodeChangeViaPR({ merge_method: 'squash', }); + if (mergeResponse.data.merged !== true) { + return { + merged: false, + prNumber, + reason: `provider did not confirm merge: ${mergeResponse.data.message ?? 'no reason given'}`, + }; + } + phaseArtifact.status = 'joined'; phaseArtifact.mergeIdentifier = mergeResponse.data.sha; phaseArtifact.mergeSha = mergeResponse.data.sha;