From bb310870edf2946659d378f29160445effd93671 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Tue, 20 Oct 2026 01:15:08 +0200 Subject: [PATCH] bugfix-605-issue-branch-preparation-loses-git-authentication-with-credential-free-checkout: authenticate issue branch preparation and gate readiness guidance --- build/cli/index.js | 4 +- build/github_action/index.js | 4 +- docs/issues/branch-management.mdx | 4 ++ .../usecases/__tests__/issue_use_case.test.ts | 39 ++++++++++++++++++- src/application/usecases/issue_workflow.ts | 2 +- .../__tests__/git_cli_repository.test.ts | 20 ++++++++++ .../issue_use_case_composition_root.test.ts | 3 ++ .../issue_use_case_composition_root.ts | 2 +- 8 files changed, 71 insertions(+), 7 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index c0dc5ef1d..13ca39972 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -58883,7 +58883,7 @@ async function runIssueWorkflow(context, taskId, ports) { } const agentAllowed = !context.membersOnly || Boolean(ports.actorAuthorizationPort && await ports.actorAuthorizationPort.isActorAllowedToUseMemberOnlyAutomation(context.actor)); - const recommendation = context.started && !sddWaiting && (!context.sddRequired || branchReady) && agentAllowed + const recommendation = context.started && !sddWaiting && (!context.branchRequired || branchReady) && agentAllowed ? context.recommendation : undefined; if (recommendation) { const recommendationOutcome = recommendation === 'answer-help' @@ -94843,7 +94843,7 @@ function createIssueUseCaseCompositionRoot(binding) { const organizationMembers = (0, organization_members_composition_root_1.createOrganizationMembersCompositionRoot)(); const branchLifecycle = new branch_lifecycle_repository_1.BranchLifecycleRepository((0, github_branch_client_factory_1.createBranchClient)()); const branchName = new branch_name_repository_1.BranchNameRepository(); - const gitCli = new git_cli_repository_1.GitCliRepository(); + const gitCli = new git_cli_repository_1.GitCliRepository(binding.token); const linkedBranch = new linked_branch_repository_1.LinkedBranchRepository((0, github_project_client_factory_1.createGraphqlTransportClient)()); const branchPropagationDelay = new timer_branch_propagation_delay_adapter_1.TimerBranchPropagationDelayAdapter(); const projectBoard = (0, project_board_composition_root_1.createProjectBoardCompositionRoot)(); diff --git a/build/github_action/index.js b/build/github_action/index.js index a243330fe..0f9bd128a 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -56715,7 +56715,7 @@ async function runIssueWorkflow(context, taskId, ports) { } const agentAllowed = !context.membersOnly || Boolean(ports.actorAuthorizationPort && await ports.actorAuthorizationPort.isActorAllowedToUseMemberOnlyAutomation(context.actor)); - const recommendation = context.started && !sddWaiting && (!context.sddRequired || branchReady) && agentAllowed + const recommendation = context.started && !sddWaiting && (!context.branchRequired || branchReady) && agentAllowed ? context.recommendation : undefined; if (recommendation) { const recommendationOutcome = recommendation === 'answer-help' @@ -85343,7 +85343,7 @@ function createIssueUseCaseCompositionRoot(binding) { const organizationMembers = (0, organization_members_composition_root_1.createOrganizationMembersCompositionRoot)(); const branchLifecycle = new branch_lifecycle_repository_1.BranchLifecycleRepository((0, github_branch_client_factory_1.createBranchClient)()); const branchName = new branch_name_repository_1.BranchNameRepository(); - const gitCli = new git_cli_repository_1.GitCliRepository(); + const gitCli = new git_cli_repository_1.GitCliRepository(binding.token); const linkedBranch = new linked_branch_repository_1.LinkedBranchRepository((0, github_project_client_factory_1.createGraphqlTransportClient)()); const branchPropagationDelay = new timer_branch_propagation_delay_adapter_1.TimerBranchPropagationDelayAdapter(); const projectBoard = (0, project_board_composition_root_1.createProjectBoardCompositionRoot)(); diff --git a/docs/issues/branch-management.mdx b/docs/issues/branch-management.mdx index e602ef937..7878128ed 100644 --- a/docs/issues/branch-management.mdx +++ b/docs/issues/branch-management.mdx @@ -20,6 +20,10 @@ To work without Action-managed branches, set `issue-managed-branches: false`. Th For a regular branch-bearing issue, the visible sequence is `in-progress` → linked branch → `branched` → implementation. The Action checks the branch's exact GitHub issue link and remote SHA on every issue run. If the link or branch is missing, it removes an unsupported `branched` label. If verification fails, it reports the error and pauses branch-dependent work. +Implementation recommendations also wait for verified branch readiness, including issues that do not require an SDD. A failed or pending verification cannot publish a new “Ready to start” plan. Branchless workflows can still provide guidance without branch evidence. + +In private repositories, keep `persist-credentials: false` on the checkout and pass the configured credential through the Action's `token` input. Issue branch preparation authenticates its Git fetches using configuration scoped to each Git process; it does not store credentials in the checkout configuration or remote URL. If preparation fails, inspect the reported error, correct token access if needed, and rerun the same issue workflow. + With `pre-branch-sdd: true`, an eligible feature or behavior-changing issue enters `state:specifying` first. The Action clarifies the contract in issue comments, validates the SDD outside the new branch, creates the linked branch, and publishes the SDD as its first commit. Only after the remote commit is verified does it add `branched` and move to `state:working`. ## Naming and source branches diff --git a/src/application/usecases/__tests__/issue_use_case.test.ts b/src/application/usecases/__tests__/issue_use_case.test.ts index c352c49ea..358d968ab 100644 --- a/src/application/usecases/__tests__/issue_use_case.test.ts +++ b/src/application/usecases/__tests__/issue_use_case.test.ts @@ -15,6 +15,7 @@ const mockLinkIssueProjectInvoke = jest.fn(); const mockCheckPriorityInvoke = jest.fn(); const mockResumeProjectInvoke = jest.fn(); const mockPrepareBranchesInvoke = jest.fn(); +const mockReconcileBranchReadinessInvoke = jest.fn(); const mockRemoveNotNeededInvoke = jest.fn(); const mockDeployAddedInvoke = jest.fn(); const mockRecommendStepsInvoke = jest.fn(); @@ -120,7 +121,7 @@ function minimalExecution(overrides: Record = {}): Execution { function createUseCase( actorAuthorizationPort?: ConstructorParameters[4], preBranchSddGate?: ConstructorParameters[5], - reconcileBranchReadiness?: jest.Mock, + reconcileBranchReadiness: jest.Mock = mockReconcileBranchReadinessInvoke, ): IssueUseCase { return new IssueUseCase( { taskId: "RecommendStepsUseCase", invoke: mockRecommendStepsInvoke }, @@ -150,6 +151,10 @@ describe("IssueUseCase", () => { mockCheckPriorityInvoke.mockResolvedValue([]); mockResumeProjectInvoke.mockResolvedValue([new Result({ id: "resume-project", success: true, executed: false })]); mockPrepareBranchesInvoke.mockResolvedValue({ results: [], configurationPatch: {} }); + mockReconcileBranchReadinessInvoke.mockResolvedValue([new Result({ + id: 'ReconcileBranchReadinessUseCase', success: true, executed: false, + payload: { branchName: 'feature/8-issue', branchSha: 'a'.repeat(40) }, + })]); mockRemoveNotNeededInvoke.mockResolvedValue([]); mockDeployAddedInvoke.mockResolvedValue([]); mockRecommendStepsInvoke.mockResolvedValue({ results: [] }); @@ -423,6 +428,38 @@ describe("IssueUseCase", () => { expect(mockDeployAddedInvoke).toHaveBeenCalledTimes(1); }); + it.each(['preparation failed', 'verification failed', 'verification pending', 'empty verification'])( + 'withholds implementation guidance when branch %s', async kind => { + const failure = new Result({ id: 'PrepareBranchesUseCase', success: false, executed: true, + steps: ['Unable to prepare the issue branch.'] }); + if (kind === 'preparation failed') mockPrepareBranchesInvoke.mockResolvedValue({ results: [failure], configurationPatch: {} }); + const readiness = kind === 'verification failed' + ? [new Result({ id: 'ReconcileBranchReadinessUseCase', success: false, executed: true })] + : kind === 'empty verification' ? [] + : [new Result({ id: 'ReconcileBranchReadinessUseCase', success: true, executed: false })]; + mockReconcileBranchReadinessInvoke.mockResolvedValue(readiness); + const param = minimalExecution({ issue: { labeled: true, labelAdded: 'in-progress' } }); + + const results = await createUseCase().invoke(param); + + expect(mockRecommendStepsInvoke).not.toHaveBeenCalled(); + expect(mockRemoveNotNeededInvoke).not.toHaveBeenCalled(); + expect(mockDeployAddedInvoke).not.toHaveBeenCalled(); + expect(param.currentConfiguration.recommendationState).toBeUndefined(); + if (kind === 'preparation failed') expect(results).toContain(failure); + }, + ); + + it('keeps branchless implementation guidance available without branch evidence', async () => { + mockReconcileBranchReadinessInvoke.mockResolvedValue([]); + await createUseCase().invoke(minimalExecution({ + issueStartDecision: { started: true, branchRequired: false, sddRequired: false, helpRequired: false }, + issue: { labeled: true, labelAdded: 'in-progress', issueManagedBranches: false }, + })); + expect(mockPrepareBranchesInvoke).not.toHaveBeenCalled(); + expect(mockRecommendStepsInvoke).toHaveBeenCalledTimes(1); + }); + it("recommends steps for a newly opened non-release issue", async () => { mockRecommendStepsInvoke.mockResolvedValue({ results: [new Result({ diff --git a/src/application/usecases/issue_workflow.ts b/src/application/usecases/issue_workflow.ts index ee8e5f0a7..d7dd326d1 100644 --- a/src/application/usecases/issue_workflow.ts +++ b/src/application/usecases/issue_workflow.ts @@ -186,7 +186,7 @@ export async function runIssueWorkflow( ports.actorAuthorizationPort && await ports.actorAuthorizationPort.isActorAllowedToUseMemberOnlyAutomation(context.actor), ); - const recommendation = context.started && !sddWaiting && (!context.sddRequired || branchReady) && agentAllowed + const recommendation = context.started && !sddWaiting && (!context.branchRequired || branchReady) && agentAllowed ? context.recommendation : undefined; if (recommendation) { const recommendationOutcome = recommendation === 'answer-help' diff --git a/src/data/repository/__tests__/git_cli_repository.test.ts b/src/data/repository/__tests__/git_cli_repository.test.ts index 1c975f442..d4a9f03dc 100644 --- a/src/data/repository/__tests__/git_cli_repository.test.ts +++ b/src/data/repository/__tests__/git_cli_repository.test.ts @@ -26,6 +26,26 @@ describe('GitCliRepository', () => { }); describe('fetchRemoteBranches', () => { + it('authenticates each fetch with process-scoped configuration', async () => { + const environment = { ...process.env }; + mockExec.mockResolvedValue(0); + await new GitCliRepository('synthetic-token').fetchRemoteBranches(); + + const authentication = { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.https://github.com/.extraheader', + GIT_CONFIG_VALUE_0: `AUTHORIZATION: basic ${Buffer.from('x-access-token:synthetic-token').toString('base64')}`, + }; + expect(mockExec).toHaveBeenNthCalledWith(1, 'git', ['fetch', '--tags', '--force'], { + env: expect.objectContaining(authentication), + }); + expect(mockExec).toHaveBeenNthCalledWith(2, 'git', ['fetch', '--all', '-v'], { + env: expect.objectContaining(authentication), + }); + expect(mockExec).toHaveBeenCalledTimes(2); + expect(process.env).toEqual(environment); + }); + it('runs fetch --tags --force and fetch --all -v', async () => { mockExec.mockResolvedValue(0); diff --git a/src/infrastructure/composition/__tests__/issue_use_case_composition_root.test.ts b/src/infrastructure/composition/__tests__/issue_use_case_composition_root.test.ts index 270625663..d0c33ecbd 100644 --- a/src/infrastructure/composition/__tests__/issue_use_case_composition_root.test.ts +++ b/src/infrastructure/composition/__tests__/issue_use_case_composition_root.test.ts @@ -88,6 +88,7 @@ const composeIssueUseCase = jest.fn( jest.mock("../issue_use_case_composition", () => ({ composeIssueUseCase })); import { createIssueUseCaseCompositionRoot } from "../issue_use_case_composition_root"; +import { GitCliRepository } from "../../../data/repository/git_cli_repository"; describe("issue use case composition root", () => { beforeEach(() => jest.clearAllMocks()); @@ -118,5 +119,7 @@ describe("issue use case composition root", () => { removeNotNeededBranches: expect.objectContaining({ invoke: expect.any(Function) }), })); expect(createBranchClient).toHaveBeenCalledTimes(1); + expect(GitCliRepository).toHaveBeenCalledTimes(1); + expect(GitCliRepository).toHaveBeenCalledWith('token'); }); }); diff --git a/src/infrastructure/composition/issue_use_case_composition_root.ts b/src/infrastructure/composition/issue_use_case_composition_root.ts index c987139a5..198adf155 100644 --- a/src/infrastructure/composition/issue_use_case_composition_root.ts +++ b/src/infrastructure/composition/issue_use_case_composition_root.ts @@ -82,7 +82,7 @@ export function createIssueUseCaseCompositionRoot(binding: RepositoryCredentialB const organizationMembers = createOrganizationMembersCompositionRoot(); const branchLifecycle = new BranchLifecycleRepository(createBranchClient()); const branchName = new BranchNameRepository(); - const gitCli = new GitCliRepository(); + const gitCli = new GitCliRepository(binding.token); const linkedBranch = new LinkedBranchRepository( createGraphqlTransportClient(), );