Skip to content

Commit 7ca488e

Browse files
prekshivyasgithub-actions[bot]cv
authored
test(e2e): strengthen fast coverage for live contracts (#10480)
<!-- markdownlint-disable MD041 --> ## Outcome Deterministic onboarding and messaging contracts that previously depended on live E2E runs now have fast integration or `e2e-support` coverage. Changes to live E2E tests and their helper modules must include a mapped, non-comment fast-test change. This PR does not remove, disable, or shorten any live E2E target. ## Reason Recent E2E-related merges exposed three gaps: the shared onboarding lifecycle mock modeled `runner.run` but not `runner.runCapture`; deterministic messaging state and proof parsing lacked fast regression coverage; and helper-only live E2E changes were outside the existing parity check. Review also found that the Hermes Slack credential scan crossed unsafe trust boundaries. ## Changes - Centralizes the onboarding fixture sandbox identity and keeps lifecycle state consistent across `runner.run` and `runner.runCapture`. A subprocess integration test protects the composed fixture contract. - Adds fast tests for persisted channel lifecycle state, Google Chat provider-egress proof parsing, Hermes Slack API proof parsing, messaging setup, sandbox identity, sparse checkout, and related E2E mappings. - Extends the parity manifest and checker so non-comment changes to a live test or any declared `liveSources` helper require a non-comment change to a mapped fast test. - Keeps raw Slack tokens on the host during file/log/process leak scans. The sandbox receives only token byte lengths and SHA-256 fingerprints through `openshell sandbox exec`; its scanner returns `LEAK`, `OK`, or `EMPTY` without receiving or returning credential values. - Adds behavior tests that execute fake OpenShell and SSH binaries, prove SSH is never invoked, verify no raw token enters the transport payload, exercise leak and clean fingerprint scans, and check transport failures propagate. - Describes Google Chat `401` and Slack authentication errors only as provider-egress evidence. These responses cannot prove which authorization value reached the provider; the controlled-capture messaging E2E remains the owner of actual Slack credential-rewrite proof. - Removes unapproved workflow/YAML source-shape tests. The repository source-shape budget is zero; no production workflow is changed by this PR. ## Verification Latest head `4c28a1b507273c054fba9299f4158217e2ca7f22` (the verified PR-specific fixes are unchanged from `3127999877fdfc0a75f548378e31db3557fdf6f2`; this head merges current `main`): - Focused `e2e-support` tests — 18/18 passed. - E2E parity integration tests — 15/15 passed. - Shared onboarding integration migration — 101 tests passed; the post-merge composed fixture contract also passed 2/2. - Codebase growth guardrails — 32/32 passed. - `npm run test:projects:check` — passed. - `npm run source-shape:check` — passed with zero source-shape cases. - `npm run checks:repository` — passed. - Real PR-diff E2E parity check — passed. - `npm run validate:pr` — passed, including TypeScript, secret scanning, E2E phase checks, formatting, lint, and repository policy gates. - Exact-head live E2E on merged head `4c28a1b` — 3/3 passed: Hermes lifecycle [run](https://github.com/NVIDIA/NemoClaw/actions/runs/33151543391), plus OpenClaw lifecycle and Slack isolation [run](https://github.com/NVIDIA/NemoClaw/actions/runs/33152685951). An initial Hermes attempt received an external provider HTTP 403 before sandbox creation; the bounded retry passed. ## Review notes - Sensitive-path context: this PR changes test fixtures and live E2E assertion helpers for onboarding, credentials, policies, sandboxes, and messaging. It does not change production source behavior. No sensitive-path review waiver is claimed. - Raw Slack credentials remain host-side during the scan; neither the OpenShell command arguments nor sandbox stdin contain credential values. - The deleted issue-9880 workflow parser must not be restored without a reviewed security or compatibility exception to the repository's zero source-shape budget. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
1 parent 29e79e7 commit 7ca488e

40 files changed

Lines changed: 2024 additions & 1181 deletions

scripts/checks/e2e-mock-parity.mts

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ import { fileURLToPath } from "node:url";
88

99
import ts from "typescript";
1010

11+
import { moduleTagDeclarations } from "../../tools/e2e/module-tags.mts";
12+
1113
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
1214
export const DEFAULT_PARITY_MANIFEST = "test/e2e/mock-parity.json";
1315

1416
export type MockParityEntry = {
1517
live: string;
18+
liveSources?: string[];
1619
fast?: string[];
1720
liveOnlyReason?: string;
1821
};
@@ -23,6 +26,7 @@ export type MockParityManifest = {
2326
};
2427

2528
const LIVE_TEST = /^test\/e2e\/live\/.+\.test\.ts$/u;
29+
const LIVE_HELPER = /^test\/e2e\/live\/(?!.*\.test\.ts$).+\.ts$/u;
2630
const FAST_TESTS = [
2731
/^src\/.+\.test\.ts$/u,
2832
/^nemoclaw\/src\/.+\.test\.ts$/u,
@@ -50,7 +54,10 @@ function sourceTokens(source: string): string {
5054
for (const child of children) visit(child);
5155
};
5256
visit(sourceFile);
53-
return JSON.stringify(tokens);
57+
return JSON.stringify({
58+
moduleTags: moduleTagDeclarations(source).map(({ tag }) => tag),
59+
tokens,
60+
});
5461
}
5562

5663
export function isMockParityRelevantSourceChange(
@@ -91,6 +98,7 @@ export function validateMockParity(options: {
9198
}
9299

93100
const entries = new Map<string, MockParityEntry>();
101+
const sourceOwners = new Map<string, MockParityEntry[]>();
94102
for (const entry of manifest.entries) {
95103
if (!entry || typeof entry !== "object" || typeof entry.live !== "string") {
96104
errors.push("mock parity entries must be objects with a live path");
@@ -106,6 +114,14 @@ export function validateMockParity(options: {
106114
}
107115
entries.set(entry.live, entry);
108116

117+
if (
118+
entry.liveSources !== undefined &&
119+
(!Array.isArray(entry.liveSources) ||
120+
entry.liveSources.some((file) => typeof file !== "string"))
121+
) {
122+
errors.push(`${entry.live}: liveSources must be an array of live E2E helper paths`);
123+
continue;
124+
}
109125
if (
110126
entry.fast !== undefined &&
111127
(!Array.isArray(entry.fast) || entry.fast.some((file) => typeof file !== "string"))
@@ -126,6 +142,18 @@ export function validateMockParity(options: {
126142
}
127143

128144
if (!fileExists(entry.live)) errors.push(`${entry.live}: live test does not exist`);
145+
for (const sourceFile of new Set(entry.liveSources ?? [])) {
146+
if (!isSafeRepoPath(sourceFile) || !LIVE_HELPER.test(sourceFile)) {
147+
errors.push(`${entry.live}: ${sourceFile} is not a test/e2e/live/**/*.ts helper file`);
148+
continue;
149+
}
150+
if (!fileExists(sourceFile)) {
151+
errors.push(`${entry.live}: live E2E helper does not exist: ${sourceFile}`);
152+
}
153+
const owners = sourceOwners.get(sourceFile) ?? [];
154+
owners.push(entry);
155+
sourceOwners.set(sourceFile, owners);
156+
}
129157
for (const fastFile of new Set(fast)) {
130158
if (!isFastPrTest(fastFile)) {
131159
errors.push(`${entry.live}: ${fastFile} is not collected by a fast PR test project`);
@@ -135,10 +163,41 @@ export function validateMockParity(options: {
135163
}
136164
}
137165

138-
for (const liveFile of [...new Set(changedFiles)].filter((file) => LIVE_TEST.test(file))) {
139-
if (!entries.has(liveFile)) {
166+
const changedFileSet = new Set(changedFiles);
167+
const requireChangedFastTest = (entry: MockParityEntry, changedSource: string): void => {
168+
const mappedFastTests = Array.isArray(entry.fast)
169+
? entry.fast.filter((fastFile): fastFile is string => typeof fastFile === "string")
170+
: [];
171+
if (
172+
mappedFastTests.length > 0 &&
173+
!mappedFastTests.some((fastFile) => changedFileSet.has(fastFile))
174+
) {
175+
errors.push(
176+
changedSource === entry.live
177+
? `${entry.live}: change at least one mapped fast PR test with the live E2E`
178+
: `${changedSource}: change at least one fast PR test mapped from ${entry.live}`,
179+
);
180+
}
181+
};
182+
183+
for (const liveFile of [...changedFileSet].filter((file) => LIVE_TEST.test(file))) {
184+
const entry = entries.get(liveFile);
185+
if (!entry) {
140186
errors.push(`${liveFile}: changed live E2E needs an entry in ${DEFAULT_PARITY_MANIFEST}`);
187+
continue;
188+
}
189+
requireChangedFastTest(entry, liveFile);
190+
}
191+
192+
for (const helperFile of [...changedFileSet].filter((file) => LIVE_HELPER.test(file))) {
193+
const owners = sourceOwners.get(helperFile) ?? [];
194+
if (owners.length === 0) {
195+
errors.push(
196+
`${helperFile}: changed live E2E helper needs an owning entry in ${DEFAULT_PARITY_MANIFEST}`,
197+
);
198+
continue;
141199
}
200+
for (const owner of owners) requireChangedFastTest(owner, helperFile);
142201
}
143202

144203
return errors.sort();
@@ -161,6 +220,18 @@ function sourceAtRef(ref: string, file: string): string | null {
161220
}
162221
}
163222

223+
/** Remove metadata-only live and fast test changes before parity validation. */
224+
export function filterMockParityRelevantChangedFiles(
225+
files: readonly string[],
226+
sourceAtBase: (file: string) => string | null,
227+
sourceAtHead: (file: string) => string | null,
228+
): string[] {
229+
return files.filter((file) => {
230+
if (!LIVE_TEST.test(file) && !LIVE_HELPER.test(file) && !isFastPrTest(file)) return true;
231+
return isMockParityRelevantSourceChange(sourceAtBase(file), sourceAtHead(file));
232+
});
233+
}
234+
164235
function changedFiles(base: string, head: string): string[] {
165236
const files = execFileSync(
166237
"git",
@@ -172,10 +243,10 @@ function changedFiles(base: string, head: string): string[] {
172243
)
173244
.split(/\r?\n/u)
174245
.filter(Boolean);
175-
return files.filter(
176-
(file) =>
177-
!LIVE_TEST.test(file) ||
178-
isMockParityRelevantSourceChange(sourceAtRef(base, file), sourceAtRef(head, file)),
246+
return filterMockParityRelevantChangedFiles(
247+
files,
248+
(file) => sourceAtRef(base, file),
249+
(file) => sourceAtRef(head, file),
179250
);
180251
}
181252

src/lib/actions/sandbox/policy-channel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ function withSandboxMutationLockUnlessPreview<T>(
151151
* Internal composition dependencies for channel mutation.
152152
*
153153
* The Google Chat capability is intentionally absent from the public CLI
154-
* composition. The live E2E entrypoint supplies it directly so environment
154+
* composition. The channels stop/start live E2E helper supplies it directly so environment
155155
* variables and predictable sandbox names cannot enable non-interactive
156156
* audience enrollment in ordinary production execution.
157157
*/

0 commit comments

Comments
 (0)