test(e2e): hermeticize DCode fresh re-onboard inference#6530
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe fresh re-onboard e2e check now starts hermetic compatible inference, uses safer payload quoting, and no longer requires a hosted-inference API key. The runner conditionally clears the API key for this check, and tests cover the new execution path and API key handling. ChangesHermetic fresh re-onboard flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Script as 04-deepagents-code-fresh-reonboard.sh
participant Hermetic as hermetic-compatible-inference.sh
Script->>Hermetic: source helper
Script->>Hermetic: start compatible inference
Script->>Hermetic: stop compatible inference on EXIT
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/support/platform-parity-cloud-experimental.test.ts (1)
78-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the behavioral proof over source-text assertions.
This case asserts on script text (
toContain/not.toContain), while the following test at Lines 89-211 already runs the script under a hostile hosted-inference env and proves the hosted path is unreachable. Consider relying on that observable contract and dropping/trimming these string guards, which are brittle to harmless refactors.As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions" and "Migration tests must prove the superseded path is unreachable or removed."
🤖 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/e2e/support/platform-parity-cloud-experimental.test.ts` around lines 78 - 87, The test around the fresh re-onboard check is relying on brittle source-text assertions instead of the stronger behavioral proof already covered by the hostile hosted-inference execution path. Update the `keeps the destructive fresh re-onboard check on hermetic compatible inference` test in `platform-parity-cloud-experimental.test.ts` to prefer the observable contract from the script execution case and remove or trim the `toContain`/`not.toContain` guards on the script contents, keeping the focus on proving the hosted inference path is unreachable.Source: Path instructions
🤖 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.
Inline comments:
In `@test/e2e/support/platform-parity-cloud-experimental.test.ts`:
- Around line 188-211: The test’s spawnSync child timeout is longer than the
enclosing Vitest timeout, so the worker can hang past the intended budget;
update the timeout settings in the platform-parity-cloud-experimental test so
the spawnSync limit and the test’s outer timeout use the same value. Keep the
change localized to the test case that launches the CLI via spawnSync and
preserves the existing assertions.
---
Nitpick comments:
In `@test/e2e/support/platform-parity-cloud-experimental.test.ts`:
- Around line 78-87: The test around the fresh re-onboard check is relying on
brittle source-text assertions instead of the stronger behavioral proof already
covered by the hostile hosted-inference execution path. Update the `keeps the
destructive fresh re-onboard check on hermetic compatible inference` test in
`platform-parity-cloud-experimental.test.ts` to prefer the observable contract
from the script execution case and remove or trim the
`toContain`/`not.toContain` guards on the script contents, keeping the focus on
proving the hosted inference path is unreachable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: da9651d6-a69e-421f-8456-373d84b071ac
📒 Files selected for processing (3)
test/e2e/e2e-cloud-experimental/checks/04-deepagents-code-fresh-reonboard.shtest/e2e/live/cloud-experimental-checks.tstest/e2e/support/platform-parity-cloud-experimental.test.ts
Signed-off-by: Ho Lim <subhoya@gmail.com>
ce9ebcd to
4b41edc
Compare
|
Current The safe composition is to retain the credential canary and all #6567 config probes; retain the hermetic helper, cleanup, start, and usage assertion; shell-quote both the model and canary seed arguments; update the final count to 12 passes; and extend the fake CLI/OpenShell fixture to implement sanitized JSON/YAML/key reads, rejected mutation guidance/status, stable hashes, and the post-re-onboard model-B read. Without that fixture work, simply accepting both sides leaves the behavioral test failing. Focused validation should include The PR description also needs this exact standalone DCO declaration (the current inline mention does not satisfy the body check):
No history rewrite or force-push is needed. |
|
Superseded by #6581, which preserves this hermetic re-onboard change on a clean GitHub-verified, DCO-compliant commit and credits Ho Lim as co-author. The replacement is based on current main and retains the newer credential-canary, config-read, mutation-boundary, and safe shell-quoting coverage that conflicts with this branch. |
Summary
Validation
DCO: Verified commit ce9ebcd contains Signed-off-by: Ho Lim subhoya@gmail.com.
Summary by CodeRabbit
Bug Fixes
Tests