fix: make durable PR waits callback-driven#335
Draft
JWThewes wants to merge 3 commits into
Draft
Conversation
Members could not start intents without a personal GitHub/GitLab connection: the orchestrator resolved the starting user's token and init-ws failed asynchronously (checkout_failed / intent_branch_push_failed). Replace per-user runtime auth with project-level source-control bindings: - Owners/admins bind each repository once, choosing GitHub App (per-repo installation discovery) or explicitly confirmed github-oauth / gitlab-oauth delegation at step 1 of the create-space flow; the App path needs no personal connection at all (repo discovery runs on App-JWT-minted metadata-read installation tokens via /github/app/*). - Bindings hold opaque credential refs in a new DynamoDB table, never tokens, and auto-invalidate when the delegator disconnects, loses scopes, or leaves the project. - Intent starts validate bindings up front (409 SOURCE_CONTROL_NOT_READY with per-repo reasons) instead of failing mid-run. - AgentCore runtime is token-free: git operations obtain short-lived credentials from a credential-broker lambda (IAM-scoped to the runtime role, validates execution liveness) via a throwaway GIT_ASKPASS helper; tokens no longer enter invocation payloads or durable history. - Provider API operations (branches, PRs, issues) route through a source-control service lambda; per-provider API proxy routes and the platform-wide GitHub auth mode are removed. Also bumps js-yaml 4.2.0 -> 4.3.0 in the lockfile (GHSA-52cp-r559-cp3m, pre-existing advisory that now trips the pre-commit audit). Closes #332
…ng (#332) - Clear-text logging (js/clear-text-logging, alerts 17-20): error objects in the credential-resolution path can carry provider-derived text. Logging sites in credential-broker and source-control now emit only allowlisted error-code constants via loggableErrorCode() and no longer log error.message. - Polynomial regex (js/polynomial-redos): canonicalRepo trimmed leading and trailing slashes with /^\/+|\/+$/g over caller-supplied repo refs; replaced with a linear-time scan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #334
Important
This PR must not be merged until #333 has merged. After #333 merges, this branch must be rebased onto the latest
mainand the checks rerun before this PR is made ready for review.Problem
PR-per-unit review used durable timer polling. One production-like execution exhausted Lambda's 3,000-operation limit while waiting for review, leaving process state active after the durable execution had failed.
The affected intent also demonstrated why repair must preserve state: one reviewed unit PR had merged remotely while local state lagged, and another unit's construction stages and draft PR were already complete and reusable.
Approach
Recovery verification
The deployed dev recovery was exercised against the failed execution that motivated #334:
FAILEDwithExecutionLimitExceeded;SUCCEEDEDat attempt 0 and no artifacts were archived;PR_WAITSindex.The review PR was deliberately left unmerged.
Testing
npm test- 117 files, 2,331 tests passedintentsandv2-orchestratorLambda bundles built successfullyterraform fmt -check -recursive terraform