Skip to content

test(dcode): split image contracts by boundary - #6493

Merged
cv merged 4 commits into
mainfrom
codex/split-deepagents-image-test
Jul 8, 2026
Merged

test(dcode): split image contracts by boundary#6493
cv merged 4 commits into
mainfrom
codex/split-deepagents-image-test

Conversation

@cv

@cv cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Splits the oversized Deep Agents Code image contract test into focused image and credential-boundary suites, with shared wrapper fixtures. This extracts the test-only split from #6431 so it can land independently and restores the 1,500-line budget without changing assertions or adding an exception.

Changes

  • Move all 57 credential-boundary tests byte-for-byte into test/langchain-deepagents-code-image-credentials.test.ts, leaving 18 image contracts in the original suite.
  • Extract shared Deep Agents Code file and wrapper fixtures into test/helpers/langchain-deepagents-code-image.ts.
  • Update source comments that identify the canonical secret-corpus regression test.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: this structural split preserves all 75 existing test cases and assertions; both focused suites pass together.
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: no product behavior changed, and documentation review confirmed the existing SECURITY.md reference remains accurate.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: credential assertions moved byte-for-byte, source changes are comment-only, and the focused 75-test integration run passed.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project integration test/langchain-deepagents-code-image.test.ts test/langchain-deepagents-code-image-credentials.test.ts (2 files, 75 tests passed); npm run test-size:check, npm run test:projects:check, npm run test:titles:check, and npm run source-shape:check passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Tests
    • Added shared integration-test utilities to create temporary wrapper fixtures and execute them in a controlled environment.
    • Introduced a dedicated credential boundary test suite to verify secret-shape rejection, strict placeholder matching, token-format exceptions, auth/credential store fail-closed behavior, and disallowed mutation/command inputs.
    • Updated the existing image test suite to reuse the shared helpers and remove redundant scenarios.
  • Documentation
    • Updated a SECURITY/guard comment to reference the correct regression test corpus for secret-pattern parity.

Extract credential-boundary coverage and shared wrapper fixtures so each
focused suite stays below the repository test-file ceiling. Update source
comments to follow the moved canonical secret corpus.

Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4efc31ab-b9a8-473b-879d-1d5f02320a35

📥 Commits

Reviewing files that changed from the base of the PR and between 64ceea9 and 6b140db.

📒 Files selected for processing (1)
  • test/helpers/langchain-deepagents-code-image.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/helpers/langchain-deepagents-code-image.ts

📝 Walkthrough

Walkthrough

This PR extracts shared wrapper test helpers, adds a dedicated credential-boundary test suite, trims the original test file, and updates comments to reference the renamed test file.

Changes

Test helper extraction and credential test suite split

Layer / File(s) Summary
Shared wrapper/agent test helper module
test/helpers/langchain-deepagents-code-image.ts
New helper module adds readAgentFile, stubManagedMcpValidator, makeWrapperFixture, makeNetworkSimulatingFixture, and runWrapper for building temp wrapper scripts, fixtures, and running them via spawnSync.
New credential-boundary test suite
test/langchain-deepagents-code-image-credentials.test.ts
Adds a Vitest suite covering placeholder matching, messaging token exceptions, Slack/Telegram/Discord token rejection, auth.json/chatgpt-auth.json handling, upstream command restrictions, network log suppression, and canonical positive secret corpus parity using the shared helper module.
Original test file updated and trimmed
test/langchain-deepagents-code-image.test.ts
Replaces locally defined helpers with imports from the new helper module, narrows node:child_process imports, and removes the large block of tests now moved to the dedicated credentials test file.
Regression-gate comment updates
agents/langchain-deepagents-code/dcode-wrapper.sh, agents/langchain-deepagents-code/managed-dcode-runtime.py
Updates comments referencing the security-guard/regression-gate test corpus consumer to point to the new langchain-deepagents-code-image-credentials.test.ts file.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: refactor

Suggested reviewers: jyaunches

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: splitting the Deep Agents Code image tests by boundary into focused suites.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/split-deepagents-image-test

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the codex/split-deepagen... branch remains at 76%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 372f31b codex/split-deepagen... e46642f +/-
src/lib/messagi...ate-resolver.ts 88% 71% -17%
src/lib/state/config-io.ts 93% 88% -5%
src/lib/runner.ts 80% 75% -5%
src/lib/agent/onboard.ts 71% 70% -1%
src/lib/adapters/http/probe.ts 89% 91% +2%
src/lib/agent/base-image.ts 82% 84% +2%
src/lib/agent/t...ersion-drift.ts 88% 96% +8%
src/lib/agent/b...availability.ts 63% 74% +11%
src/lib/onboard/config-sync.ts 31% 46% +15%
src/lib/sandbox...rsion-scheme.ts 73% 100% +27%

Updated July 08, 2026 20:39 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is required. This PR is a test refactor plus comment-only updates in Deep Agents Code runtime files; it does not alter executable runtime behavior or user-facing flows. The relevant confidence should come from the affected unit/Vitest files, especially the newly split langchain-deepagents-code image credential-boundary tests.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • None. No E2E target dispatch is required: the agent runtime file changes are comment-only/test-reference updates, and the remaining changes are non-E2E unit test/helper refactors outside test/e2e. These changes do not affect E2E target registry, workflow wiring, fixtures, live support, or live target behavior.

Optional E2E targets

  • None.

Relevant changed files

  • None.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/helpers/langchain-deepagents-code-image.ts (1)

28-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated fixture-building logic between makeWrapperFixture and makeNetworkSimulatingFixture.

Both functions repeat the same read/patch/write/chmod sequence for the env-file substitution and wrapper file materialization. Consider factoring the shared prelude (reading + DEEPAGENTS_ENV_FILE substitution + write/chmod) into a small private helper to reduce duplication.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/helpers/langchain-deepagents-code-image.ts` around lines 28 - 88, The
fixture setup in makeWrapperFixture and makeNetworkSimulatingFixture duplicates
the same read/patch/write/chmod flow for dcode-wrapper.sh and the
DEEPAGENTS_ENV_FILE replacement. Extract that shared materialization logic into
a small private helper that takes the target env path and wrapper content
transformation, then have both functions call it and keep only their unique
substitutions (auth files, ran marker, network log, and deepagents_code exec
replacement) in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/helpers/langchain-deepagents-code-image.ts`:
- Around line 28-88: The fixture setup in makeWrapperFixture and
makeNetworkSimulatingFixture duplicates the same read/patch/write/chmod flow for
dcode-wrapper.sh and the DEEPAGENTS_ENV_FILE replacement. Extract that shared
materialization logic into a small private helper that takes the target env path
and wrapper content transformation, then have both functions call it and keep
only their unique substitutions (auth files, ran marker, network log, and
deepagents_code exec replacement) in place.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3307e5a8-c365-4df6-8362-607062fc3a4f

📥 Commits

Reviewing files that changed from the base of the PR and between 5ddf9a1 and 6cf3aee.

📒 Files selected for processing (5)
  • agents/langchain-deepagents-code/dcode-wrapper.sh
  • agents/langchain-deepagents-code/managed-dcode-runtime.py
  • test/helpers/langchain-deepagents-code-image.ts
  • test/langchain-deepagents-code-image-credentials.test.ts
  • test/langchain-deepagents-code-image.test.ts

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: agents/langchain-deepagents-code/dcode-wrapper.sh, agents/langchain-deepagents-code/managed-dcode-runtime.py. This is a pre-existing property of those files, not introduced by this PR (tracked as PRA-T1 from prior review). Current PR adds no new test gaps — 75 focused integration tests pass.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@cv cv mentioned this pull request Jul 8, 2026
21 tasks
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review follow-up: the CodeRabbit fixture-helper duplication nit is resolved in 64ceea9. PRA-T1 justification: there is no linked issue or acceptance clause for this mechanical test split; behavior parity is covered by the unchanged 75-test focused suite, which passes locally. No separate test follow-up is needed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
test/helpers/langchain-deepagents-code-image.ts (2)

70-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate exec short-circuit target string.

"exec /opt/venv/bin/python3 -I -m deepagents_code" is hard-coded identically in both makeWrapperFixture and makeNetworkSimulatingFixture. Extracting it as a shared constant would prevent the two occurrences from silently drifting apart if the wrapper script changes.

Also applies to: 86-87

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/helpers/langchain-deepagents-code-image.ts` at line 70, The wrapper
script target string is duplicated in makeWrapperFixture and
makeNetworkSimulatingFixture, which risks the two fixtures drifting apart.
Extract the hard-coded exec command into a shared constant in
test/helpers/langchain-deepagents-code-image.ts and have both fixture builders
reference that symbol so there is a single source of truth for the short-circuit
target.

28-91: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Unverified .replace() calls can silently produce broken fixtures.

materializeWrapperFixture, makeWrapperFixture, and makeNetworkSimulatingFixture all rely on chained .replace() calls against literal substrings from dcode-wrapper.sh. Unlike stubManagedMcpValidator (which asserts its target string is absent), none of these substitutions verify a match actually occurred. If the wrapper script's quoting/variable naming ever drifts, these replacements become silent no-ops — fixtures would then point at real /sandbox/... paths instead of temp paths, and the exec short-circuit wouldn't fire. Since this helper is now shared across two large test suites, a silent break here has outsized blast radius and could mask real regressions or cause confusing failures.

Consider asserting the source actually changed after each .replace(), mirroring the existing stubManagedMcpValidator pattern.

♻️ Example approach
+function replaceOrThrow(source: string, target: string, replacement: string): string {
+  const updated = source.replace(target, replacement);
+  expect(updated).not.toBe(source);
+  return updated;
+}

Then use replaceOrThrow(source, target, replacement) in place of source.replace(target, replacement) at each call site.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/helpers/langchain-deepagents-code-image.ts` around lines 28 - 91, The
fixture builders in materializeWrapperFixture, makeWrapperFixture, and
makeNetworkSimulatingFixture rely on chained string replacements that can fail
silently if dcode-wrapper.sh changes. Update these substitutions to verify each
target was actually replaced, using a helper like replaceOrThrow or an assertion
after each replace, similar to stubManagedMcpValidator. Ensure the
DEEPAGENTS_ENV_FILE, DEEPAGENTS_AUTH_FILE, DEEPAGENTS_CODEX_AUTH_FILE, python3
invocation, and deepagents_code exec short-circuit replacements all fail fast
when the source text no longer matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/helpers/langchain-deepagents-code-image.ts`:
- Line 70: The wrapper script target string is duplicated in makeWrapperFixture
and makeNetworkSimulatingFixture, which risks the two fixtures drifting apart.
Extract the hard-coded exec command into a shared constant in
test/helpers/langchain-deepagents-code-image.ts and have both fixture builders
reference that symbol so there is a single source of truth for the short-circuit
target.
- Around line 28-91: The fixture builders in materializeWrapperFixture,
makeWrapperFixture, and makeNetworkSimulatingFixture rely on chained string
replacements that can fail silently if dcode-wrapper.sh changes. Update these
substitutions to verify each target was actually replaced, using a helper like
replaceOrThrow or an assertion after each replace, similar to
stubManagedMcpValidator. Ensure the DEEPAGENTS_ENV_FILE, DEEPAGENTS_AUTH_FILE,
DEEPAGENTS_CODEX_AUTH_FILE, python3 invocation, and deepagents_code exec
short-circuit replacements all fail fast when the source text no longer matches.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 908fbb93-52bf-4b6d-b04a-80cf567725b7

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf3aee and 64ceea9.

📒 Files selected for processing (1)
  • test/helpers/langchain-deepagents-code-image.ts

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review follow-up: both latest CodeRabbit nits are resolved in 6b140db. The Deep Agents Code exec target is centralized, and every fixture substitution now requires exactly one source match so wrapper drift fails fast. The focused suite remains 75/75.

@cv
cv requested a review from cjagwani July 8, 2026 20:29
@cv

cv commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

PRA-T1 runtime-validation justification: the only changes under agents/langchain-deepagents-code are comment-only test-reference updates; executable runtime behavior is unchanged. The split preserves all original assertions, and the focused integration suites pass 75/75. The E2E advisor also reports that no runtime or live-flow validation is required, so no additional test follow-up is needed for this PR.

@cv
cv merged commit 3fa591a into main Jul 8, 2026
41 checks passed
@cv
cv deleted the codex/split-deepagents-image-test branch July 8, 2026 20:43
cv added a commit that referenced this pull request Jul 9, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Adds a shared deterministic regression risk plan for the PR head commit
and changed-file set to PR Review Advisor and E2E Advisor. Known
high-risk runtime surfaces now restore a reviewed floor of required E2E
jobs even when a model omits or downgrades them.

Execution is intentionally separated from plan generation: stacked
follow-up #6447 consumes this contract in a trusted post-merge shadow
gate without exposing repository secrets to unmerged code.

PR Review Advisor analysis is split into seven bounded, evidence-focused
turns with completion logs and per-turn response artifacts, improving
review visibility and traceability while preserving one read-only
session and the existing final JSON/comment contract.

The seven stages now live in one ordered array. Stage names, evidence,
prompt text, runtime numbering, and artifact ordering derive from that
single declaration instead of parallel hard-coded topology.

## Changes

- Add a stable risk-plan contract with lifecycle, upgrade, agent,
inference, messaging, platform, credential, and security invariants.
- Keep the canonical cloud-onboard live target in the platform-install
floor through an explicit test-file allowlist, while ordinary tests
remain excluded.
- Feed the plan into PR Review Advisor scope/risk, security/trust, and
tests/regressions contexts and both E2E Advisor normalizers.\n-
Deduplicate optional recommendations against deterministic required jobs
by both selector ID and workflow job.
- Clarify commit references in advisor, E2E, and security-review
documentation.
- Emit `risk-plan.json` from both E2E Advisor lanes with a plan hash,
required jobs, capped automatic candidates, and manual-expansion state;
PR Review Advisor exposes the same plan through its scoped context
artifacts.
- Replace four uneven advisor turns with seven bounded stages:
scope/risk, correctness/state, security/trust, tests/regressions,
CI/operations, reconciliation, and final synthesis.
- Derive turn numbering and prompt-artifact ordering from the ordered
stage declaration, and remove five duplicate standalone context JSONs
while retaining the same evidence in per-stage synthetic-tool artifacts.
- Log every turn boundary and persist each settled response immediately,
including partial output and failure/timeout status, while treating
missing trace artifacts as fatal.
- Leave live enforcement to stacked follow-up #6447 so this PR remains a
reviewable deterministic planning primitive.
- Remove unused PR-advisor check-wait configuration and align
contributor-facing documentation with actual workflow behavior.
- Add focused normalization, prompt, risk-mapping, workflow-inventory,
and review-depth regression tests.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: Internal CI/advisor behavior
only; contributor-facing tool and E2E documentation was updated, with no
Fern user documentation required.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Automated and local
security review confirmed the deterministic floor is additive, bound to
the PR head commit and changed-file set, capped, and does not itself
execute secret-bearing live E2E work; enforcement is isolated in #6447.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — 112 focused risk-plan, PR-advisor,
E2E-advisor, workflow-boundary, and maintainer-policy tests passed; the
E2E-advisor suite was rerun after the final normalization fix (7/7); CLI
typechecking, Biome, test-size, and Vitest project-membership checks
passed
- [ ] Applicable broad gate passed — CI for PR head commit `9700a6ba`
has 39 successful checks and 2 expected skips. `static-checks` fails
only because merged #6441 left
`test/langchain-deepagents-code-image.test.ts` at 1501 lines against the
1500-line mainline ceiling; aggregate `checks` mirrors that failure. The
repair is tracked in #6493.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Splits the oversized Deep Agents Code image contract test into focused
image and credential-boundary suites, with shared wrapper fixtures. This
extracts the test-only split from NVIDIA#6431 so it can land independently and
restores the 1,500-line budget without changing assertions or adding an
exception.

## Changes
- Move all 57 credential-boundary tests byte-for-byte into
`test/langchain-deepagents-code-image-credentials.test.ts`, leaving 18
image contracts in the original suite.
- Extract shared Deep Agents Code file and wrapper fixtures into
`test/helpers/langchain-deepagents-code-image.ts`.
- Update source comments that identify the canonical secret-corpus
regression test.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: this
structural split preserves all 75 existing test cases and assertions;
both focused suites pass together.
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: no product behavior changed,
and documentation review confirmed the existing `SECURITY.md` reference
remains accurate.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: credential assertions
moved byte-for-byte, source changes are comment-only, and the focused
75-test integration run passed.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run --project integration
test/langchain-deepagents-code-image.test.ts
test/langchain-deepagents-code-image-credentials.test.ts` (2 files, 75
tests passed); `npm run test-size:check`, `npm run test:projects:check`,
`npm run test:titles:check`, and `npm run source-shape:check` passed.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added shared integration-test utilities to create temporary wrapper
fixtures and execute them in a controlled environment.
* Introduced a dedicated credential boundary test suite to verify
secret-shape rejection, strict placeholder matching, token-format
exceptions, auth/credential store fail-closed behavior, and disallowed
mutation/command inputs.
* Updated the existing image test suite to reuse the shared helpers and
remove redundant scenarios.
* **Documentation**
* Updated a SECURITY/guard comment to reference the correct regression
test corpus for secret-pattern parity.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Adds a shared deterministic regression risk plan for the PR head commit
and changed-file set to PR Review Advisor and E2E Advisor. Known
high-risk runtime surfaces now restore a reviewed floor of required E2E
jobs even when a model omits or downgrades them.

Execution is intentionally separated from plan generation: stacked
follow-up NVIDIA#6447 consumes this contract in a trusted post-merge shadow
gate without exposing repository secrets to unmerged code.

PR Review Advisor analysis is split into seven bounded, evidence-focused
turns with completion logs and per-turn response artifacts, improving
review visibility and traceability while preserving one read-only
session and the existing final JSON/comment contract.

The seven stages now live in one ordered array. Stage names, evidence,
prompt text, runtime numbering, and artifact ordering derive from that
single declaration instead of parallel hard-coded topology.

## Changes

- Add a stable risk-plan contract with lifecycle, upgrade, agent,
inference, messaging, platform, credential, and security invariants.
- Keep the canonical cloud-onboard live target in the platform-install
floor through an explicit test-file allowlist, while ordinary tests
remain excluded.
- Feed the plan into PR Review Advisor scope/risk, security/trust, and
tests/regressions contexts and both E2E Advisor normalizers.\n-
Deduplicate optional recommendations against deterministic required jobs
by both selector ID and workflow job.
- Clarify commit references in advisor, E2E, and security-review
documentation.
- Emit `risk-plan.json` from both E2E Advisor lanes with a plan hash,
required jobs, capped automatic candidates, and manual-expansion state;
PR Review Advisor exposes the same plan through its scoped context
artifacts.
- Replace four uneven advisor turns with seven bounded stages:
scope/risk, correctness/state, security/trust, tests/regressions,
CI/operations, reconciliation, and final synthesis.
- Derive turn numbering and prompt-artifact ordering from the ordered
stage declaration, and remove five duplicate standalone context JSONs
while retaining the same evidence in per-stage synthetic-tool artifacts.
- Log every turn boundary and persist each settled response immediately,
including partial output and failure/timeout status, while treating
missing trace artifacts as fatal.
- Leave live enforcement to stacked follow-up NVIDIA#6447 so this PR remains a
reviewable deterministic planning primitive.
- Remove unused PR-advisor check-wait configuration and align
contributor-facing documentation with actual workflow behavior.
- Add focused normalization, prompt, risk-mapping, workflow-inventory,
and review-depth regression tests.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: Internal CI/advisor behavior
only; contributor-facing tool and E2E documentation was updated, with no
Fern user documentation required.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Automated and local
security review confirmed the deterministic floor is additive, bound to
the PR head commit and changed-file set, capped, and does not itself
execute secret-bearing live E2E work; enforcement is isolated in NVIDIA#6447.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — 112 focused risk-plan, PR-advisor,
E2E-advisor, workflow-boundary, and maintainer-policy tests passed; the
E2E-advisor suite was rerun after the final normalization fix (7/7); CLI
typechecking, Biome, test-size, and Vitest project-membership checks
passed
- [ ] Applicable broad gate passed — CI for PR head commit `9700a6ba`
has 39 successful checks and 2 expected skips. `static-checks` fails
only because merged NVIDIA#6441 left
`test/langchain-deepagents-code-image.test.ts` at 1501 lines against the
1500-line mainline ceiling; aggregate `checks` mirrors that failure. The
repair is tracked in NVIDIA#6493.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: dcode LangChain Deep Code integration behavior refactor PR restructures code without intended behavior change labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: dcode LangChain Deep Code integration behavior refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants