Skip to content

fix(control-plane): confirm provider merge + configurable required-check wait policy - #3

Open
DanielSoCra wants to merge 2 commits into
mainfrom
claude/github-issue-discussion-2ad0e5
Open

fix(control-plane): confirm provider merge + configurable required-check wait policy#3
DanielSoCra wants to merge 2 commits into
mainfrom
claude/github-issue-discussion-2ad0e5

Conversation

@DanielSoCra

Copy link
Copy Markdown
Owner

Extracts the two bugfix-class quick wins from #2 so they land ahead of the full integration-contract design.

Changes

  1. Provider-confirmed merge (fail-closed). deliverCodeChangeViaPR previously set merged: true and marked the phase artifact joined right after the merge API call, without checking mergeResponse.data.merged. It now requires merged === true; a response with merged: false returns a parked result that preserves GitHub's message (e.g. "Base branch was modified.") and leaves the artifact un-joined.

  2. Configurable required-check wait policy. awaitRequiredChecks already accepted budgetMs/pollMs, but the delivery path never passed them — every deployment was pinned to the 60s default budget. LandingTarget now accepts optional checkBudgetMs/checkPollMs (strict zod, positive ints), threaded from the deployment registry through readLandingTarget into the check-polling call. Absent values keep today's defaults, so existing profiles are unaffected.

Tests

  • p1-pr-delivery.gate.test.ts: regression for merged: false (result, reason, artifact status) and for budget/poll forwarding.
  • deployment-registry/schema.test.ts: wait-policy fields parse; non-positive checkBudgetMs is rejected.
  • Full src/control-plane suite: 147 files / 1866 tests green. tsc --noEmit clean. The 10 eslint errors in phases.ts are pre-existing (identical on baseline).

Spec: STACK-AC-CONTROLLED-ARTIFACT-DELIVERY (no new files; traceability unchanged).

Refs #2

🤖 Generated with Claude Code

…policy (#2 quick wins)

Two fail-closed fixes extracted from #2:

- deliverCodeChangeViaPR records the join transition only when GitHub
  responds merged: true; a 200 with merged: false now parks the work
  item with the provider's message instead of reporting success.
- LandingTarget gains optional checkBudgetMs/checkPollMs so a
  deployment whose required checks take longer than the built-in 60s
  budget can be supported without a source change. Values thread from
  the deployment registry through readLandingTarget into
  awaitRequiredChecks; absent values keep the existing defaults.

Regression tests cover merged: false, budget forwarding, and schema
acceptance/rejection of the new wait-policy fields.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation contract (#2)

Drafted from the Operator-authored issue #2 (remaining scope after the
quick wins in PR #3): semantic label roles, proposal wording templates,
per-repository check patience, governance/containment profiles,
read-only compatibility preflight, and fail-closed behavior on an
incomplete contract. Status draft — L1 content awaits Operator
approval before L2/L3 are written.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant