diff --git a/setup-env/config/constants.js b/setup-env/config/constants.js index 84219c0..8a48670 100644 --- a/setup-env/config/constants.js +++ b/setup-env/config/constants.js @@ -24,9 +24,14 @@ module.exports = { // Object.keys(...).forEach(core.exportVariable, ...) call let any caller // who could influence the API response inject arbitrary env vars into // the workflow runner (CVSS 9.3 - env-var injection). + // Must list every name the rebuild/details response actually carries; a name missing + // here is dropped silently from the runner's perspective (only a ::warning:: in the log). ALLOWED_RERUN_ENV_VARS: [ 'BROWSERSTACK_RERUN', 'BROWSERSTACK_RERUN_TESTS', 'BROWSERSTACK_BUILD_NAME', + // SDK-7461: the SDKs read this as build_run_identifier in the build-start payload; + // without it a re-run is filed as an unrelated build run instead of an attempt of its parent. + 'BROWSERSTACK_BUILD_RUN_IDENTIFIER', ], }; diff --git a/setup-env/dist/index.js b/setup-env/dist/index.js index cdd56d3..f8391b0 100644 --- a/setup-env/dist/index.js +++ b/setup-env/dist/index.js @@ -30,10 +30,15 @@ module.exports = { // Object.keys(...).forEach(core.exportVariable, ...) call let any caller // who could influence the API response inject arbitrary env vars into // the workflow runner (CVSS 9.3 - env-var injection). + // Must list every name the rebuild/details response actually carries; a name missing + // here is dropped silently from the runner's perspective (only a ::warning:: in the log). ALLOWED_RERUN_ENV_VARS: [ 'BROWSERSTACK_RERUN', 'BROWSERSTACK_RERUN_TESTS', 'BROWSERSTACK_BUILD_NAME', + // SDK-7461: the SDKs read this as build_run_identifier in the build-start payload; + // without it a re-run is filed as an unrelated build run instead of an attempt of its parent. + 'BROWSERSTACK_BUILD_RUN_IDENTIFIER', ], }; @@ -34661,20 +34666,45 @@ class ActionInput { } async checkIfBStackReRun() { - // Ensure rerunAttempt is a number and greater than 1 + // Attempt 1 is an ordinary run, not a re-run — stay silent, this is not a failure. if (!this.rerunAttempt || Number(this.rerunAttempt) <= 1) { return false; } - // Ensure runId, repository, username, and accessKey are valid - if (!this.runId || !this.repository || this.repository === 'none' - || !this.githubToken || this.githubToken === 'none' || !this.username || !this.accessKey) { + // Past this point GitHub re-ran the workflow, so the failed-test list was meant to be + // delivered. Every bail below silently degrades the re-run into a full-suite run, which + // is indistinguishable from correct behaviour unless we say so here (SDK-7461). + const missing = []; + if (!this.githubToken || this.githubToken === 'none') missing.push("the 'github-token' input"); + if (!this.runId) missing.push('GITHUB_RUN_ID'); + if (!this.repository || this.repository === 'none') missing.push('GITHUB_REPOSITORY'); + if (!this.username) missing.push("the 'username' input"); + if (!this.accessKey) missing.push("the 'access-key' input"); + + if (missing.length) { + core.warning(`This is re-run attempt ${this.rerunAttempt}, but BrowserStack cannot deliver the failed-test list because ${missing.join(', ')} ${missing.length > 1 ? 'are' : 'is'} not set. Every test will run again instead of only the failed ones. Pass github-token to this action to enable re-running only failed tests.`); return false; } const triggeringActor = process.env.GITHUB_TRIGGERING_ACTOR; + + // The actor check is only a cheap pre-filter; the rebuild/details endpoint is the + // authority on whether BrowserStack triggered this re-run, and it returns no variables + // when it did not. GITHUB_TRIGGERING_ACTOR comes from the runner binary, so a + // self-hosted runner can simply not set it — bailing there would turn a working re-run + // into a full-suite run on an otherwise correct setup (SDK-7461). Ask the API instead. + if (!triggeringActor) { + core.warning(`This is re-run attempt ${this.rerunAttempt} and the runner did not report GITHUB_TRIGGERING_ACTOR, so BrowserStack cannot pre-confirm that it triggered this re-run — asking BrowserStack directly instead. This variable is set by the runner itself; on a self-hosted runner, updating the runner restores the faster check.`); + return true; + } + core.info(`Triggering actor is - ${triggeringActor}`); - return triggeringActor === this.githubApp; + if (triggeringActor !== this.githubApp) { + core.info(`This re-run was started by '${triggeringActor}', not by the BrowserStack GitHub App ('${this.githubApp}'), so there is no failed-test list to apply and every test will run again. Re-runs started from the BrowserStack dashboard run only the failed tests; check that the BrowserStack GitHub App is installed on ${this.repository}.`); + return false; + } + + return true; } async setBStackRerunEnvVars() { @@ -34710,7 +34740,8 @@ class ActionInput { }); } } catch (error) { - core.info(`Error setting BrowserStack rerun environment variables: ${error.message}`); + // Swallowing this as info hid a total delivery failure behind a normal-looking log. + core.warning(`Could not fetch the failed-test list from BrowserStack (${error.message}). Every test will run again instead of only the failed ones.`); } } } diff --git a/setup-env/src/actionInput/index.js b/setup-env/src/actionInput/index.js index 11c8c0c..da1ed32 100644 --- a/setup-env/src/actionInput/index.js +++ b/setup-env/src/actionInput/index.js @@ -94,20 +94,45 @@ class ActionInput { } async checkIfBStackReRun() { - // Ensure rerunAttempt is a number and greater than 1 + // Attempt 1 is an ordinary run, not a re-run — stay silent, this is not a failure. if (!this.rerunAttempt || Number(this.rerunAttempt) <= 1) { return false; } - // Ensure runId, repository, username, and accessKey are valid - if (!this.runId || !this.repository || this.repository === 'none' - || !this.githubToken || this.githubToken === 'none' || !this.username || !this.accessKey) { + // Past this point GitHub re-ran the workflow, so the failed-test list was meant to be + // delivered. Every bail below silently degrades the re-run into a full-suite run, which + // is indistinguishable from correct behaviour unless we say so here (SDK-7461). + const missing = []; + if (!this.githubToken || this.githubToken === 'none') missing.push("the 'github-token' input"); + if (!this.runId) missing.push('GITHUB_RUN_ID'); + if (!this.repository || this.repository === 'none') missing.push('GITHUB_REPOSITORY'); + if (!this.username) missing.push("the 'username' input"); + if (!this.accessKey) missing.push("the 'access-key' input"); + + if (missing.length) { + core.warning(`This is re-run attempt ${this.rerunAttempt}, but BrowserStack cannot deliver the failed-test list because ${missing.join(', ')} ${missing.length > 1 ? 'are' : 'is'} not set. Every test will run again instead of only the failed ones. Pass github-token to this action to enable re-running only failed tests.`); return false; } const triggeringActor = process.env.GITHUB_TRIGGERING_ACTOR; + + // The actor check is only a cheap pre-filter; the rebuild/details endpoint is the + // authority on whether BrowserStack triggered this re-run, and it returns no variables + // when it did not. GITHUB_TRIGGERING_ACTOR comes from the runner binary, so a + // self-hosted runner can simply not set it — bailing there would turn a working re-run + // into a full-suite run on an otherwise correct setup (SDK-7461). Ask the API instead. + if (!triggeringActor) { + core.warning(`This is re-run attempt ${this.rerunAttempt} and the runner did not report GITHUB_TRIGGERING_ACTOR, so BrowserStack cannot pre-confirm that it triggered this re-run — asking BrowserStack directly instead. This variable is set by the runner itself; on a self-hosted runner, updating the runner restores the faster check.`); + return true; + } + core.info(`Triggering actor is - ${triggeringActor}`); - return triggeringActor === this.githubApp; + if (triggeringActor !== this.githubApp) { + core.info(`This re-run was started by '${triggeringActor}', not by the BrowserStack GitHub App ('${this.githubApp}'), so there is no failed-test list to apply and every test will run again. Re-runs started from the BrowserStack dashboard run only the failed tests; check that the BrowserStack GitHub App is installed on ${this.repository}.`); + return false; + } + + return true; } async setBStackRerunEnvVars() { @@ -143,7 +168,8 @@ class ActionInput { }); } } catch (error) { - core.info(`Error setting BrowserStack rerun environment variables: ${error.message}`); + // Swallowing this as info hid a total delivery failure behind a normal-looking log. + core.warning(`Could not fetch the failed-test list from BrowserStack (${error.message}). Every test will run again instead of only the failed ones.`); } } } diff --git a/setup-env/test/actionInput/index.test.js b/setup-env/test/actionInput/index.test.js index bfab23a..53d4d84 100644 --- a/setup-env/test/actionInput/index.test.js +++ b/setup-env/test/actionInput/index.test.js @@ -241,6 +241,51 @@ describe('Action Input operations for fetching all inputs, triggering validation expect(result).to.be.false; delete process.env.GITHUB_TRIGGERING_ACTOR; }); + + it('Falls through to the API when the runner reports no triggering actor (SDK-7461)', async () => { + // Self-hosted runners set GITHUB_TRIGGERING_ACTOR only from a certain runner version. + // The actor check is a pre-filter, not the authority — bailing here turned a working + // re-run into a full-suite run on an otherwise correct setup. + const coreWarningStub = sinon.stub(core, 'warning'); + const actionInput = new ActionInput(); + delete process.env.GITHUB_TRIGGERING_ACTOR; + + const result = await actionInput.checkIfBStackReRun(); + + // eslint-disable-next-line no-unused-expressions + expect(result).to.be.true; + sinon.assert.calledWith(coreWarningStub, sinon.match(/did not report GITHUB_TRIGGERING_ACTOR/)); + sinon.assert.calledWith(coreWarningStub, sinon.match(/asking BrowserStack directly/)); + sinon.assert.neverCalledWith(coreWarningStub, sinon.match(/GitHub App is installed/)); + }); + + it('Warns which input is missing when a re-run cannot be delivered (SDK-7461)', async () => { + // Without this the job log for a degraded re-run is byte-identical to a healthy + // run, so "all tests ran again" is undiagnosable from the customer's side. + const coreWarningStub = sinon.stub(core, 'warning'); + const actionInput = new ActionInput(); + actionInput.githubToken = 'none'; + + const result = await actionInput.checkIfBStackReRun(); + + // eslint-disable-next-line no-unused-expressions + expect(result).to.be.false; + sinon.assert.calledWith(coreWarningStub, sinon.match(/github-token/)); + sinon.assert.calledWith(coreWarningStub, sinon.match(/Every test will run again/)); + }); + + it('Explains a human-triggered re-run rather than failing silently (SDK-7461)', async () => { + const coreInfoStub = sinon.stub(core, 'info'); + const actionInput = new ActionInput(); + process.env.GITHUB_TRIGGERING_ACTOR = 'someHuman'; + + const result = await actionInput.checkIfBStackReRun(); + + // eslint-disable-next-line no-unused-expressions + expect(result).to.be.false; + sinon.assert.calledWith(coreInfoStub, sinon.match(/not by the BrowserStack GitHub App/)); + delete process.env.GITHUB_TRIGGERING_ACTOR; + }); }); context('Set BrowserStack Rerun Environment Variables', () => { @@ -320,13 +365,39 @@ describe('Action Input operations for fetching all inputs, triggering validation }); it('Handles errors when BrowserStack API fails', async () => { + // SDK-7461: a delivery failure is a warning, not info — as info it was + // indistinguishable from a healthy run in the job log. + const coreWarningStub = sinon.stub(core, 'warning'); const actionInput = new ActionInput(); axiosGetStub.rejects(new Error('API failed')); await actionInput.setBStackRerunEnvVars(); - sinon.assert.calledTwice(core.info); + sinon.assert.calledWith(coreWarningStub, sinon.match(/API failed/)); + sinon.assert.calledWith(coreWarningStub, sinon.match(/Every test will run again/)); sinon.assert.neverCalledWith(core.exportVariable, sinon.match.any, sinon.match.any); }); + + it('Exports BROWSERSTACK_BUILD_RUN_IDENTIFIER from the API response (SDK-7461)', async () => { + // Regression: the rebuild/details response carries this alongside RERUN/RERUN_TESTS, + // and the SDKs send it as build_run_identifier to link a re-run to its parent build + // run. It was absent from ALLOWED_RERUN_ENV_VARS, so the allowlist dropped it. + const actionInput = new ActionInput(); + axiosGetStub.resolves({ + data: { + data: { + variables: { + BROWSERSTACK_RERUN: 'true', + BROWSERSTACK_RERUN_TESTS: 'cypress/tests/A/spec1.ts,cypress/tests/B/spec2.ts', + BROWSERSTACK_BUILD_RUN_IDENTIFIER: '1784546348645-29728341550', + }, + }, + }, + }); + + await actionInput.setBStackRerunEnvVars(); + + sinon.assert.calledWith(core.exportVariable, 'BROWSERSTACK_BUILD_RUN_IDENTIFIER', '1784546348645-29728341550'); + }); }); });