From 4f0f9b27969457070796107df6d6343008015a54 Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:34:04 -0500 Subject: [PATCH 1/4] ci(windows): record bounded Bash temp diagnostics --- .../issue-refresh-router-workflow.test.mjs | 4 +- .../scripts/windows-bash-temp-diagnostics.mjs | 98 +++++++++++++++++ .../windows-bash-temp-diagnostics.test.mjs | 103 ++++++++++++++++++ .github/scripts/windows-suite.test.mjs | 26 ++++- .github/workflows/ci.yml | 20 +++- CONTRIBUTING.md | 13 ++- 6 files changed, 256 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/windows-bash-temp-diagnostics.mjs create mode 100644 .github/scripts/windows-bash-temp-diagnostics.test.mjs diff --git a/.github/scripts/issue-refresh-router-workflow.test.mjs b/.github/scripts/issue-refresh-router-workflow.test.mjs index 4fafb985f..cc4cbe54a 100644 --- a/.github/scripts/issue-refresh-router-workflow.test.mjs +++ b/.github/scripts/issue-refresh-router-workflow.test.mjs @@ -72,12 +72,14 @@ test('workflow baselines retain readiness guards and pin original-event identity // #2160 permits repeated parameterized names while retaining numeric TAP identity checks. // #1891 adds only the observational Windows suite/discovery, including trusted Bash/jq prerequisites, // Git/bin root resolution and its native Windows path contract step. + // #2179 adds safe temp observations and post-suite representative diagnostics/artifacts; + // the original suite block is unchanged, with collector contracts registered alongside it. // Normalize to LF for Windows and archives; retain strict full-file hash comparisons. const baseline = { '.github/workflows/pr-check.yml': '33259c75e4f7dfdfca3143fc5efa0521a8ca289f06eca0d3efb429fbad7565bf', '.github/workflows/issue-validation.yml': 'd996ffae8e1341390b818e294a1a34a70f1b06f5eabda03cb96c0f065f7115b8', '.github/workflows/issue-policy.yml': '5f8bfbd9ab354f6b60d3ef35d30d5f4ee43a97eb83f61d8e0a955da37947adfd', - '.github/workflows/ci.yml': 'aeb32473db4636bb74e2a7a100361101e569f64ea0be5e6351ef22737eac1df6', + '.github/workflows/ci.yml': '66148266af9aaf1fc0c638620a9214e722cc9557d290b3f52d6844ecae7edad3', }; for (const [path, expected] of Object.entries(baseline)) { const source = path.endsWith('/pr-check.yml') ? read(path).replace(/^ if \(!Array\.isArray\(issue\.labels\).+\n throw new Error\('issue labels are malformed'\);\n }\n/m, '').replace(/^ if \(!Array\.isArray\(issue\.assignees\).+\n throw new Error\('issue assignees are malformed'\);\n }\n if \(issue\.assignees\.length === 0\) \{\n errors\.push\(.+\);\n }\n/m, '') : read(path); diff --git a/.github/scripts/windows-bash-temp-diagnostics.mjs b/.github/scripts/windows-bash-temp-diagnostics.mjs new file mode 100644 index 000000000..ece791dad --- /dev/null +++ b/.github/scripts/windows-bash-temp-diagnostics.mjs @@ -0,0 +1,98 @@ +import { spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const aliases = ['HOME', 'USERPROFILE', 'APPDATA', 'LOCALAPPDATA', 'TEMP', 'TMP', 'TMPDIR']; +const context = ['GITHUB_SHA', 'GITHUB_RUN_ID', 'GITHUB_RUN_ATTEMPT', 'RUNNER_OS', 'ImageVersion', 'ImageOS']; +const runtime = ['SYSTEMROOT', 'WINDIR', 'OS', 'COMSPEC', 'PATH', 'PATHEXT', 'MSYSTEM', + 'MSYS_NO_PATHCONV', 'MSYS2_ARG_CONV_EXCL', 'MSYS2_ENV_CONV_EXCL']; +const usage = 'Usage: windows-bash-temp-diagnostics.mjs snapshot FILE | collect BASELINE FILE\n'; +function select(env, names) { + const upper = Object.fromEntries(Object.entries(env).map(([key, value]) => [key.toUpperCase(), value])); + return Object.fromEntries(names.map(key => [key, upper[key.toUpperCase()] ?? null])); +} +const supplied = env => Object.fromEntries(Object.entries(env).filter(([, value]) => value !== null)); +export function snapshot(env = process.env) { + return { version: 1, aliases: select(env, aliases), context: select(env, context), node: process.version }; +} +function validate(baseline) { + if (baseline?.version !== 1 || !aliases.every(key => baseline.aliases?.[key] === null || typeof baseline.aliases?.[key] === 'string')) { + throw new Error('Invalid diagnostic baseline'); + } +} +const shell = [ + 'printf "BASH_VERSION=%s\\n" "$BASH_VERSION"', + 'printf "POSIX_TMP_WINDOWS="', + 'if "$DIAG_CYGPATH" -w /tmp; then printf "MAP_EXIT=0\\n"; else printf "MAP_EXIT=%s\\n" "$?"; fi', + 'if [ -d /tmp ]; then printf "POSIX_TMP_EXISTS=yes\\n"; else printf "POSIX_TMP_EXISTS=no\\n"; fi', + 'for key in TEMP TMP TMPDIR; do', + ' if [[ -v $key ]]; then', + ' printf "%s=%s\\n" "$key" "${!key}"', + ' if "$DIAG_CYGPATH" -u "${!key}"; then printf "%s_MAP_EXIT=0\\n" "$key"; else printf "%s_MAP_EXIT=%s\\n" "$key" "$?"; fi', + ' if [ -d "${!key}" ]; then printf "%s_EXISTS=yes\\n" "$key"; else printf "%s_EXISTS=no\\n" "$key"; fi', + ' else printf "%s=\\n" "$key"; fi', + 'done', +].join('\n'); + +// Called after the suite: never recreate a missing alias or mapped /tmp. +// execute is the process boundary; contract tests do not start Bash before CI's suite. +export function collect(baseline, parent, env = process.env, execute = spawnSync) { + validate(baseline); + const bash = env.CODEX_TEST_TRUSTED_BASH; + if (!bash || !path.isAbsolute(bash)) throw new Error('Absolute CODEX_TEST_TRUSTED_BASH required'); + const base = { ...supplied(select(env, runtime)), BASH_ENV: '', ENV: '', LC_ALL: 'C', + DIAG_CYGPATH: path.join(path.dirname(bash), 'cygpath.exe') }; + const original = { version: 1, aliases: select(baseline.aliases, aliases), + context: select(baseline.context ?? {}, context), node: baseline.node ?? null }; + const result = { version: 1, representativeOnly: true, goTempDirMeasured: false, + startupFilesDisabled: true, baseline: original, current: snapshot(env), bash, cases: [] }; + const observe = (name, environment) => { + const native = select(environment, ['TEMP', 'TMP', 'TMPDIR']); + const directories = Object.fromEntries(Object.entries(native).map(([key, value]) => [key, value === null ? null : existsSync(value)])); + const child = execute(bash, ['--noprofile', '--norc', '-c', shell], { + env: environment, encoding: 'utf8', timeout: 10000, windowsHide: true, + }); + result.cases.push({ name, environment, directories, at: new Date().toISOString(), + exit: child.status, signal: child.signal, error: child.error?.message ?? null, + stdout: child.stdout ?? '', stderr: child.stderr ?? '' }); + }; + try { + // Observe existing aliases before allocating any diagnostic profile. + observe('original-aliases', { ...base, ...supplied(original.aliases) }); + observe('suite-aliases', { ...base, ...supplied(select(env, aliases)) }); + result.scratch = mkdtempSync(path.join(parent, 'bash-temp-')); + const fixture = { ...base, PATH: '', MSYS_NO_PATHCONV: '1' }; + for (const [dir, keys] of Object.entries({ home: ['HOME', 'USERPROFILE'], + config: ['APPDATA', 'XDG_CONFIG_HOME', 'ENGRAM_CONFIG_DIR', 'CURL_HOME', 'CODEX_HOME', 'CLAUDE_CONFIG_DIR'], + data: ['LOCALAPPDATA', 'XDG_DATA_HOME', 'ENGRAM_DATA_DIR'], tmp: ['TEMP', 'TMP', 'TMPDIR'] })) { + const value = path.join(result.scratch, dir); + mkdirSync(value); + for (const key of keys) fixture[key] = value; + } + observe('fixture-like-no-gate', fixture); + } catch (error) { + result.setupError = error.message; + } finally { + if (result.scratch) rmSync(result.scratch, { recursive: true, force: true }); + result.scratchRemoved = !result.scratch || !existsSync(result.scratch); + } + return result; +} +function main(args) { + if (args.length === 1 && args[0] === '--help') { process.stdout.write(usage); return; } + if (!(args[0] === 'snapshot' && args.length === 2) && !(args[0] === 'collect' && args.length === 3)) { + process.stderr.write(usage); process.exitCode = 2; return; + } + try { + if (args[0] === 'snapshot') { + mkdirSync(path.dirname(args[1]), { recursive: true }); + writeFileSync(args[1], JSON.stringify(snapshot(), null, 2) + '\n'); + } else { + const result = collect(JSON.parse(readFileSync(args[1], 'utf8')), path.dirname(args[2])); + writeFileSync(args[2], JSON.stringify(result, null, 2) + '\n'); + if (result.setupError || result.cases.length !== 3 || result.cases.some(c => c.exit !== 0 || c.error) || !result.scratchRemoved) process.exitCode = 1; + } + } catch (error) { process.stderr.write(error.message + '\n'); process.exitCode = 1; } +} +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) main(process.argv.slice(2)); diff --git a/.github/scripts/windows-bash-temp-diagnostics.test.mjs b/.github/scripts/windows-bash-temp-diagnostics.test.mjs new file mode 100644 index 000000000..0ec0619d2 --- /dev/null +++ b/.github/scripts/windows-bash-temp-diagnostics.test.mjs @@ -0,0 +1,103 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import test from 'node:test'; +import { fileURLToPath } from 'node:url'; +const script = fileURLToPath(new URL('./windows-bash-temp-diagnostics.mjs', import.meta.url)); +const usage = 'Usage: windows-bash-temp-diagnostics.mjs snapshot FILE | collect BASELINE FILE\n'; +function fixture(t) { + const parent = path.resolve('odd'); + mkdirSync(parent, { recursive: true }); + const root = mkdtempSync(path.join(parent, 'temp-contract-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + return root; +} +const cli = (args, env = process.env) => spawnSync(process.execPath, [script, ...args], { env, encoding: 'utf8', timeout: 10000 }); + +test('snapshot CLI records only safe aliases/context, including unset values', t => { + const root = fixture(t), output = path.join(root, 'baseline.json'); + const result = cli(['snapshot', output], { SystemRoot: process.env.SystemRoot, HOME: root, TEMP: root, GITHUB_SHA: 'revision', GITHUB_TOKEN: 'never-record' }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, ''); assert.equal(result.stderr, ''); + const data = JSON.parse(readFileSync(output, 'utf8')); + assert.equal(data.version, 1); assert.equal(data.aliases.HOME, root); + assert.equal(data.aliases.TMPDIR, null); assert.equal(data.context.GITHUB_SHA, 'revision'); + assert.doesNotMatch(JSON.stringify(data), /never-record|GITHUB_TOKEN/); + assert.deepEqual(readdirSync(root), ['baseline.json']); +}); + +test('CLI help has exact stdout and no stderr', () => { + const result = cli(['--help']); + assert.equal(result.status, 0); assert.equal(result.stdout, usage); assert.equal(result.stderr, ''); +}); + +test('CLI rejects invalid arguments without altering existing output or creating scratch', t => { + const root = fixture(t), output = path.join(root, 'result.json'); + writeFileSync(output, 'sentinel'); + const result = cli(['unknown', output]); + assert.equal(result.status, 2); assert.equal(result.stdout, ''); assert.equal(result.stderr, usage); + assert.equal(readFileSync(output, 'utf8'), 'sentinel'); assert.deepEqual(readdirSync(root), ['result.json']); +}); + +test('representative matrix preserves every child exit and exact streams, environment and cleanup', async t => { + const { collect, snapshot } = await import('./windows-bash-temp-diagnostics.mjs'); + const root = fixture(t), calls = []; + const env = { SystemRoot: process.env.SystemRoot, TEMP: root, TMP: root, TMPDIR: root, CODEX_TEST_TRUSTED_BASH: script, SECRET_TOKEN: 'never-record' }; + const baseline = snapshot({ TEMP: 'original-temp' }); + baseline.aliases.BASH_ENV = 'never-execute'; baseline.aliases.SECRET_TOKEN = 'never-record'; + const result = collect(baseline, root, env, (exe, args, options) => { + calls.push({ exe, args, options }); + return { status: 31, signal: null, stdout: 'raw\r\nstdout\n', stderr: 'bash.exe: warning: could not find /tmp, please create!\r\n' }; + }); + assert.deepEqual(result.cases.map(c => c.name), ['original-aliases', 'suite-aliases', 'fixture-like-no-gate']); + assert.equal(result.representativeOnly, true); assert.equal(result.goTempDirMeasured, false); + assert.equal(calls.length, 3); + for (const [i, c] of result.cases.entries()) { + assert.equal(c.exit, 31); assert.equal(c.signal, null); assert.equal(c.error, null); + assert.equal(c.stdout, 'raw\r\nstdout\n'); assert.equal(c.stderr, 'bash.exe: warning: could not find /tmp, please create!\r\n'); + assert.equal(calls[i].options.timeout, 10000); + assert.deepEqual(calls[i].args.slice(0, 3), ['--noprofile', '--norc', '-c']); + assert.equal(calls[i].options.env.BASH_ENV, ''); assert.equal(calls[i].options.env.ENV, ''); + assert.doesNotMatch(JSON.stringify(c.environment), /SECRET_TOKEN|never-record/); + } + assert.doesNotMatch(JSON.stringify(result), /never-record|never-execute|SECRET_TOKEN/); + assert.equal(calls[0].options.env.TEMP, 'original-temp'); + assert.equal(Object.hasOwn(calls[0].options.env, 'TMPDIR'), false); + assert.equal(calls[1].options.env.TEMP, root); + assert.equal(calls[2].options.env.PATH, ''); assert.equal(calls[2].options.env.MSYS_NO_PATHCONV, '1'); + assert.equal(result.scratchRemoved, true); assert.equal(existsSync(result.scratch), false); + assert.deepEqual(readdirSync(root), []); +}); + +test('timeout retains partial stdout/stderr and cleans private scratch', async t => { + const { collect, snapshot } = await import('./windows-bash-temp-diagnostics.mjs'); + const root = fixture(t); + const result = collect(snapshot({}), root, { CODEX_TEST_TRUSTED_BASH: script }, () => ({ status: null, signal: 'SIGTERM', error: new Error('ETIMEDOUT'), stdout: 'partial', stderr: 'original error' })); + assert.equal(result.cases.length, 3); + for (const c of result.cases) { + assert.equal(c.exit, null); assert.equal(c.signal, 'SIGTERM'); assert.equal(c.error, 'ETIMEDOUT'); + assert.equal(c.stdout, 'partial'); assert.equal(c.stderr, 'original error'); + } + assert.equal(result.scratchRemoved, true); assert.deepEqual(readdirSync(root), []); +}); + +test('profile setup error retains earlier observations without repairing missing parents', async t => { + const { collect, snapshot } = await import('./windows-bash-temp-diagnostics.mjs'); + const root = fixture(t), missing = path.join(root, 'missing'); + const result = collect(snapshot({}), missing, { CODEX_TEST_TRUSTED_BASH: script }, () => ({ status: 0, signal: null, stdout: 'recorded', stderr: 'warning' })); + assert.equal(result.cases.length, 2); assert.match(result.setupError, /ENOENT/); + assert.equal(result.cases[0].stdout, 'recorded'); assert.equal(result.cases[1].stderr, 'warning'); + assert.equal(result.scratchRemoved, true); assert.equal(existsSync(missing), false); + assert.deepEqual(readdirSync(root), []); +}); + +test('CLI setup failure leaves existing output and directory contents unchanged', t => { + const root = fixture(t), baseline = path.join(root, 'invalid.json'), output = path.join(root, 'result.json'); + writeFileSync(baseline, '{}'); writeFileSync(output, 'sentinel'); + const result = cli(['collect', baseline, output]); + assert.equal(result.status, 1); assert.equal(result.stdout, ''); + assert.equal(result.stderr, 'Invalid diagnostic baseline\n'); + assert.equal(readFileSync(output, 'utf8'), 'sentinel'); + assert.deepEqual(readdirSync(root).sort(), ['invalid.json', 'result.json']); +}); diff --git a/.github/scripts/windows-suite.test.mjs b/.github/scripts/windows-suite.test.mjs index f5908418b..27300359a 100644 --- a/.github/scripts/windows-suite.test.mjs +++ b/.github/scripts/windows-suite.test.mjs @@ -10,7 +10,7 @@ function contract(text) { const body = job(text); assert.ok(body, 'independent Windows job'); assert.match(text, /on:\n pull_request:\n merge_group:\n types: \[checks_requested\]\n push:\n branches:\n - main\n/); - assert.match(text, /run: node --test \.github\/scripts\/windows-suite\.test\.mjs\n/); + assert.match(text, /run: node --test \.github\/scripts\/windows-suite\.test\.mjs \.github\/scripts\/windows-bash-temp-diagnostics\.test\.mjs\n/); for (const line of ['name: Windows Full Suite (Observational)', 'runs-on: windows-2025', 'timeout-minutes: 30', 'contents: read', 'GOTOOLCHAIN: local', 'ENGRAM_PI_SCHEMA_REQUIRE_REAL: "1"', 'shell: pwsh', @@ -48,10 +48,10 @@ function contract(text) { assert.match(body, new RegExp(`(?:^|[ ;])${key} = '(?:home|appdata|localappdata|data|tmp)'`, 'm'), `isolated ${key}`); } assert.doesNotMatch(body, /continue-on-error|needs:|: write|secrets\.|\|\| true| -p | -parallel| -run | -short| -skip| -tags| -timeout|GOFLAGS|GOOS:|GOARCH:/); - assert.equal((body.match(/\n\s+if:/g) ?? []).length, 1, 'only failure artifact guard'); + assert.equal((body.match(/\n\s+if:/g) ?? []).length, 3, 'failure evidence plus diagnostic collection/artifact guards'); assert.equal(body.split('Add-Content $env:GITHUB_ENV "$key=$value"').length - 1, 2, 'export both preserved caches and isolated test state'); - assert.ok(body.includes('run: node --test .github/scripts/windows-suite.test.mjs\n')); + assert.ok(body.includes('run: node --test .github/scripts/windows-suite.test.mjs .github/scripts/windows-bash-temp-diagnostics.test.mjs\n')); assert.ok(body.indexOf('run: node --test') < body.indexOf('npm ci')); assert.ok(body.indexOf('npm ci') < body.indexOf('& go test')); assert.ok(body.indexOf('Add-Content $env:GITHUB_ENV') < body.indexOf('npm ci')); @@ -59,6 +59,26 @@ function contract(text) { test('native observational full-module Windows contract', () => contract(source)); +function diagnostics(text) { + const body = job(text); + const snapshot = body.indexOf('windows-bash-temp-diagnostics.mjs snapshot'); + assert.ok(snapshot > 0 && snapshot < body.indexOf('$paths = @{'), 'snapshot before private exports'); + assert.match(body, /curl --version; printf "BASH_TEMP/); + const suite = body.indexOf('exit $suiteExit'); + const collection = body.indexOf('windows-bash-temp-diagnostics.mjs collect'); + assert.ok(collection > suite, 'collect after original suite'); + assert.match(body, /name: Collect representative Bash temp diagnostics\n\s+if: \$\{\{ !cancelled\(\) \}\}/); + assert.match(body, /name: Preserve Bash temp diagnostics\n\s+if: always\(\)/); + assert.ok(body.includes('path: ${{ runner.temp }}/windows-suite/diagnostics')); +} +test('bounded post-suite temp diagnostics preserve prerequisite ordering', () => diagnostics(source)); +test('reject missing diagnostic snapshot, streams and artifact wiring', () => { + for (const token of ['windows-bash-temp-diagnostics.mjs snapshot', 'windows-bash-temp-diagnostics.mjs collect', 'curl --version; printf "BASH_TEMP', 'path: ${{ runner.temp }}/windows-suite/diagnostics']) { + assert.ok(job(source).includes(token)); + assert.throws(() => diagnostics(source.replace(token, 'lost-diagnostic'))); + } +}); + test('installed Git entrypoints resolve to actual usr/bin Bash', { skip: process.platform !== 'win32' && 'requires native Windows PowerShell path semantics', }, t => { diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b56c64c3..45c2f0f8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -494,7 +494,7 @@ jobs: run: node --test .github/scripts/ci-scanners.test.mjs - name: Run Windows suite contract tests - run: node --test .github/scripts/windows-suite.test.mjs + run: node --test .github/scripts/windows-suite.test.mjs .github/scripts/windows-bash-temp-diagnostics.test.mjs - name: Run technical CI contract tests run: node --test .github/scripts/ci-technical-contracts.test.mjs @@ -754,7 +754,7 @@ jobs: node-version: "24" - name: Verify Windows suite contracts - run: node --test .github/scripts/windows-suite.test.mjs + run: node --test .github/scripts/windows-suite.test.mjs .github/scripts/windows-bash-temp-diagnostics.test.mjs - name: Verify native prerequisites and isolate test state run: | @@ -785,7 +785,7 @@ jobs: $bashDir = Split-Path $bash $env:PATH = "$bashDir;$env:PATH" Add-Content $env:GITHUB_PATH $bashDir - & $bash -lc 'set -euo pipefail; command -v bash jq curl; bash --version; jq --version; curl --version' + & $bash -lc 'set -euo pipefail; command -v bash jq curl; bash --version; jq --version; curl --version; printf "BASH_TEMP=%s\nBASH_TMP=%s\nBASH_TMPDIR=%s\n" "${TEMP-}" "${TMP-}" "${TMPDIR-}"; if cygpath -w /tmp; then printf "TMP_MAP_EXIT=0\n"; else printf "TMP_MAP_EXIT=%s\n" "$?"; fi; if [ -d /tmp ]; then printf "POSIX_TMP_EXISTS=yes\n"; else printf "POSIX_TMP_EXISTS=no\n"; fi' Get-Command bash, jq, curl # Existing Codex fixtures require explicit reviewed real executables. Add-Content $env:GITHUB_ENV "CODEX_TEST_TRUSTED_BASH=$bash" @@ -795,6 +795,7 @@ jobs: $value = go env $key Add-Content $env:GITHUB_ENV "$key=$value" } + node .github/scripts/windows-bash-temp-diagnostics.mjs snapshot "$root/diagnostics/baseline.json" $paths = @{ HOME = 'home'; USERPROFILE = 'home' APPDATA = 'appdata'; LOCALAPPDATA = 'localappdata' @@ -826,6 +827,19 @@ jobs: Get-Content "$evidence/stderr.txt" exit $suiteExit + - name: Collect representative Bash temp diagnostics + if: ${{ !cancelled() }} + timeout-minutes: 2 + run: node .github/scripts/windows-bash-temp-diagnostics.mjs collect "$env:RUNNER_TEMP/windows-suite/diagnostics/baseline.json" "$env:RUNNER_TEMP/windows-suite/diagnostics/result.json" + + - name: Preserve Bash temp diagnostics + if: always() + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 + with: + name: windows-bash-temp-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/windows-suite/diagnostics + if-no-files-found: warn + - name: Preserve failure evidence if: failure() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8b8187af0..4c5defec6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -221,7 +221,18 @@ Go caches remain usable. A dedicated hosted job and test-owned child lifecycle provide process isolation, not measured proof of cleanup. JSON, stderr, suite exit code and elapsed seconds are retained in logs, with -failure artifacts when available. Cancellation/outer timeout can leave partial +failure artifacts when available. The existing prerequisite Bash invocation also +records safe temp aliases and `/tmp` mapping/existence. After the unchanged suite, +a cancellation-aware, two-minute diagnostic step records three representative +Bash environments (original profile aliases, suite aliases, minimal fixture-like). +`windows-bash-temp--` artifacts retain the original safe-alias +snapshot and exact child exit/stdout/stderr, including errors; private diagnostic +scratch is removed. These are not the actual Go fixture children or Go +`os.TempDir()`, and do not establish root cause or comparable baseline/candidate +behavior. No startup files or fixture gate run in the representative matrix. +For local collection, run `node .github/scripts/windows-bash-temp-diagnostics.mjs --help` for snapshot/collect arguments; `CODEX_TEST_TRUSTED_BASH` must identify the +reviewed absolute Bash executable. Diagnostic errors never replace the recorded +suite exit or repair a missing temp directory. Cancellation/outer timeout can leave partial output without a completed exit/timing record. Wiring and structural tests do **not** complete #1891: hosted checked-out SHA, actual toolchain, exit, elapsed time and skip review are still missing. Optional-service and foreign-OS skips From 4692178c5ac1e670120d9e4bb9595f7d37edea7e Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Tue, 27 Oct 2026 17:56:43 -0500 Subject: [PATCH 2/4] fix(ci): normalize diagnostic executable separators --- .../scripts/windows-bash-temp-diagnostics.mjs | 2 +- .../windows-bash-temp-diagnostics.test.mjs | 25 +++++++++++++++++++ CONTRIBUTING.md | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/.github/scripts/windows-bash-temp-diagnostics.mjs b/.github/scripts/windows-bash-temp-diagnostics.mjs index ece791dad..c84b83aca 100644 --- a/.github/scripts/windows-bash-temp-diagnostics.mjs +++ b/.github/scripts/windows-bash-temp-diagnostics.mjs @@ -42,7 +42,7 @@ export function collect(baseline, parent, env = process.env, execute = spawnSync const bash = env.CODEX_TEST_TRUSTED_BASH; if (!bash || !path.isAbsolute(bash)) throw new Error('Absolute CODEX_TEST_TRUSTED_BASH required'); const base = { ...supplied(select(env, runtime)), BASH_ENV: '', ENV: '', LC_ALL: 'C', - DIAG_CYGPATH: path.join(path.dirname(bash), 'cygpath.exe') }; + DIAG_CYGPATH: path.join(path.dirname(bash), 'cygpath.exe').replaceAll('\\', '/') }; const original = { version: 1, aliases: select(baseline.aliases, aliases), context: select(baseline.context ?? {}, context), node: baseline.node ?? null }; const result = { version: 1, representativeOnly: true, goTempDirMeasured: false, diff --git a/.github/scripts/windows-bash-temp-diagnostics.test.mjs b/.github/scripts/windows-bash-temp-diagnostics.test.mjs index 0ec0619d2..d25397665 100644 --- a/.github/scripts/windows-bash-temp-diagnostics.test.mjs +++ b/.github/scripts/windows-bash-temp-diagnostics.test.mjs @@ -70,6 +70,31 @@ test('representative matrix preserves every child exit and exact streams, enviro assert.deepEqual(readdirSync(root), []); }); +test('Bash receives a slash-delimited sibling executable with spaces even when PATH is empty', async t => { + const { collect, snapshot } = await import('./windows-bash-temp-diagnostics.mjs'); + const root = fixture(t), calls = []; + const bash = path.join(root, 'Git With Spaces', 'usr', 'bin', 'bash.exe'); + const expected = path.join(path.dirname(bash), 'cygpath.exe').replaceAll('\\', '/'); + const result = collect(snapshot({}), root, { CODEX_TEST_TRUSTED_BASH: bash }, (exe, args, options) => { + calls.push({ exe, args, options }); + return { status: 0, signal: null, stdout: 'mapping observation\r\n', stderr: '' }; + }); + assert.equal(calls.length, 3); + for (const [i, call] of calls.entries()) { + assert.equal(call.exe, bash); + assert.equal(call.options.env.DIAG_CYGPATH, expected); + assert.doesNotMatch(call.options.env.DIAG_CYGPATH, /\\\\/); + assert.ok(call.options.env.DIAG_CYGPATH.includes('/Git With Spaces/')); + assert.ok(call.args[3].includes('"$DIAG_CYGPATH" -w /tmp')); + assert.equal(result.cases[i].environment.DIAG_CYGPATH, expected); + assert.equal(result.cases[i].exit, 0); + assert.equal(result.cases[i].stdout, 'mapping observation\r\n'); + assert.equal(result.cases[i].stderr, ''); + } + assert.equal(calls[2].options.env.PATH, ''); + assert.equal(result.scratchRemoved, true); assert.deepEqual(readdirSync(root), []); +}); + test('timeout retains partial stdout/stderr and cleans private scratch', async t => { const { collect, snapshot } = await import('./windows-bash-temp-diagnostics.mjs'); const root = fixture(t); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4c5defec6..76700917c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -230,6 +230,8 @@ snapshot and exact child exit/stdout/stderr, including errors; private diagnosti scratch is removed. These are not the actual Go fixture children or Go `os.TempDir()`, and do not establish root cause or comparable baseline/candidate behavior. No startup files or fixture gate run in the representative matrix. +The quoted sibling `cygpath.exe` path uses forward slashes, including with spaces +and an empty `PATH`, rather than depending on implicit Windows-path handling. For local collection, run `node .github/scripts/windows-bash-temp-diagnostics.mjs --help` for snapshot/collect arguments; `CODEX_TEST_TRUSTED_BASH` must identify the reviewed absolute Bash executable. Diagnostic errors never replace the recorded suite exit or repair a missing temp directory. Cancellation/outer timeout can leave partial From bb362187caf25de8f135be3b6244e61307d75b22 Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Fri, 30 Oct 2026 19:06:01 -0500 Subject: [PATCH 3/4] fix(ci): keep Bash diagnostics separate from suite outcome --- .../scripts/issue-refresh-router-workflow.test.mjs | 2 +- .github/scripts/windows-suite.test.mjs | 11 ++++++++++- .github/workflows/ci.yml | 1 + CONTRIBUTING.md | 6 ++++-- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/.github/scripts/issue-refresh-router-workflow.test.mjs b/.github/scripts/issue-refresh-router-workflow.test.mjs index 4b43cda85..b5f33ae58 100644 --- a/.github/scripts/issue-refresh-router-workflow.test.mjs +++ b/.github/scripts/issue-refresh-router-workflow.test.mjs @@ -80,7 +80,7 @@ test('workflow baselines retain readiness guards and pin original-event identity '.github/workflows/pr-check.yml': '33259c75e4f7dfdfca3143fc5efa0521a8ca289f06eca0d3efb429fbad7565bf', '.github/workflows/issue-validation.yml': 'd996ffae8e1341390b818e294a1a34a70f1b06f5eabda03cb96c0f065f7115b8', '.github/workflows/issue-policy.yml': '5f8bfbd9ab354f6b60d3ef35d30d5f4ee43a97eb83f61d8e0a955da37947adfd', - '.github/workflows/ci.yml': 'f4ec9bf5a14cfc02bcfd1cf28aba99b97fe071cb3af65e9bcfc2e5166086fa76', + '.github/workflows/ci.yml': '1c2acfa8957e68a798ed1192a4e196072ebcd0eed556d1c7ca3669a18b10a1de', }; for (const [path, expected] of Object.entries(baseline)) { const source = path.endsWith('/pr-check.yml') ? read(path).replace(/^ if \(!Array\.isArray\(issue\.labels\).+\n throw new Error\('issue labels are malformed'\);\n }\n/m, '').replace(/^ if \(!Array\.isArray\(issue\.assignees\).+\n throw new Error\('issue assignees are malformed'\);\n }\n if \(issue\.assignees\.length === 0\) \{\n errors\.push\(.+\);\n }\n/m, '') : read(path); diff --git a/.github/scripts/windows-suite.test.mjs b/.github/scripts/windows-suite.test.mjs index 27300359a..ab4abc083 100644 --- a/.github/scripts/windows-suite.test.mjs +++ b/.github/scripts/windows-suite.test.mjs @@ -47,7 +47,9 @@ function contract(text) { for (const key of ['HOME', 'USERPROFILE', 'APPDATA', 'LOCALAPPDATA', 'ENGRAM_DATA_DIR', 'TEMP', 'TMP', 'TMPDIR']) { assert.match(body, new RegExp(`(?:^|[ ;])${key} = '(?:home|appdata|localappdata|data|tmp)'`, 'm'), `isolated ${key}`); } - assert.doesNotMatch(body, /continue-on-error|needs:|: write|secrets\.|\|\| true| -p | -parallel| -run | -short| -skip| -tags| -timeout|GOFLAGS|GOOS:|GOARCH:/); + const collection = body.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; + const strictBody = body.replace(collection, collection.replace(' continue-on-error: true\n', '')); + assert.doesNotMatch(strictBody, /continue-on-error|needs:|: write|secrets\.|\|\| true| -p | -parallel| -run | -short| -skip| -tags| -timeout|GOFLAGS|GOOS:|GOARCH:/); assert.equal((body.match(/\n\s+if:/g) ?? []).length, 3, 'failure evidence plus diagnostic collection/artifact guards'); assert.equal(body.split('Add-Content $env:GITHUB_ENV "$key=$value"').length - 1, 2, 'export both preserved caches and isolated test state'); @@ -61,6 +63,8 @@ test('native observational full-module Windows contract', () => contract(source) function diagnostics(text) { const body = job(text); + const collectionStep = body.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; + assert.match(collectionStep, /^ continue-on-error: true$/m); const snapshot = body.indexOf('windows-bash-temp-diagnostics.mjs snapshot'); assert.ok(snapshot > 0 && snapshot < body.indexOf('$paths = @{'), 'snapshot before private exports'); assert.match(body, /curl --version; printf "BASH_TEMP/); @@ -72,6 +76,11 @@ function diagnostics(text) { assert.ok(body.includes('path: ${{ runner.temp }}/windows-suite/diagnostics')); } test('bounded post-suite temp diagnostics preserve prerequisite ordering', () => diagnostics(source)); +test('diagnostics must remain nonfatal while the Go suite still propagates failure', () => { + const collection = source.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)[0]; + assert.throws(() => diagnostics(source.replace(collection, collection.replace(' continue-on-error: true\n', '')))); + assert.throws(() => contract(source.replace(' - name: Run full default-tags module suite\n', ' - name: Run full default-tags module suite\n continue-on-error: true\n'))); +}); test('reject missing diagnostic snapshot, streams and artifact wiring', () => { for (const token of ['windows-bash-temp-diagnostics.mjs snapshot', 'windows-bash-temp-diagnostics.mjs collect', 'curl --version; printf "BASH_TEMP', 'path: ${{ runner.temp }}/windows-suite/diagnostics']) { assert.ok(job(source).includes(token)); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90f7d997b..1b8ba4afc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -902,6 +902,7 @@ jobs: - name: Collect representative Bash temp diagnostics if: ${{ !cancelled() }} + continue-on-error: true timeout-minutes: 2 run: node .github/scripts/windows-bash-temp-diagnostics.mjs collect "$env:RUNNER_TEMP/windows-suite/diagnostics/baseline.json" "$env:RUNNER_TEMP/windows-suite/diagnostics/result.json" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 76700917c..bfd658f8f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -233,8 +233,10 @@ behavior. No startup files or fixture gate run in the representative matrix. The quoted sibling `cygpath.exe` path uses forward slashes, including with spaces and an empty `PATH`, rather than depending on implicit Windows-path handling. For local collection, run `node .github/scripts/windows-bash-temp-diagnostics.mjs --help` for snapshot/collect arguments; `CODEX_TEST_TRUSTED_BASH` must identify the -reviewed absolute Bash executable. Diagnostic errors never replace the recorded -suite exit or repair a missing temp directory. Cancellation/outer timeout can leave partial +reviewed absolute Bash executable. Diagnostic failures remain visible on their +step and in the artifact; that step continues on error so the diagnostic cannot +change the job conclusion. It never replaces the recorded suite exit or repairs +a missing temp directory. Cancellation/outer timeout can leave partial output without a completed exit/timing record. Wiring and structural tests do **not** complete #1891: hosted checked-out SHA, actual toolchain, exit, elapsed time and skip review are still missing. Optional-service and foreign-OS skips From 5fdcd76c4e5110e1ac8c7de32b28a7bb90253536 Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Fri, 30 Oct 2026 19:46:00 -0500 Subject: [PATCH 4/4] ci(windows): keep diagnostic upload failures nonfatal --- .github/scripts/issue-refresh-router-workflow.test.mjs | 2 +- .github/scripts/windows-suite.test.mjs | 8 +++++++- .github/workflows/ci.yml | 1 + CONTRIBUTING.md | 7 ++++--- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/scripts/issue-refresh-router-workflow.test.mjs b/.github/scripts/issue-refresh-router-workflow.test.mjs index b5f33ae58..2674a4705 100644 --- a/.github/scripts/issue-refresh-router-workflow.test.mjs +++ b/.github/scripts/issue-refresh-router-workflow.test.mjs @@ -80,7 +80,7 @@ test('workflow baselines retain readiness guards and pin original-event identity '.github/workflows/pr-check.yml': '33259c75e4f7dfdfca3143fc5efa0521a8ca289f06eca0d3efb429fbad7565bf', '.github/workflows/issue-validation.yml': 'd996ffae8e1341390b818e294a1a34a70f1b06f5eabda03cb96c0f065f7115b8', '.github/workflows/issue-policy.yml': '5f8bfbd9ab354f6b60d3ef35d30d5f4ee43a97eb83f61d8e0a955da37947adfd', - '.github/workflows/ci.yml': '1c2acfa8957e68a798ed1192a4e196072ebcd0eed556d1c7ca3669a18b10a1de', + '.github/workflows/ci.yml': 'a355a3d0e4a0a1ef032f52b3f53f385357597cd6902e855c9526de3c9f726b7e', }; for (const [path, expected] of Object.entries(baseline)) { const source = path.endsWith('/pr-check.yml') ? read(path).replace(/^ if \(!Array\.isArray\(issue\.labels\).+\n throw new Error\('issue labels are malformed'\);\n }\n/m, '').replace(/^ if \(!Array\.isArray\(issue\.assignees\).+\n throw new Error\('issue assignees are malformed'\);\n }\n if \(issue\.assignees\.length === 0\) \{\n errors\.push\(.+\);\n }\n/m, '') : read(path); diff --git a/.github/scripts/windows-suite.test.mjs b/.github/scripts/windows-suite.test.mjs index ab4abc083..cea878492 100644 --- a/.github/scripts/windows-suite.test.mjs +++ b/.github/scripts/windows-suite.test.mjs @@ -48,7 +48,9 @@ function contract(text) { assert.match(body, new RegExp(`(?:^|[ ;])${key} = '(?:home|appdata|localappdata|data|tmp)'`, 'm'), `isolated ${key}`); } const collection = body.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; - const strictBody = body.replace(collection, collection.replace(' continue-on-error: true\n', '')); + const artifact = body.match(/ - name: Preserve Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; + const strictBody = body.replace(collection, collection.replace(' continue-on-error: true\n', '')) + .replace(artifact, artifact.replace(' continue-on-error: true\n', '')); assert.doesNotMatch(strictBody, /continue-on-error|needs:|: write|secrets\.|\|\| true| -p | -parallel| -run | -short| -skip| -tags| -timeout|GOFLAGS|GOOS:|GOARCH:/); assert.equal((body.match(/\n\s+if:/g) ?? []).length, 3, 'failure evidence plus diagnostic collection/artifact guards'); assert.equal(body.split('Add-Content $env:GITHUB_ENV "$key=$value"').length - 1, 2, @@ -65,6 +67,8 @@ function diagnostics(text) { const body = job(text); const collectionStep = body.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; assert.match(collectionStep, /^ continue-on-error: true$/m); + const artifactStep = body.match(/ - name: Preserve Bash temp diagnostics\n[\s\S]*?(?= - name:)/)?.[0] ?? ''; + assert.match(artifactStep, /^ continue-on-error: true$/m); const snapshot = body.indexOf('windows-bash-temp-diagnostics.mjs snapshot'); assert.ok(snapshot > 0 && snapshot < body.indexOf('$paths = @{'), 'snapshot before private exports'); assert.match(body, /curl --version; printf "BASH_TEMP/); @@ -79,6 +83,8 @@ test('bounded post-suite temp diagnostics preserve prerequisite ordering', () => test('diagnostics must remain nonfatal while the Go suite still propagates failure', () => { const collection = source.match(/ - name: Collect representative Bash temp diagnostics\n[\s\S]*?(?= - name:)/)[0]; assert.throws(() => diagnostics(source.replace(collection, collection.replace(' continue-on-error: true\n', '')))); + const artifact = source.match(/ - name: Preserve Bash temp diagnostics\n[\s\S]*?(?= - name:)/)[0]; + assert.throws(() => diagnostics(source.replace(artifact, artifact.replace(' continue-on-error: true\n', '')))); assert.throws(() => contract(source.replace(' - name: Run full default-tags module suite\n', ' - name: Run full default-tags module suite\n continue-on-error: true\n'))); }); test('reject missing diagnostic snapshot, streams and artifact wiring', () => { diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b8ba4afc..5f3006495 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -908,6 +908,7 @@ jobs: - name: Preserve Bash temp diagnostics if: always() + continue-on-error: true uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: windows-bash-temp-${{ github.run_id }}-${{ github.run_attempt }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bfd658f8f..d48d1058f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -233,9 +233,10 @@ behavior. No startup files or fixture gate run in the representative matrix. The quoted sibling `cygpath.exe` path uses forward slashes, including with spaces and an empty `PATH`, rather than depending on implicit Windows-path handling. For local collection, run `node .github/scripts/windows-bash-temp-diagnostics.mjs --help` for snapshot/collect arguments; `CODEX_TEST_TRUSTED_BASH` must identify the -reviewed absolute Bash executable. Diagnostic failures remain visible on their -step and in the artifact; that step continues on error so the diagnostic cannot -change the job conclusion. It never replaces the recorded suite exit or repairs +reviewed absolute Bash executable. Collection and artifact-upload failures remain +visible in their step outcomes and logs, with artifacts when upload succeeds. +Both diagnostic steps continue on error so neither changes the job conclusion. +The diagnostic never replaces the recorded suite exit or repairs a missing temp directory. Cancellation/outer timeout can leave partial output without a completed exit/timing record. Wiring and structural tests do **not** complete #1891: hosted checked-out SHA, actual toolchain, exit, elapsed