From aec1ac33076d241459ab0b1f74fa392cb82e709b Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Thu, 13 Aug 2026 23:55:06 +0530 Subject: [PATCH 1/5] feat: emit machine identity for slow-build diagnostics (PER-10496) New env.machine getter (hostname-based id with CI node-index suffix, per-provider run URL) sent as build-create attributes plus an X-Percy-Machine-Id header on snapshot/finalize calls so percy-api can track per-machine liveness on parallel builds and name the dead machine when one stops sending. The getter is excluded from env debug logging (those logs are uploaded with build logs) and the header is attached per-call, never in headers(), so it cannot reach off-domain requests. percy-api validates everything server-side; old CLIs degrade gracefully. --- packages/client/src/client.js | 19 +++++++++-- packages/client/test/client.test.js | 43 ++++++++++++++++++++++++ packages/env/src/environment.js | 47 ++++++++++++++++++++++++++- packages/env/test/environment.test.js | 41 +++++++++++++++++++++++ 4 files changed, 147 insertions(+), 3 deletions(-) diff --git a/packages/client/src/client.js b/packages/client/src/client.js index ad9f29881..5d8889394 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -383,6 +383,13 @@ export class PercyClient { 'skip-base-build': this.config.percy?.skipBaseBuild, 'testhub-build-uuid': this.env.testhubBuildUuid, 'testhub-build-run-id': this.env.testhubBuildRunId, + // machine identity for slow-build diagnostics (server-validated; + // percy-api may discard any of these) + ...(this.env.machine?.id ? { + 'machine-id': this.env.machine.id, + 'machine-hostname': this.env.machine.hostname, + 'machine-ci-run-url': this.env.machine.runUrl + } : {}), ...(dropinBaselineCandidate ? { 'dropin-baseline-candidate': true } : {}), ...(dropinBaselineSetup ? { 'dropin-baseline-setup': true } : {}), ...(visualConfig ? { 'visual-config': visualConfig } : {}), @@ -405,13 +412,21 @@ export class PercyClient { }); } + // Machine-identity header for per-machine liveness on parallel builds. + // Attached per-call (never in headers()) so it only ever reaches percy.io + // API endpoints — headers() is also used for off-domain requests. + machineHeaders() { + let id = this.env.machine?.id; + return id ? { 'X-Percy-Machine-Id': id } : {}; + } + // Finalizes the active build. When `all` is true, `all-shards=true` is // added as a query param so the API finalizes all other build shards. async finalizeBuild(buildId, { all = false } = {}) { validateId('build', buildId); let qs = all ? 'all-shards=true' : ''; this.log.debug(`Finalizing build ${buildId}...`); - return this.post(`builds/${buildId}/finalize?${qs}`, {}, { identifier: 'build.finalze' }); + return this.post(`builds/${buildId}/finalize?${qs}`, {}, { identifier: 'build.finalze' }, this.machineHeaders()); } // Retrieves build data by id. Requires a read access token. @@ -714,7 +729,7 @@ export class PercyClient { } } } - }, { identifier: 'snapshot.post', ...meta }); + }, { identifier: 'snapshot.post', ...meta }, this.machineHeaders()); } // Finalizes a snapshot. diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index bdc56f81e..2e14724e8 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -235,6 +235,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [] @@ -288,6 +291,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [] @@ -373,6 +379,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [] @@ -415,6 +424,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -458,6 +470,9 @@ describe('PercyClient', () => { 'cli-start-time': cliStartTime, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'auto_enabled_group', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -500,6 +515,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, 'skip-base-build': true, @@ -540,6 +558,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': 'test-uuid-123', 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [] @@ -579,6 +600,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': 'test-run-id-123', + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'user_created', partial: client.env.partial, tags: [] @@ -618,6 +642,9 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, + 'machine-id': client.env.machine.id, + 'machine-hostname': client.env.machine.hostname, + 'machine-ci-run-url': client.env.machine.runUrl, source: 'bstack_sdk_created', partial: client.env.partial, tags: [] @@ -1211,6 +1238,14 @@ describe('PercyClient', () => { expect(api.requests['/builds/123/finalize']).toBeDefined(); }); + it('sends the machine identity header for per-machine liveness', async () => { + await expectAsync(client.finalizeBuild(123)).toBeResolved(); + expect(api.requests['/builds/123/finalize'][0].headers).toEqual( + jasmine.objectContaining({ + 'X-Percy-Machine-Id': client.env.machine.id + })); + }); + it('can finalize all shards of a build', async () => { await expectAsync(client.finalizeBuild(123, { all: true })).toBeResolved(); expect(api.requests['/builds/123/finalize?all-shards=true']).toBeDefined(); @@ -1359,6 +1394,14 @@ describe('PercyClient', () => { .toBeRejectedWithError('Invalid build ID'); }); + it('sends the machine identity header for per-machine liveness', async () => { + await expectAsync(client.createSnapshot(123, { name: 'snap' })).toBeResolved(); + expect(api.requests['/builds/123/snapshots'][0].headers).toEqual( + jasmine.objectContaining({ + 'X-Percy-Machine-Id': client.env.machine.id + })); + }); + it('creates a snapshot', async () => { spyOn(fs.promises, 'readFile') .withArgs('foo/bar').and.resolveTo('bar'); diff --git a/packages/env/src/environment.js b/packages/env/src/environment.js index 222b51d35..4fdb05fad 100644 --- a/packages/env/src/environment.js +++ b/packages/env/src/environment.js @@ -1,3 +1,4 @@ +import os from 'os'; import { getCommitData, getJenkinsSha, @@ -405,6 +406,48 @@ export class PercyEnv { return !!partial && partial !== '0'; } + // machine identity for slow-build diagnostics (dead-CI-machine detection on + // parallel builds). percy-api validates all of these server-side and may + // discard any of them. Deliberately excluded from the getter debug logging + // below (like `token`) — env debug logs are uploaded with build logs, and + // hostnames should not ride along in them. + get machine() { + let hostname = null; + try { hostname = os.hostname() || null; } catch { hostname = null; } + + let index = null; + let runUrl = null; + switch (this.ci) { + case 'circle': + index = this.vars.CIRCLE_NODE_INDEX ?? null; + runUrl = this.vars.CIRCLE_BUILD_URL || null; + break; + case 'buildkite': + index = this.vars.BUILDKITE_PARALLEL_JOB ?? null; + runUrl = this.vars.BUILDKITE_BUILD_URL || null; + break; + case 'github': + runUrl = (this.vars.GITHUB_SERVER_URL && this.vars.GITHUB_REPOSITORY && this.vars.GITHUB_RUN_ID) + ? `${this.vars.GITHUB_SERVER_URL}/${this.vars.GITHUB_REPOSITORY}/actions/runs/${this.vars.GITHUB_RUN_ID}` + : null; + break; + case 'gitlab': + runUrl = this.vars.CI_JOB_URL || null; + break; + } + + // stable id: sanitized hostname, suffixed with the CI node index when the + // provider exposes one (the same host can run multiple shards) + let id = hostname && hostname.replace(/[^A-Za-z0-9._-]/g, '-'); + if (id && index != null && index !== '') id = `${id}.n${index}`; + + return { + id: id || null, + hostname: hostname || null, + runUrl: runUrl || null + }; + } + // percy token get token() { return this.vars.PERCY_TOKEN || null; @@ -441,7 +484,9 @@ Object.defineProperties(PercyEnv.prototype, ( get() { let value = get.call(this); Object.defineProperty(this, key, { value }); - if (key !== 'token') { + // `machine` carries a hostname and these debug logs are uploaded + // with build logs — keep it out, like the token. + if (key !== 'token' && key !== 'machine') { this.log.debug(`Detected ${key} as ${JSON.stringify(value)}`); } return value; diff --git a/packages/env/test/environment.test.js b/packages/env/test/environment.test.js index e66a0dda5..b344adef4 100644 --- a/packages/env/test/environment.test.js +++ b/packages/env/test/environment.test.js @@ -23,6 +23,47 @@ describe('PercyEnv', () => { }); }); + describe('machine', () => { + it('returns a sanitized hostname-based id and the hostname', () => { + let env = new PercyEnv({}); + expect(env.machine.hostname).toEqual(jasmine.any(String)); + expect(env.machine.id).toMatch(/^[A-Za-z0-9._-]+$/); + }); + + it('suffixes the CI node index and captures the run url on circle', () => { + let env = new PercyEnv({ + CIRCLECI: 'true', + CIRCLE_NODE_INDEX: '2', + CIRCLE_BUILD_URL: 'https://app.circleci.com/pipelines/x/1' + }); + expect(env.machine.id).toMatch(/\.n2$/); + expect(env.machine.runUrl).toEqual('https://app.circleci.com/pipelines/x/1'); + }); + + it('composes the github actions run url', () => { + let env = new PercyEnv({ + GITHUB_ACTIONS: 'true', + GITHUB_SERVER_URL: 'https://github.com', + GITHUB_REPOSITORY: 'org/repo', + GITHUB_RUN_ID: '123' + }); + expect(env.machine.runUrl).toEqual('https://github.com/org/repo/actions/runs/123'); + }); + + it('returns a null run url when the provider exposes none', () => { + let env = new PercyEnv({}); + expect(env.machine.runUrl).toBeNull(); + }); + + it('is excluded from getter debug logging', () => { + let env = new PercyEnv({}); + env.ci; // eslint-disable-line babel/no-unused-expressions -- warm nested getters + spyOn(env.log, 'debug'); + env.machine; // eslint-disable-line babel/no-unused-expressions + expect(env.log.debug).not.toHaveBeenCalled(); + }); + }); + describe('testhubBuildUuid', () => { it('should return TH_BUILD_UUID when it is set', () => { let env = new PercyEnv({ TH_BUILD_UUID: 'test_id' }); From ee8b24dea43c8efe07227f9053a46f10c6717a88 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 14 Aug 2026 13:46:08 +0530 Subject: [PATCH 2/5] test(env): cover every machine-identity branch for the 100% gate @percy/env enforces 100% line and branch coverage; the new machine getter left the buildkite/gitlab cases, the hostname failure path and the optional-var fallbacks uncovered. --- packages/env/test/environment.test.js | 61 +++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/packages/env/test/environment.test.js b/packages/env/test/environment.test.js index b344adef4..92d7b0676 100644 --- a/packages/env/test/environment.test.js +++ b/packages/env/test/environment.test.js @@ -1,3 +1,4 @@ +import os from 'os'; import PercyEnv from '@percy/env'; describe('PercyEnv', () => { @@ -50,11 +51,71 @@ describe('PercyEnv', () => { expect(env.machine.runUrl).toEqual('https://github.com/org/repo/actions/runs/123'); }); + it('suffixes the parallel job index and captures the run url on buildkite', () => { + let env = new PercyEnv({ + BUILDKITE: 'true', + BUILDKITE_PARALLEL_JOB: '3', + BUILDKITE_BUILD_URL: 'https://buildkite.com/org/pipe/builds/9' + }); + expect(env.machine.id).toMatch(/\.n3$/); + expect(env.machine.runUrl).toEqual('https://buildkite.com/org/pipe/builds/9'); + }); + + it('captures the job url on gitlab', () => { + let env = new PercyEnv({ + GITLAB_CI: 'true', + CI_SERVER_VERSION: '16.0', + CI_JOB_URL: 'https://gitlab.com/org/repo/-/jobs/42' + }); + expect(env.machine.runUrl).toEqual('https://gitlab.com/org/repo/-/jobs/42'); + }); + + it('omits the index suffix when the provider exposes no node index', () => { + let env = new PercyEnv({ BUILDKITE: 'true' }); + expect(env.machine.id).not.toMatch(/\.n/); + }); + + it('handles circle without a node index or build url', () => { + let env = new PercyEnv({ CIRCLECI: 'true' }); + expect(env.machine.id).not.toMatch(/\.n/); + expect(env.machine.runUrl).toBeNull(); + }); + + it('handles gitlab without a job url', () => { + let env = new PercyEnv({ GITLAB_CI: 'true', CI_SERVER_VERSION: '16.0' }); + expect(env.machine.runUrl).toBeNull(); + }); + + it('omits an incomplete github run url', () => { + let env = new PercyEnv({ GITHUB_ACTIONS: 'true', GITHUB_RUN_ID: '123' }); + expect(env.machine.runUrl).toBeNull(); + }); + it('returns a null run url when the provider exposes none', () => { let env = new PercyEnv({}); expect(env.machine.runUrl).toBeNull(); }); + it('degrades to null identity when the hostname cannot be read', () => { + spyOn(os, 'hostname').and.throwError('EPERM'); + let env = new PercyEnv({}); + expect(env.machine.hostname).toBeNull(); + expect(env.machine.id).toBeNull(); + }); + + it('treats an empty hostname as absent', () => { + spyOn(os, 'hostname').and.returnValue(''); + let env = new PercyEnv({}); + expect(env.machine.hostname).toBeNull(); + expect(env.machine.id).toBeNull(); + }); + + it('sanitizes characters that are invalid in a machine id', () => { + spyOn(os, 'hostname').and.returnValue('host name/with:chars'); + let env = new PercyEnv({}); + expect(env.machine.id).toEqual('host-name-with-chars'); + }); + it('is excluded from getter debug logging', () => { let env = new PercyEnv({}); env.ci; // eslint-disable-line babel/no-unused-expressions -- warm nested getters From 3c6ceff98c46c6593b67999f0babd95646ac1231 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 14 Aug 2026 16:20:25 +0530 Subject: [PATCH 3/5] test(client): cover the no-machine-identity branches @percy/client also enforces 100% branch coverage; the build-attribute spread and machineHeaders() fallbacks (old CLI / no resolvable hostname) were untested. --- packages/client/test/client.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 2e14724e8..f0fd3c1d4 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -260,6 +260,17 @@ describe('PercyClient', () => { expect(api.requests['/builds'][0].body.data.attributes.priority).toBeUndefined(); }); + it('omits machine attributes when no machine identity is available', async () => { + spyOnProperty(client.env, 'machine').and.returnValue({ id: null }); + + await client.createBuild(); + + let attributes = api.requests['/builds'][0].body.data.attributes; + expect(attributes['machine-id']).toBeUndefined(); + expect(attributes['machine-hostname']).toBeUndefined(); + expect(attributes['machine-ci-run-url']).toBeUndefined(); + }); + it('creates a new build with projectType passed as null', async () => { await expectAsync(client.createBuild({ projectType: null })).toBeResolvedTo({ data: { @@ -1246,6 +1257,14 @@ describe('PercyClient', () => { })); }); + it('omits the machine header when no machine identity is available', async () => { + spyOnProperty(client.env, 'machine').and.returnValue({ id: null }); + + await expectAsync(client.finalizeBuild(123)).toBeResolved(); + expect(api.requests['/builds/123/finalize'][0].headers['X-Percy-Machine-Id']) + .toBeUndefined(); + }); + it('can finalize all shards of a build', async () => { await expectAsync(client.finalizeBuild(123, { all: true })).toBeResolved(); expect(api.requests['/builds/123/finalize?all-shards=true']).toBeDefined(); From 6dc66771e6be967eea14d9f049ae3fead230d9b4 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Tue, 1 Sep 2026 18:04:40 +0530 Subject: [PATCH 4/5] feat: report the CI platform with machine identity (PER-10496) Co-Authored-By: Claude Opus 5 --- packages/client/src/client.js | 3 ++- packages/client/test/client.test.js | 10 ++++++++++ packages/env/src/environment.js | 5 ++++- packages/env/test/environment.test.js | 10 ++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/client/src/client.js b/packages/client/src/client.js index 5d8889394..c07895392 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -388,7 +388,8 @@ export class PercyClient { ...(this.env.machine?.id ? { 'machine-id': this.env.machine.id, 'machine-hostname': this.env.machine.hostname, - 'machine-ci-run-url': this.env.machine.runUrl + 'machine-ci-run-url': this.env.machine.runUrl, + 'machine-ci-platform': this.env.machine.platform } : {}), ...(dropinBaselineCandidate ? { 'dropin-baseline-candidate': true } : {}), ...(dropinBaselineSetup ? { 'dropin-baseline-setup': true } : {}), diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index f0fd3c1d4..c1e01e96e 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -238,6 +238,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [] @@ -269,6 +270,7 @@ describe('PercyClient', () => { expect(attributes['machine-id']).toBeUndefined(); expect(attributes['machine-hostname']).toBeUndefined(); expect(attributes['machine-ci-run-url']).toBeUndefined(); + expect(attributes['machine-ci-platform']).toBeUndefined(); }); it('creates a new build with projectType passed as null', async () => { @@ -305,6 +307,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [] @@ -393,6 +396,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [] @@ -438,6 +442,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -484,6 +489,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'auto_enabled_group', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -529,6 +535,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, 'skip-base-build': true, @@ -572,6 +579,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [] @@ -614,6 +622,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'user_created', partial: client.env.partial, tags: [] @@ -656,6 +665,7 @@ describe('PercyClient', () => { 'machine-id': client.env.machine.id, 'machine-hostname': client.env.machine.hostname, 'machine-ci-run-url': client.env.machine.runUrl, + 'machine-ci-platform': client.env.machine.platform, source: 'bstack_sdk_created', partial: client.env.partial, tags: [] diff --git a/packages/env/src/environment.js b/packages/env/src/environment.js index 4fdb05fad..fb48a7709 100644 --- a/packages/env/src/environment.js +++ b/packages/env/src/environment.js @@ -444,7 +444,10 @@ export class PercyEnv { return { id: id || null, hostname: hostname || null, - runUrl: runUrl || null + runUrl: runUrl || null, + // which CI product the agent belongs to ("jenkins", "buildkite", ...); + // percy-web maps it to a display name in the stopped-responding copy + platform: this.ci || null }; } diff --git a/packages/env/test/environment.test.js b/packages/env/test/environment.test.js index 92d7b0676..f7923372a 100644 --- a/packages/env/test/environment.test.js +++ b/packages/env/test/environment.test.js @@ -96,6 +96,16 @@ describe('PercyEnv', () => { expect(env.machine.runUrl).toBeNull(); }); + it('reports the CI platform when one is detected', () => { + let env = new PercyEnv({ JENKINS_URL: 'http://jenkins.local/' }); + expect(env.machine.platform).toEqual('jenkins'); + }); + + it('reports a null platform outside of CI', () => { + let env = new PercyEnv({}); + expect(env.machine.platform).toBeNull(); + }); + it('degrades to null identity when the hostname cannot be read', () => { spyOn(os, 'hostname').and.throwError('EPERM'); let env = new PercyEnv({}); From 7f69e079247f00a76b0981f20c26db51068716b3 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Wed, 9 Sep 2026 17:43:22 +0530 Subject: [PATCH 5/5] =?UTF-8?q?fix(env):=20harden=20machine=20identity=20?= =?UTF-8?q?=E2=80=94=20header-safe=20shard=20index,=20per-shard=20ids=20on?= =?UTF-8?q?=20gitlab/jenkins,=20buildkite=20job=20anchor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the machine-identity getter: - Sanitize the CI shard index like the hostname: the id is an HTTP header value on every snapshot POST, and a stray newline in CIRCLE_NODE_INDEX rejected the upload. - Suffix GitLab CI_NODE_INDEX and Jenkins EXECUTOR_NUMBER so several shards on one host stop sharing a machine id (a dead shard looked alive while a sibling uploaded). - Anchor the Buildkite run url to BUILDKITE_JOB_ID so the link lands on the dead job. - Cap the id at the API's 128 chars; drop an id that sanitizes to dashes alone (non-ASCII hostnames collided); send null instead of the CI/unknown marker. - Tests: one machineAttrs mapping in client.test.js instead of nine pasted copies. Co-Authored-By: Claude Fable 5.1 --- packages/client/test/client.test.js | 53 +++++++++----------------- packages/env/src/environment.js | 55 ++++++++++++++++++++------- packages/env/test/environment.test.js | 53 ++++++++++++++++++++++++-- 3 files changed, 109 insertions(+), 52 deletions(-) diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index c1e01e96e..d6b5a09ef 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -195,6 +195,14 @@ describe('PercyClient', () => { describe('#createBuild()', () => { let cliStartTime = new Date().toISOString(); + // the wire mapping for machine identity, kept in one place so a renamed key + // is one edit rather than one per expectation + let machineAttrs = env => ({ + 'machine-id': env.machine.id, + 'machine-hostname': env.machine.hostname, + 'machine-ci-run-url': env.machine.runUrl, + 'machine-ci-platform': env.machine.platform + }); beforeEach(() => { delete process.env.PERCY_AUTO_ENABLED_GROUP_BUILD; delete process.env.PERCY_ORIGINATED_SOURCE; @@ -235,10 +243,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [] @@ -304,10 +309,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [] @@ -393,10 +395,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [] @@ -439,10 +438,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -486,10 +482,7 @@ describe('PercyClient', () => { 'cli-start-time': cliStartTime, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'auto_enabled_group', partial: client.env.partial, tags: [{ id: null, name: 'tag1' }, { id: null, name: 'tag2' }] @@ -532,10 +525,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, 'skip-base-build': true, @@ -576,10 +566,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': 'test-uuid-123', 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [] @@ -619,10 +606,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': 'test-run-id-123', - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'user_created', partial: client.env.partial, tags: [] @@ -662,10 +646,7 @@ describe('PercyClient', () => { 'cli-start-time': null, 'testhub-build-uuid': client.env.testhubBuildUuid, 'testhub-build-run-id': client.env.testhubBuildRunId, - 'machine-id': client.env.machine.id, - 'machine-hostname': client.env.machine.hostname, - 'machine-ci-run-url': client.env.machine.runUrl, - 'machine-ci-platform': client.env.machine.platform, + ...machineAttrs(client.env), source: 'bstack_sdk_created', partial: client.env.partial, tags: [] diff --git a/packages/env/src/environment.js b/packages/env/src/environment.js index fb48a7709..d51ae1ae5 100644 --- a/packages/env/src/environment.js +++ b/packages/env/src/environment.js @@ -6,6 +6,15 @@ import { } from './utils.js'; import logger from '@percy/logger'; +// machine ids are capped and restricted to header-safe characters on both +// sides; percy-api rejects anything longer or outside this alphabet +const MACHINE_ID_MAX_LENGTH = 128; + +function machineToken(value) { + if (value == null || value === '') return null; + return String(value).replace(/[^A-Za-z0-9._-]/g, '-'); +} + export class PercyEnv { constructor(vars = process.env) { this.vars = vars; @@ -415,16 +424,23 @@ export class PercyEnv { let hostname = null; try { hostname = os.hostname() || null; } catch { hostname = null; } + // the per-shard index for providers that can run several shards on one + // host; without it every shard on that host would share a machine id and a + // dead shard would look alive as long as any sibling kept uploading let index = null; let runUrl = null; switch (this.ci) { case 'circle': - index = this.vars.CIRCLE_NODE_INDEX ?? null; - runUrl = this.vars.CIRCLE_BUILD_URL || null; + index = this.vars.CIRCLE_NODE_INDEX; + runUrl = this.vars.CIRCLE_BUILD_URL; break; case 'buildkite': - index = this.vars.BUILDKITE_PARALLEL_JOB ?? null; - runUrl = this.vars.BUILDKITE_BUILD_URL || null; + index = this.vars.BUILDKITE_PARALLEL_JOB; + // the build url is shared by every parallel job; the job id anchor is + // what lets the "stopped responding" link land on the dead agent's log + runUrl = this.vars.BUILDKITE_BUILD_URL && this.vars.BUILDKITE_JOB_ID + ? `${this.vars.BUILDKITE_BUILD_URL}#${this.vars.BUILDKITE_JOB_ID}` + : this.vars.BUILDKITE_BUILD_URL; break; case 'github': runUrl = (this.vars.GITHUB_SERVER_URL && this.vars.GITHUB_REPOSITORY && this.vars.GITHUB_RUN_ID) @@ -432,22 +448,35 @@ export class PercyEnv { : null; break; case 'gitlab': - runUrl = this.vars.CI_JOB_URL || null; + index = this.vars.CI_NODE_INDEX; + runUrl = this.vars.CI_JOB_URL; + break; + case 'jenkins': + case 'jenkins-prb': + index = this.vars.EXECUTOR_NUMBER; break; } - // stable id: sanitized hostname, suffixed with the CI node index when the - // provider exposes one (the same host can run multiple shards) - let id = hostname && hostname.replace(/[^A-Za-z0-9._-]/g, '-'); - if (id && index != null && index !== '') id = `${id}.n${index}`; + // stable id: sanitized hostname, suffixed with the sanitized shard index. + // The id travels as an HTTP header value on every snapshot POST, so every + // part of it must be header-safe — an unsanitized index with a stray + // newline would reject the whole upload. + let id = machineToken(hostname); + let shard = machineToken(index); + if (id && shard) id = `${id}.n${shard}`; + if (id) id = id.slice(0, MACHINE_ID_MAX_LENGTH); + // a hostname with no ASCII alphanumerics sanitizes to dashes alone, which + // identifies nothing and would collide across hosts — better no id at all + if (id && !/[A-Za-z0-9]/.test(id)) id = null; return { - id: id || null, - hostname: hostname || null, + id, + hostname, runUrl: runUrl || null, // which CI product the agent belongs to ("jenkins", "buildkite", ...); - // percy-web maps it to a display name in the stopped-responding copy - platform: this.ci || null + // percy-web maps it to a display name in the stopped-responding copy. + // The generic CI/unknown marker is not a product and is dropped. + platform: this.ci && this.ci !== 'CI/unknown' ? this.ci : null }; } diff --git a/packages/env/test/environment.test.js b/packages/env/test/environment.test.js index f7923372a..da7c13cdc 100644 --- a/packages/env/test/environment.test.js +++ b/packages/env/test/environment.test.js @@ -51,25 +51,52 @@ describe('PercyEnv', () => { expect(env.machine.runUrl).toEqual('https://github.com/org/repo/actions/runs/123'); }); - it('suffixes the parallel job index and captures the run url on buildkite', () => { + it('suffixes the parallel job index and anchors the run url to the job on buildkite', () => { let env = new PercyEnv({ BUILDKITE: 'true', BUILDKITE_PARALLEL_JOB: '3', - BUILDKITE_BUILD_URL: 'https://buildkite.com/org/pipe/builds/9' + BUILDKITE_BUILD_URL: 'https://buildkite.com/org/pipe/builds/9', + BUILDKITE_JOB_ID: '0192a-job' }); expect(env.machine.id).toMatch(/\.n3$/); + expect(env.machine.runUrl).toEqual('https://buildkite.com/org/pipe/builds/9#0192a-job'); + }); + + it('falls back to the build url on buildkite without a job id', () => { + let env = new PercyEnv({ + BUILDKITE: 'true', + BUILDKITE_BUILD_URL: 'https://buildkite.com/org/pipe/builds/9' + }); expect(env.machine.runUrl).toEqual('https://buildkite.com/org/pipe/builds/9'); }); - it('captures the job url on gitlab', () => { + it('suffixes the parallel node index and captures the job url on gitlab', () => { let env = new PercyEnv({ GITLAB_CI: 'true', CI_SERVER_VERSION: '16.0', + CI_NODE_INDEX: '2', CI_JOB_URL: 'https://gitlab.com/org/repo/-/jobs/42' }); + expect(env.machine.id).toMatch(/\.n2$/); expect(env.machine.runUrl).toEqual('https://gitlab.com/org/repo/-/jobs/42'); }); + it('suffixes the executor number on jenkins so shards on one agent stay distinct', () => { + let env = new PercyEnv({ JENKINS_URL: 'http://jenkins.local/', EXECUTOR_NUMBER: '1' }); + expect(env.machine.id).toMatch(/\.n1$/); + }); + + it('keeps a zero shard index', () => { + let env = new PercyEnv({ CIRCLECI: 'true', CIRCLE_NODE_INDEX: '0' }); + expect(env.machine.id).toMatch(/\.n0$/); + }); + + it('sanitizes the shard index so the id stays header-safe', () => { + spyOn(os, 'hostname').and.returnValue('host'); + let env = new PercyEnv({ CIRCLECI: 'true', CIRCLE_NODE_INDEX: '2\n' }); + expect(env.machine.id).toEqual('host.n2-'); + }); + it('omits the index suffix when the provider exposes no node index', () => { let env = new PercyEnv({ BUILDKITE: 'true' }); expect(env.machine.id).not.toMatch(/\.n/); @@ -106,6 +133,26 @@ describe('PercyEnv', () => { expect(env.machine.platform).toBeNull(); }); + it('reports a null platform for an unrecognized CI', () => { + let env = new PercyEnv({ CI: 'true' }); + expect(env.ci).toEqual('CI/unknown'); + expect(env.machine.platform).toBeNull(); + }); + + it('caps the id at the length the API accepts', () => { + spyOn(os, 'hostname').and.returnValue('h'.repeat(300)); + let env = new PercyEnv({}); + expect(env.machine.id).toHaveSize(128); + expect(env.machine.hostname).toHaveSize(300); + }); + + it('drops an id that sanitizes to nothing identifying', () => { + spyOn(os, 'hostname').and.returnValue('сервер'); + let env = new PercyEnv({}); + expect(env.machine.id).toBeNull(); + expect(env.machine.hostname).toEqual('сервер'); + }); + it('degrades to null identity when the hostname cannot be read', () => { spyOn(os, 'hostname').and.throwError('EPERM'); let env = new PercyEnv({});