Skip to content

fix(messaging): compose OpenClaw runtime loaders - #6474

Merged
apurvvkumaria merged 6 commits into
mainfrom
fix/6467-openclaw-slack-runtime-proof
Jul 8, 2026
Merged

fix(messaging): compose OpenClaw runtime loaders#6474
apurvvkumaria merged 6 commits into
mainfrom
fix/6467-openclaw-slack-runtime-proof

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replaces the WhatsApp compact-QR preload's asynchronous Node loader with a synchronous hook so it composes safely with the Slack runtime guard. It also makes an empty installed-runtime proof fail with already-redacted loader diagnostics instead of hiding the cause. Reported by @ericksoa.

Related Issue

Fixes #6467.

Changes

  • Register the reviewed WhatsApp QR source rewrite through Module.registerHooks() while preserving its SHA-256/preimage checks, supported source-type boundary, and fail-closed behavior.
  • Require OpenClaw channel-list evidence and include redacted stderr when the Slack proof emits no valid stdout record.
  • Cover Slack-plus-WhatsApp loader composition, stdout-only proof parsing, and mandatory channel-list output.

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:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This restores the existing documented Slack and WhatsApp behavior; only internal loader composition and CI diagnostics change. An independent documentation review found the existing channel docs 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: Independent diff review and the GPT-5.5 PR Review Advisor found no blocking correctness or security issues. The reviewed-source integrity check, fail-closed behavior, credential assertions, policy assertions, and redaction boundary remain intact. A thrown registerHooks() call is intentionally reported by failure class without reflecting arbitrary upstream exception text into logs; Node owns that source boundary, the regression test distinguishes unavailable from failed registration, and the fallback can be removed if the source rewrite is no longer needed.
  • 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: 29 CLI QR-loader tests, 7 Slack composition integration tests, and 23 E2E-support proof tests passed (59/59). Compiled production preloads also loaded successfully in both orders on Node 22.16.0. Exact-head Ubuntu messaging-providers passed consecutively in run 1 and run 2; the real-renderer whatsapp-qr-compact-e2e also passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: npm test was attempted with sandbox restrictions removed, but this macOS host cannot satisfy unrelated Linux ownership/capability, Docker, GNU stat -c, and Python >=3.10 test prerequisites. Ubuntu PR CI passed its build, type-check, test shards, installer, WSL, and macOS jobs; the exact-head Ubuntu live acceptance lane passed twice as linked above.
  • 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: Apurv Kumaria akumaria@nvidia.com

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria self-assigned this Jul 8, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The WhatsApp QR compact runtime now uses a synchronous load hook instead of a generated loader module, and Slack runtime proof parsing now includes stderr diagnostics. The affected e2e tests were updated to match the new loader behavior and proof failure handling.

Changes

Loader hook and Slack proof diagnostics fix

Layer / File(s) Summary
WhatsApp QR terminal load hook implementation
src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts
Adds decodeOpenClawQrTerminalSource and createOpenClawQrTerminalLoadHook, replacing the data-URL loader generator; reworks registration via Module.registerHooks and updates exported symbols.
Load hook test coverage
src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts
Replaces loader-source assertions with tests for the synchronous load hook covering pass-through, fail-closed, rewrite, and registration scenarios.
Slack runtime proof stderr-aware parsing
test/e2e/live/messaging-providers-slack-runtime-proof.ts
parseInstalledSlackProof now accepts stderr and includes it in thrown diagnostics; runInstalledSlackRuntimeProof forwards both stdout and stderr.
Slack proof parsing test coverage
test/e2e/support/messaging-providers-runtime-proofs.test.ts
Adds tests for stderr-triggered failures and stdout-only successful parsing, plus an assertion that the live messaging providers source doesn't suppress loader failures.
Channels list e2e assertion update
test/e2e/live/messaging-providers.test.ts
Replaces skip-on-empty-output logic with a runSandboxShell call enforcing exit success, throwing on empty stdout, and validating channel state and WhatsApp origin.
Composed Slack/WhatsApp loader guard probe test
test/openclaw-slack-deny-feedback-patch.test.ts
Adds a WhatsApp QR compact path constant and whatsappPreloadOrder, with a new test validating combined Slack/WhatsApp synchronous loader composition under ESM imports.

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

Sequence Diagram(s)

sequenceDiagram
  participant NodeModule
  participant LoadHook
  participant RendererSource
  NodeModule->>LoadHook: load(url, context, nextLoad)
  LoadHook->>NodeModule: nextLoad(url, context)
  NodeModule-->>LoadHook: module result
  LoadHook->>RendererSource: decode and check source
  LoadHook->>LoadHook: compute integrity and patch source
  LoadHook->>NodeModule: return patched or original result
Loading

Suggested labels: area: messaging, area: e2e, bug-fix

Suggested reviewers: cv, ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses the loader/API mismatch, preserves loader diagnostics via stderr handling, and keeps the Slack/WhatsApp runtime-proof tests aligned with the issue goals.
Out of Scope Changes check ✅ Passed The changes all relate to OpenClaw loader composition, Slack proof parsing, or the associated E2E coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the main change: composing OpenClaw runtime loaders.
✨ 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 fix/6467-openclaw-slack-runtime-proof

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 in the fix/6467-openclaw-sl... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/6467-openclaw-sl... 383414b +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the fix/6467-openclaw-sl... branch is 76%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/6467-openclaw-sl... 383414b +/-
src/lib/onboard/preflight.ts 82%
src/lib/state/o...oard-session.ts 82%
src/lib/actions...all/run-plan.ts 81%
src/lib/actions...licy-channel.ts 79%
src/lib/actions...box/snapshot.ts 79%
src/lib/state/sandbox.ts 75%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/policy/index.ts 65%
src/lib/shields/index.ts 60%
src/lib/onboard.ts 28%

Updated July 08, 2026 19:02 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: whatsapp-qr-compact-e2e, messaging-providers
Optional E2E: hermes-slack

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • whatsapp-qr-compact-e2e (low, about 15 minutes): Direct regression coverage for the changed WhatsApp compact-QR preload. It drives the real bundled OpenClaw/@openclaw/whatsapp renderQrTerminal path and verifies compact output with the NemoClaw preload.
  • messaging-providers (high, up to 90 minutes): Required because the source change can affect installed channel runtime startup, OpenClaw channels list, WhatsApp channel availability, and Slack preload composition. This live job covers WhatsApp QR-only channel setup/policy/runtime listing plus the installed Slack runtime allow/deny/send proof.

Optional E2E

  • hermes-slack (high, up to 75 minutes): Optional adjacent confidence for the Slack assistant path because the PR specifically changes preload composition that can interact with Slack runtime loading. Messaging-providers is the merge-blocking Slack coverage; this adds broader Hermes Slack flow confidence.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: messaging-providers
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=messaging-providers

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • messaging-providers: Focused free-standing E2E job wired for changed live test test/e2e/live/messaging-providers.test.ts.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=messaging-providers

Optional E2E targets

  • None.

Relevant changed files

  • src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts
  • test/e2e/live/messaging-providers-slack-runtime-proof.ts
  • test/e2e/live/messaging-providers.test.ts
  • test/e2e/support/messaging-providers-runtime-proofs.test.ts

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Blocked

Merge posture: Do not merge until addressed
Primary next action: Fix PRA-3: @ts-nocheck suppresses type safety on sandbox trust boundary; then add or justify PRA-T1.
Open items: 2 required · 8 warnings · 3 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 9 still apply · 2 new items found

Action checklist

  • PRA-3 Fix: @ts-nocheck suppresses type safety on sandbox trust boundary in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • PRA-4 Fix: registerOpenClawQrTerminalSourceLoader catch block swallows error detail without source-of-truth review in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • PRA-1 Resolve or justify: Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350-356 (registerOpenClawQrTerminalSourceLoader catch block)
  • PRA-2 Resolve or justify: Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140 (decodeOpenClawQrTerminalSource null return)
  • PRA-5 Resolve or justify: registerOpenClawQrTerminalSourceLoader accepts Module without shape validation or trust boundary comment in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:340
  • PRA-6 Resolve or justify: decodeOpenClawQrTerminalSource returns null for unsupported types but callers treat null as 'not target source' silently in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • PRA-8 Resolve or justify: Composed loader test verifies stderr cleanliness but not WhatsApp QR patch application in test/openclaw-slack-deny-feedback-patch.test.ts:230
  • PRA-9 Resolve or justify: Slack proof stderr rejection test only covers one error pattern in test/e2e/support/messaging-providers-runtime-proofs.test.ts:175
  • PRA-10 Resolve or justify: Source-of-truth review needed for catch block workaround in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • PRA-11 Resolve or justify: Source-of-truth review needed for decodeOpenClawQrTerminalSource null handling in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Composed loader test verifies stderr cleanliness but not WhatsApp QR patch application
  • PRA-T7 Add or justify test follow-up: Slack proof stderr rejection test only covers one error pattern
  • PRA-T8 Add or justify test follow-up: Acceptance clause
  • PRA-7 In-scope improvement: Legacy var/function declarations and missing TypeScript types on security boundary in src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • PRA-12 In-scope improvement: Cross-channel test coupling between Slack guard and WhatsApp QR compact in test/openclaw-slack-deny-feedback-patch.test.ts:230
  • PRA-13 In-scope improvement: Meta-test duplicates E2E behavioral assertions in test/e2e/support/messaging-providers-runtime-proofs.test.ts:215

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Required security src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1 Remove @ts-nocheck and fix any resulting type errors, or justify why this file cannot be typed.
PRA-4 Required architecture src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350 Either (a) include caught error detail in warning: warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed: ' + (_e?.message ?? String(_e)) + '; explicit compact quiet-zone rewrite skipped'); or (b) provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
PRA-5 Resolve/justify correctness src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:340 Add a comment documenting that Module is expected to be the Node.js core module object from require('module'), and that this is an internal trust boundary. Consider a lightweight brand check (e.g., Module._load === Module._load) or accept the test double pattern as intentional for testability.
PRA-6 Resolve/justify correctness src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140 Emit a warning when decodeOpenClawQrTerminalSource returns null for a module that matches the OpenClaw renderer source pattern (by request URL), or narrow the accepted types with explicit handling for all known Node.js module source formats.
PRA-7 Improvement workflow src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1 After removing @ts-nocheck, migrate to const/let, add parameter/return types on exported functions, and enable strict checking.
PRA-8 Resolve/justify tests test/openclaw-slack-deny-feedback-patch.test.ts:230 Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
PRA-9 Resolve/justify tests test/e2e/support/messaging-providers-runtime-proofs.test.ts:175 Add broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
PRA-10 Resolve/justify architecture src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350 Provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
PRA-11 Resolve/justify architecture src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140 Either add warning for null decode on matching modules, or provide source-of-truth review.
PRA-12 Improvement correctness test/openclaw-slack-deny-feedback-patch.test.ts:230 Extract a shared test helper for composing multiple runtime loaders, or move the cross-channel composition test to a dedicated test file.
PRA-13 Improvement scope test/e2e/support/messaging-providers-runtime-proofs.test.ts:215 Consider whether this meta-test adds value beyond the actual E2E test behavior. If kept, document why source-code assertion is preferred over behavioral test.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-3 Required — @ts-nocheck suppresses type safety on sandbox trust boundary

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • Category: security
  • Problem: The file whatsapp-qr-compact.ts has // @ts-nocheck at line 1, suppressing all TypeScript type checking for a preload script that runs inside the OpenShell sandbox and hooks Module._load / Module.registerHooks — a security-sensitive trust boundary. Type errors indicating incorrect hook signatures, unsafe source mutations, or wrong Module API usage are silently ignored.
  • Impact: Reduced confidence in sandbox boundary enforcement. Type errors that could indicate incorrect hook signatures or unsafe source transformations would be silently ignored, potentially allowing runtime loader misbehavior that compromises the sandbox trust boundary.
  • Required action: Remove @ts-nocheck and fix any resulting type errors, or justify why this file cannot be typed.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check line 1 of whatsapp-qr-compact.ts for @ts-nocheck. Run tsc --noEmit on the file to see suppressed errors.
  • Missing regression test: TypeScript compilation should pass without @ts-nocheck on this file.
  • Done when: The required change is committed and verification passes: Check line 1 of whatsapp-qr-compact.ts for @ts-nocheck. Run tsc --noEmit on the file to see suppressed errors.
  • Evidence: // @ts-nocheck at line 1 of whatsapp-qr-compact.ts

PRA-4 Required — registerOpenClawQrTerminalSourceLoader catch block swallows error detail without source-of-truth review

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • Category: architecture
  • Problem: registerOpenClawQrTerminalSourceLoader catch block (lines ~350-356) catches errors from Module.registerHooks() but discards the error detail (_e unused), emitting only a generic 'registration failed' warning. This is a localized workaround for upstream API mismatch. No source-of-truth review documents: the invalid state, source boundary, why source can't be fixed, regression test, or removal condition.
  • Impact: Operators cannot distinguish failure modes (Node.js internal error vs API mismatch vs memory pressure) from stderr alone. A regression where registerHooks throws for a new reason would surface only as a generic warning, potentially masking real issues.
  • Required action: Either (a) include caught error detail in warning: warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed: ' + (_e?.message ?? String(_e)) + '; explicit compact quiet-zone rewrite skipped'); or (b) provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Inspect registerOpenClawQrTerminalSourceLoader catch block — the caught error _e is unused and not logged.
  • Missing regression test: Test that registration failure includes error detail in stderr warning, OR documented source-of-truth review with regression test for the upstream API mismatch condition.
  • Done when: The required change is committed and verification passes: Inspect registerOpenClawQrTerminalSourceLoader catch block — the caught error _e is unused and not logged.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }
Review findings by urgency: 2 required fixes, 8 items to resolve/justify, 3 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350-356 (registerOpenClawQrTerminalSourceLoader catch block)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test 'distinguishes unavailable and failed synchronous hook registration' verifies failed registration returns false and emits warning, but does NOT verify error detail is included.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }

PRA-2 Resolve/justify — Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140 (decodeOpenClawQrTerminalSource null return)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test 'passes unsupported non-text module source through without throwing' passes {} as source, expects result returned unmodified. But no warning emitted.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { if (typeof source === 'string') return source; if (typeof Buffer !== 'undefined') { if (Buffer.isBuffer(source)) return source.toString('utf8'); if (source instanceof Uint8Array) return Buffer.from(source).toString('utf8'); if (source instanceof ArrayBuffer) return Buffer.from(source).toString('utf8'); } return null; }

PRA-5 Resolve/justify — registerOpenClawQrTerminalSourceLoader accepts Module without shape validation or trust boundary comment

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:340
  • Category: correctness
  • Problem: registerOpenClawQrTerminalSourceLoader accepts a Module parameter and only checks typeof Module.registerHooks === 'function'. No validation that Module is the Node.js core module object from require('module'), and no comment documenting this as an internal trust boundary. The test double pattern (passing { registerHooks: vi.fn() }) is used in tests but not documented as intentional.
  • Impact: If a malicious or buggy caller passes a fake Module object with a registerHooks function, the hook could be registered with unexpected behavior. Lack of trust boundary documentation makes it unclear whether the test double pattern is intentional or a gap.
  • Recommended action: Add a comment documenting that Module is expected to be the Node.js core module object from require('module'), and that this is an internal trust boundary. Consider a lightweight brand check (e.g., Module._load === Module._load) or accept the test double pattern as intentional for testability.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect registerOpenClawQrTerminalSourceLoader parameter handling and the test at whatsapp-qr-compact.test.ts:230 that passes a mock Module.
  • Missing regression test: Test that validates Module shape beyond registerHooks existence, or explicit trust boundary comment documenting that Module is expected to be Node's core module object.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect registerOpenClawQrTerminalSourceLoader parameter handling and the test at whatsapp-qr-compact.test.ts:230 that passes a mock Module.
  • Evidence: function registerOpenClawQrTerminalSourceLoader(Module) { if (!Module || typeof Module.registerHooks !== 'function') { ... }

PRA-6 Resolve/justify — decodeOpenClawQrTerminalSource returns null for unsupported types but callers treat null as 'not target source' silently

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • Category: correctness
  • Problem: decodeOpenClawQrTerminalSource (formerly openClawQrLoaderSourceToText) returns null for non-string, non-Buffer, non-Uint8Array, non-ArrayBuffer sources. The synchronous load hook treats null as 'not the OpenClaw renderer source' and passes the module through without any warning. If a future Node.js module format introduces a new source type (e.g., ReadableStream), it would silently fail to patch.
  • Impact: Silent degradation when encountering unexpected module source types. No diagnostic emitted when source cannot be decoded, making it impossible to detect format drift.
  • Recommended action: Emit a warning when decodeOpenClawQrTerminalSource returns null for a module that matches the OpenClaw renderer source pattern (by request URL), or narrow the accepted types with explicit handling for all known Node.js module source formats.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect decodeOpenClawQrTerminalSource and its caller createOpenClawQrTerminalLoadHook — source === null returns result unmodified without warning.
  • Missing regression test: Test that an unsupported source type (e.g., ReadableStream, plain object) emits a warning via warnWhatsappQrCompact, or explicit handling for all expected source types.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect decodeOpenClawQrTerminalSource and its caller createOpenClawQrTerminalLoadHook — source === null returns result unmodified without warning.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { if (typeof source === 'string') return source; if (typeof Buffer !== 'undefined') { if (Buffer.isBuffer(source)) return source.toString('utf8'); if (source instanceof Uint8Array) return Buffer.from(source).toString('utf8'); if (source instanceof ArrayBuffer) return Buffer.from(source).toString('utf8'); } return null; }

PRA-8 Resolve/justify — Composed loader test verifies stderr cleanliness but not WhatsApp QR patch application

  • Location: test/openclaw-slack-deny-feedback-patch.test.ts:230
  • Category: tests
  • Problem: The new test 'composes Slack-before-WhatsApp synchronous loaders for ESM imports (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' asserts that stderr doesn't contain loadSync errors, but does not verify that the WhatsApp QR compact patch was actually applied (e.g., COMPACT_MARGIN_MODULES = 4 in loaded module, or renderQrTerminal produces compact output). The test only confirms the Slack deny-feedback patch works.
  • Impact: The cross-preload composition could silently fail to apply the WhatsApp QR patch while appearing to pass. No regression coverage for the actual QR compact behavior when composed with Slack preload.
  • Recommended action: Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the test at openclaw-slack-deny-feedback-patch.test.ts:230 — it checks result.stderr but not the QR renderer source transformation.
  • Missing regression test: Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the test at openclaw-slack-deny-feedback-patch.test.ts:230 — it checks result.stderr but not the QR renderer source transformation.
  • Evidence: it('composes Slack-before-WhatsApp synchronous loaders for ESM imports (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { ... expect(result.stderr).not.toMatch(/loadSync|returned for the "source" from the "load" hook/u); ... })

PRA-9 Resolve/justify — Slack proof stderr rejection test only covers one error pattern

  • Location: test/e2e/support/messaging-providers-runtime-proofs.test.ts:175
  • Category: tests
  • Problem: The new test 'reports loader stderr without accepting stderr as a Slack proof (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' only tests one specific stderr pattern ('loadSync is not a function'). Other loader failure patterns (e.g., Module.registerHooks errors, syntax errors, OOM) would not be covered by this test.
  • Impact: A different loader failure mode could leak into stdout and be misparsed as a valid proof, or a real proof with different stderr could be incorrectly rejected.
  • Recommended action: Add broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect parseInstalledSlackProof — it only parses stdout for the proof JSON; stderr is only included in the error message on failure.
  • Missing regression test: Broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect parseInstalledSlackProof — it only parses stdout for the proof JSON; stderr is only included in the error message on failure.
  • Evidence: it('reports loader stderr without accepting stderr as a Slack proof (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { const stderr = ['[channels] [slack] provider failed to start: this[#customizations].loadSync is not a function', proof].join('\n'); expect(() => parseInstalledSlackProof('', stderr)).toThrow(/stderr:.*loadSync is not a function/su); })

PRA-10 Resolve/justify — Source-of-truth review needed for catch block workaround

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • Category: architecture
  • Problem: The catch block in registerOpenClawQrTerminalSourceLoader is a localized workaround for Module.registerHooks API mismatch. No documentation of: what invalid state is handled, where that state is created, why the source cannot be fixed in this PR, what regression test proves the source cannot regress, and when the workaround can be removed.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear. Future maintainers cannot evaluate whether the workaround is still needed.
  • Recommended action: Provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the catch block and surrounding context — no source-of-truth review fields present.
  • Missing regression test: Documented source-of-truth review with: invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the catch block and surrounding context — no source-of-truth review fields present.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }

PRA-11 Resolve/justify — Source-of-truth review needed for decodeOpenClawQrTerminalSource null handling

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • Category: architecture
  • Problem: decodeOpenClawQrTerminalSource returns null for unsupported source types, and callers treat null as 'not target source' without warning. This is a localized tolerant-parsing workaround. No documentation of: what invalid state is handled, where that state is created, why the source cannot be fixed, what regression test proves the source cannot regress, and when the workaround can be removed.
  • Impact: Silent degradation when Node.js introduces new module source formats. The workaround masks format drift instead of detecting it.
  • Recommended action: Either add warning for null decode on matching modules, or provide source-of-truth review.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect decodeOpenClawQrTerminalSource and createOpenClawQrTerminalLoadHook — null returns result unmodified.
  • Missing regression test: Documented source-of-truth review with regression test for unsupported source type handling.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect decodeOpenClawQrTerminalSource and createOpenClawQrTerminalLoadHook — null returns result unmodified.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { ... return null; }

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-7 Improvement — Legacy var/function declarations and missing TypeScript types on security boundary

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • Category: workflow
  • Problem: The file uses var and function declarations throughout, lacks parameter/return type annotations on exported functions, and has @ts-nocheck suppressing all checking. This is a security-boundary preload that runs inside the sandbox.
  • Impact: Reduced maintainability and auditability. Type errors that could indicate incorrect hook signatures or unsafe mutations are not caught.
  • Suggested action: After removing @ts-nocheck, migrate to const/let, add parameter/return types on exported functions, and enable strict checking.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run tsc --noEmit on the file after removing @ts-nocheck to see all type errors.
  • Missing regression test: TypeScript compilation should pass in strict mode for this file.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: var REVIEWED_OPENCLAW_QR_TERMINAL_RENDERER_SHA256 = ...; function isOpenClawQrTerminalRendererSource(source) { ... }

PRA-12 Improvement — Cross-channel test coupling between Slack guard and WhatsApp QR compact

  • Location: test/openclaw-slack-deny-feedback-patch.test.ts:230
  • Category: correctness
  • Problem: The test file openclaw-slack-deny-feedback-patch.test.ts imports and requires whatsapp-qr-compact.ts directly to test cross-preload composition. This creates test coupling between two unrelated channel preloads. The new test 'composes Slack-before-WhatsApp synchronous loaders for ESM imports' adds to this coupling.
  • Impact: Changes to WhatsApp QR compact could break Slack tests and vice versa. Test isolation is reduced. The coupling exists because both preloads hook Module._load/registerHooks and must coexist.
  • Suggested action: Extract a shared test helper for composing multiple runtime loaders, or move the cross-channel composition test to a dedicated test file.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect openclaw-slack-deny-feedback-patch.test.ts — it requires WHATSAPP_QR_COMPACT path and tests composition order.
  • Missing regression test: Extract a shared test helper for composing multiple runtime loaders, or move the cross-channel composition test to a dedicated test file.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: const WHATSAPP_QR_COMPACT = path.join(import.meta.dirname, '..', 'src', 'lib', 'messaging', 'channels', 'whatsapp', 'runtime', 'whatsapp-qr-compact.ts');

PRA-13 Improvement — Meta-test duplicates E2E behavioral assertions

  • Location: test/e2e/support/messaging-providers-runtime-proofs.test.ts:215
  • Category: scope
  • Problem: The test 'requires channel-list output without suppressing loader failures (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' asserts that the live E2E test source code contains specific strings (command text, error messages, absence of old patterns). This is a meta-test that duplicates behavioral assertions already covered by the actual E2E test execution.
  • Impact: Test maintenance burden: changes to the E2E test require updating both the behavioral test and the meta-test. Source-code assertions are brittle and don't prove runtime behavior.
  • Suggested action: Consider whether this meta-test adds value beyond the actual E2E test behavior. If kept, document why source-code assertion is preferred over behavioral test.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect messaging-providers-runtime-proofs.test.ts:215 — it reads LIVE_MESSAGING_PROVIDERS_SOURCE and checks for string presence.
  • Missing regression test: Consider whether this meta-test adds value beyond the actual E2E test behavior. If kept, document why source-code assertion is preferred over behavioral test.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: it('requires channel-list output without suppressing loader failures (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { expect(LIVE_MESSAGING_PROVIDERS_SOURCE).toContain('timeout 45 openclaw channels list --all --json --no-color'); ... })
Simplification opportunities: 1 possible cut, net -30 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-13 yagni (test/e2e/support/messaging-providers-runtime-proofs.test.ts:215): Meta-test asserting source code strings in messaging-providers.test.ts
    • Replacement: Rely on behavioral E2E test execution; remove meta-test or document why source-code assertion is needed
    • Net: -30 lines
    • Safety boundary: Do not remove the actual E2E test behavior in messaging-providers.test.ts that validates channel-list output and stderr surfacing
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 — Unit test: createOpenClawQrTerminalLoadHook with unsupported source type (e.g., {}) on URL matching OpenClaw renderer pattern emits warnWhatsappQrCompact. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts. Unit tests cover pure helpers but composed loader behavior only tested with mocks, not in real sandbox.
  • PRA-T2 Runtime validation — Unit test: registerOpenClawQrTerminalSourceLoader with throwing registerHooks includes error.message in warnWhatsappQrCompact call. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts. Unit tests cover pure helpers but composed loader behavior only tested with mocks, not in real sandbox.
  • PRA-T3 Runtime validation — Unit test: registerOpenClawQrTerminalSourceLoader validates Module has core properties (e.g., _load) or documents trust boundary expectation. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts. Unit tests cover pure helpers but composed loader behavior only tested with mocks, not in real sandbox.
  • PRA-T4 Runtime validation — Integration test: composed Slack+WhatsApp preloads in real Node process — verify loaded QR renderer source contains COMPACT_MARGIN_MODULES = 4. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts. Unit tests cover pure helpers but composed loader behavior only tested with mocks, not in real sandbox.
  • PRA-T5 Runtime validation — Parametrized test: parseInstalledSlackProof with stderr containing '[channels] WhatsApp compact-QR warning:', '[slack] provider failed', SyntaxError, etc. — all rejected. Runtime/sandbox/infrastructure paths need behavioral runtime validation: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts. Unit tests cover pure helpers but composed loader behavior only tested with mocks, not in real sandbox.
  • PRA-T6 Composed loader test verifies stderr cleanliness but not WhatsApp QR patch application — Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
  • PRA-T7 Slack proof stderr rejection test only covers one error pattern — Add broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
  • PRA-T8 Acceptance clause — Pass messaging-providers on two consecutive Ubuntu live E2E runs — add test evidence or identify existing coverage. PR description links two passing runs (run 1 and run 2). External CI verification not performed in this review.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350-356 (registerOpenClawQrTerminalSourceLoader catch block)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test 'distinguishes unavailable and failed synchronous hook registration' verifies failed registration returns false and emits warning, but does NOT verify error detail is included.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }

PRA-2 Resolve/justify — Source-of-truth review needed: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140 (decodeOpenClawQrTerminalSource null return)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Test 'passes unsupported non-text module source through without throwing' passes {} as source, expects result returned unmodified. But no warning emitted.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { if (typeof source === 'string') return source; if (typeof Buffer !== 'undefined') { if (Buffer.isBuffer(source)) return source.toString('utf8'); if (source instanceof Uint8Array) return Buffer.from(source).toString('utf8'); if (source instanceof ArrayBuffer) return Buffer.from(source).toString('utf8'); } return null; }

PRA-3 Required — @ts-nocheck suppresses type safety on sandbox trust boundary

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • Category: security
  • Problem: The file whatsapp-qr-compact.ts has // @ts-nocheck at line 1, suppressing all TypeScript type checking for a preload script that runs inside the OpenShell sandbox and hooks Module._load / Module.registerHooks — a security-sensitive trust boundary. Type errors indicating incorrect hook signatures, unsafe source mutations, or wrong Module API usage are silently ignored.
  • Impact: Reduced confidence in sandbox boundary enforcement. Type errors that could indicate incorrect hook signatures or unsafe source transformations would be silently ignored, potentially allowing runtime loader misbehavior that compromises the sandbox trust boundary.
  • Required action: Remove @ts-nocheck and fix any resulting type errors, or justify why this file cannot be typed.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check line 1 of whatsapp-qr-compact.ts for @ts-nocheck. Run tsc --noEmit on the file to see suppressed errors.
  • Missing regression test: TypeScript compilation should pass without @ts-nocheck on this file.
  • Done when: The required change is committed and verification passes: Check line 1 of whatsapp-qr-compact.ts for @ts-nocheck. Run tsc --noEmit on the file to see suppressed errors.
  • Evidence: // @ts-nocheck at line 1 of whatsapp-qr-compact.ts

PRA-4 Required — registerOpenClawQrTerminalSourceLoader catch block swallows error detail without source-of-truth review

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • Category: architecture
  • Problem: registerOpenClawQrTerminalSourceLoader catch block (lines ~350-356) catches errors from Module.registerHooks() but discards the error detail (_e unused), emitting only a generic 'registration failed' warning. This is a localized workaround for upstream API mismatch. No source-of-truth review documents: the invalid state, source boundary, why source can't be fixed, regression test, or removal condition.
  • Impact: Operators cannot distinguish failure modes (Node.js internal error vs API mismatch vs memory pressure) from stderr alone. A regression where registerHooks throws for a new reason would surface only as a generic warning, potentially masking real issues.
  • Required action: Either (a) include caught error detail in warning: warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed: ' + (_e?.message ?? String(_e)) + '; explicit compact quiet-zone rewrite skipped'); or (b) provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Inspect registerOpenClawQrTerminalSourceLoader catch block — the caught error _e is unused and not logged.
  • Missing regression test: Test that registration failure includes error detail in stderr warning, OR documented source-of-truth review with regression test for the upstream API mismatch condition.
  • Done when: The required change is committed and verification passes: Inspect registerOpenClawQrTerminalSourceLoader catch block — the caught error _e is unused and not logged.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }

PRA-5 Resolve/justify — registerOpenClawQrTerminalSourceLoader accepts Module without shape validation or trust boundary comment

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:340
  • Category: correctness
  • Problem: registerOpenClawQrTerminalSourceLoader accepts a Module parameter and only checks typeof Module.registerHooks === 'function'. No validation that Module is the Node.js core module object from require('module'), and no comment documenting this as an internal trust boundary. The test double pattern (passing { registerHooks: vi.fn() }) is used in tests but not documented as intentional.
  • Impact: If a malicious or buggy caller passes a fake Module object with a registerHooks function, the hook could be registered with unexpected behavior. Lack of trust boundary documentation makes it unclear whether the test double pattern is intentional or a gap.
  • Recommended action: Add a comment documenting that Module is expected to be the Node.js core module object from require('module'), and that this is an internal trust boundary. Consider a lightweight brand check (e.g., Module._load === Module._load) or accept the test double pattern as intentional for testability.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect registerOpenClawQrTerminalSourceLoader parameter handling and the test at whatsapp-qr-compact.test.ts:230 that passes a mock Module.
  • Missing regression test: Test that validates Module shape beyond registerHooks existence, or explicit trust boundary comment documenting that Module is expected to be Node's core module object.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect registerOpenClawQrTerminalSourceLoader parameter handling and the test at whatsapp-qr-compact.test.ts:230 that passes a mock Module.
  • Evidence: function registerOpenClawQrTerminalSourceLoader(Module) { if (!Module || typeof Module.registerHooks !== 'function') { ... }

PRA-6 Resolve/justify — decodeOpenClawQrTerminalSource returns null for unsupported types but callers treat null as 'not target source' silently

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • Category: correctness
  • Problem: decodeOpenClawQrTerminalSource (formerly openClawQrLoaderSourceToText) returns null for non-string, non-Buffer, non-Uint8Array, non-ArrayBuffer sources. The synchronous load hook treats null as 'not the OpenClaw renderer source' and passes the module through without any warning. If a future Node.js module format introduces a new source type (e.g., ReadableStream), it would silently fail to patch.
  • Impact: Silent degradation when encountering unexpected module source types. No diagnostic emitted when source cannot be decoded, making it impossible to detect format drift.
  • Recommended action: Emit a warning when decodeOpenClawQrTerminalSource returns null for a module that matches the OpenClaw renderer source pattern (by request URL), or narrow the accepted types with explicit handling for all known Node.js module source formats.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect decodeOpenClawQrTerminalSource and its caller createOpenClawQrTerminalLoadHook — source === null returns result unmodified without warning.
  • Missing regression test: Test that an unsupported source type (e.g., ReadableStream, plain object) emits a warning via warnWhatsappQrCompact, or explicit handling for all expected source types.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect decodeOpenClawQrTerminalSource and its caller createOpenClawQrTerminalLoadHook — source === null returns result unmodified without warning.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { if (typeof source === 'string') return source; if (typeof Buffer !== 'undefined') { if (Buffer.isBuffer(source)) return source.toString('utf8'); if (source instanceof Uint8Array) return Buffer.from(source).toString('utf8'); if (source instanceof ArrayBuffer) return Buffer.from(source).toString('utf8'); } return null; }

PRA-7 Improvement — Legacy var/function declarations and missing TypeScript types on security boundary

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:1
  • Category: workflow
  • Problem: The file uses var and function declarations throughout, lacks parameter/return type annotations on exported functions, and has @ts-nocheck suppressing all checking. This is a security-boundary preload that runs inside the sandbox.
  • Impact: Reduced maintainability and auditability. Type errors that could indicate incorrect hook signatures or unsafe mutations are not caught.
  • Suggested action: After removing @ts-nocheck, migrate to const/let, add parameter/return types on exported functions, and enable strict checking.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run tsc --noEmit on the file after removing @ts-nocheck to see all type errors.
  • Missing regression test: TypeScript compilation should pass in strict mode for this file.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: var REVIEWED_OPENCLAW_QR_TERMINAL_RENDERER_SHA256 = ...; function isOpenClawQrTerminalRendererSource(source) { ... }

PRA-8 Resolve/justify — Composed loader test verifies stderr cleanliness but not WhatsApp QR patch application

  • Location: test/openclaw-slack-deny-feedback-patch.test.ts:230
  • Category: tests
  • Problem: The new test 'composes Slack-before-WhatsApp synchronous loaders for ESM imports (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' asserts that stderr doesn't contain loadSync errors, but does not verify that the WhatsApp QR compact patch was actually applied (e.g., COMPACT_MARGIN_MODULES = 4 in loaded module, or renderQrTerminal produces compact output). The test only confirms the Slack deny-feedback patch works.
  • Impact: The cross-preload composition could silently fail to apply the WhatsApp QR patch while appearing to pass. No regression coverage for the actual QR compact behavior when composed with Slack preload.
  • Recommended action: Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the test at openclaw-slack-deny-feedback-patch.test.ts:230 — it checks result.stderr but not the QR renderer source transformation.
  • Missing regression test: Add assertion that the OpenClaw QR renderer source was patched — e.g., verify COMPACT_MARGIN_MODULES = 4 in loaded module, or that renderQrTerminal produces compact output.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the test at openclaw-slack-deny-feedback-patch.test.ts:230 — it checks result.stderr but not the QR renderer source transformation.
  • Evidence: it('composes Slack-before-WhatsApp synchronous loaders for ESM imports (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { ... expect(result.stderr).not.toMatch(/loadSync|returned for the "source" from the "load" hook/u); ... })

PRA-9 Resolve/justify — Slack proof stderr rejection test only covers one error pattern

  • Location: test/e2e/support/messaging-providers-runtime-proofs.test.ts:175
  • Category: tests
  • Problem: The new test 'reports loader stderr without accepting stderr as a Slack proof (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' only tests one specific stderr pattern ('loadSync is not a function'). Other loader failure patterns (e.g., Module.registerHooks errors, syntax errors, OOM) would not be covered by this test.
  • Impact: A different loader failure mode could leak into stdout and be misparsed as a valid proof, or a real proof with different stderr could be incorrectly rejected.
  • Recommended action: Add broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect parseInstalledSlackProof — it only parses stdout for the proof JSON; stderr is only included in the error message on failure.
  • Missing regression test: Broader test that any stderr containing '[channels]' or '[slack]' loader diagnostics is rejected unless stdout contains a valid proof, or document that the proof parser only accepts stdout.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect parseInstalledSlackProof — it only parses stdout for the proof JSON; stderr is only included in the error message on failure.
  • Evidence: it('reports loader stderr without accepting stderr as a Slack proof (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { const stderr = ['[channels] [slack] provider failed to start: this[#customizations].loadSync is not a function', proof].join('\n'); expect(() => parseInstalledSlackProof('', stderr)).toThrow(/stderr:.*loadSync is not a function/su); })

PRA-10 Resolve/justify — Source-of-truth review needed for catch block workaround

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:350
  • Category: architecture
  • Problem: The catch block in registerOpenClawQrTerminalSourceLoader is a localized workaround for Module.registerHooks API mismatch. No documentation of: what invalid state is handled, where that state is created, why the source cannot be fixed in this PR, what regression test proves the source cannot regress, and when the workaround can be removed.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear. Future maintainers cannot evaluate whether the workaround is still needed.
  • Recommended action: Provide source-of-truth review documenting invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the catch block and surrounding context — no source-of-truth review fields present.
  • Missing regression test: Documented source-of-truth review with: invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the catch block and surrounding context — no source-of-truth review fields present.
  • Evidence: } catch (_e) { warnWhatsappQrCompact('OpenClaw QR renderer source loader registration failed; explicit compact quiet-zone rewrite skipped'); return false; }

PRA-11 Resolve/justify — Source-of-truth review needed for decodeOpenClawQrTerminalSource null handling

  • Location: src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts:140
  • Category: architecture
  • Problem: decodeOpenClawQrTerminalSource returns null for unsupported source types, and callers treat null as 'not target source' without warning. This is a localized tolerant-parsing workaround. No documentation of: what invalid state is handled, where that state is created, why the source cannot be fixed, what regression test proves the source cannot regress, and when the workaround can be removed.
  • Impact: Silent degradation when Node.js introduces new module source formats. The workaround masks format drift instead of detecting it.
  • Recommended action: Either add warning for null decode on matching modules, or provide source-of-truth review.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect decodeOpenClawQrTerminalSource and createOpenClawQrTerminalLoadHook — null returns result unmodified.
  • Missing regression test: Documented source-of-truth review with regression test for unsupported source type handling.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect decodeOpenClawQrTerminalSource and createOpenClawQrTerminalLoadHook — null returns result unmodified.
  • Evidence: function decodeOpenClawQrTerminalSource(source) { ... return null; }

PRA-12 Improvement — Cross-channel test coupling between Slack guard and WhatsApp QR compact

  • Location: test/openclaw-slack-deny-feedback-patch.test.ts:230
  • Category: correctness
  • Problem: The test file openclaw-slack-deny-feedback-patch.test.ts imports and requires whatsapp-qr-compact.ts directly to test cross-preload composition. This creates test coupling between two unrelated channel preloads. The new test 'composes Slack-before-WhatsApp synchronous loaders for ESM imports' adds to this coupling.
  • Impact: Changes to WhatsApp QR compact could break Slack tests and vice versa. Test isolation is reduced. The coupling exists because both preloads hook Module._load/registerHooks and must coexist.
  • Suggested action: Extract a shared test helper for composing multiple runtime loaders, or move the cross-channel composition test to a dedicated test file.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect openclaw-slack-deny-feedback-patch.test.ts — it requires WHATSAPP_QR_COMPACT path and tests composition order.
  • Missing regression test: Extract a shared test helper for composing multiple runtime loaders, or move the cross-channel composition test to a dedicated test file.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: const WHATSAPP_QR_COMPACT = path.join(import.meta.dirname, '..', 'src', 'lib', 'messaging', 'channels', 'whatsapp', 'runtime', 'whatsapp-qr-compact.ts');

PRA-13 Improvement — Meta-test duplicates E2E behavioral assertions

  • Location: test/e2e/support/messaging-providers-runtime-proofs.test.ts:215
  • Category: scope
  • Problem: The test 'requires channel-list output without suppressing loader failures (test(e2e): fix empty OpenClaw Slack runtime proof #6467)' asserts that the live E2E test source code contains specific strings (command text, error messages, absence of old patterns). This is a meta-test that duplicates behavioral assertions already covered by the actual E2E test execution.
  • Impact: Test maintenance burden: changes to the E2E test require updating both the behavioral test and the meta-test. Source-code assertions are brittle and don't prove runtime behavior.
  • Suggested action: Consider whether this meta-test adds value beyond the actual E2E test behavior. If kept, document why source-code assertion is preferred over behavioral test.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Inspect messaging-providers-runtime-proofs.test.ts:215 — it reads LIVE_MESSAGING_PROVIDERS_SOURCE and checks for string presence.
  • Missing regression test: Consider whether this meta-test adds value beyond the actual E2E test behavior. If kept, document why source-code assertion is preferred over behavioral test.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: it('requires channel-list output without suppressing loader failures (test(e2e): fix empty OpenClaw Slack runtime proof #6467)', () => { expect(LIVE_MESSAGING_PROVIDERS_SOURCE).toContain('timeout 45 openclaw channels list --all --json --no-color'); ... })

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 — 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 · 5 test follow-ups
Since last review: 0 prior items resolved · 3 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Acceptance clause
  • PRA-T5 Add or justify test follow-up: Acceptance clause
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 — Validate compiled production Slack and WhatsApp preload artifacts under the sandbox Node version in both preload orders, then import an ESM Slack fixture and assert no `loadSync` or `returned for the "source" from the "load" hook` error appears.. Changed code touches sandbox/OpenClaw runtime loader infrastructure where compiled production artifacts and sandbox Node behavior can differ from source-level unit and spawned-fixture tests. Existing changed tests are strong for the source hook, negative paths, and Slack/WhatsApp preload ordering, but they do not by themselves prove compiled artifact behavior inside the target sandbox runtime.
  • PRA-T2 Runtime validation — Validate the compiled WhatsApp QR source hook against the reviewed OpenClaw renderer source in the sandbox Node runtime for string, Buffer, Uint8Array, and ArrayBuffer source forms, asserting reviewed sources rewrite to `COMPACT_MARGIN_MODULES = 4` and unreviewed hashes pass through unchanged.. Changed code touches sandbox/OpenClaw runtime loader infrastructure where compiled production artifacts and sandbox Node behavior can differ from source-level unit and spawned-fixture tests. Existing changed tests are strong for the source hook, negative paths, and Slack/WhatsApp preload ordering, but they do not by themselves prove compiled artifact behavior inside the target sandbox runtime.
  • PRA-T3 Runtime validation — Record or identify maintained acceptance evidence for two consecutive Ubuntu `messaging-providers` live E2E runs; external run pass/fail status is not evaluated by this advisor surface.. Changed code touches sandbox/OpenClaw runtime loader infrastructure where compiled production artifacts and sandbox Node behavior can differ from source-level unit and spawned-fixture tests. Existing changed tests are strong for the source hook, negative paths, and Slack/WhatsApp preload ordering, but they do not by themselves prove compiled artifact behavior inside the target sandbox runtime.
  • PRA-T4 Acceptance clause — Reproduce and identify the OpenClaw Slack loader/API mismatch on current `main`. — add test evidence or identify existing coverage. The changed code identifies the mismatch class in `registerOpenClawQrTerminalSourceLoader()` comments: mixing async `Module.register()` with sync `registerHooks()` breaks synchronous module loads because the async customization object has no `loadSync`. The support test `reports loader stderr without accepting stderr as a Slack proof (test(e2e): fix empty OpenClaw Slack runtime proof #6467)` encodes the `this[#customizations].loadSync is not a function` diagnostic. This read-only review did not reproduce current `main`.
  • PRA-T5 Acceptance clause — Pass `messaging-providers` on two consecutive Ubuntu live E2E runs. — add test evidence or identify existing coverage. External live E2E run status is outside this advisor review surface. The diff makes the live lane stricter by requiring channel-list output and preserving loader stderr diagnostics, but this review did not verify external Ubuntu run results.

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

E2E Target Results — ⚠️ Run cancelled — no signal

Run: 28955537719
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 0 passed, 0 failed, 1 cancelled, 0 skipped

Job Result
messaging-providers ⚠️ cancelled

@apurvvkumaria
apurvvkumaria requested a review from jyaunches July 8, 2026 15:59
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28958127861
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28959131356
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

@apurvvkumaria
apurvvkumaria marked this pull request as ready for review July 8, 2026 16:51

@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)
src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts (1)

197-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the Uint8Array/ArrayBuffer decode branches too.

The rewrite test only exercises the Buffer branch of decodeOpenClawQrTerminalSource; the Uint8Array and ArrayBuffer branches (Lines 150-151 in the source file) aren't exercised by any test.

✅ Suggested additional coverage
+  it("decodes Uint8Array and ArrayBuffer module sources through the load hook", () => {
+    const load = createOpenClawQrTerminalLoadHook(
+      () => REVIEWED_OPENCLAW_QR_TERMINAL_RENDERER_SHA256,
+    );
+    const encoded = new TextEncoder().encode(OPENCLAW_QR_RENDERER_SOURCE);
+    const result = { format: "module", source: encoded };
+
+    expect(load("file:///tmp/openclaw-renderer.mjs", {}, () => result)).toMatchObject({
+      format: "module",
+      source: expect.stringContaining("const COMPACT_MARGIN_MODULES = 4;"),
+    });
+  });
🤖 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 `@src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts`
around lines 197 - 207, Add test coverage for the remaining decode branches in
decodeOpenClawQrTerminalSource: the current rewrite test only passes a Buffer
through createOpenClawQrTerminalLoadHook. Extend the whatsapp-qr-compact.test.ts
suite with cases that feed a Uint8Array and an ArrayBuffer into the load hook
(or directly into decodeOpenClawQrTerminalSource if that is the best entry
point) and assert they are rewritten the same way as the Buffer case.
test/e2e/support/messaging-providers-runtime-proofs.test.ts (1)

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

Source-text assertion locks to implementation, not behavior.

This test only checks that specific string literals exist/don't exist in another file's raw source. It provides a fast regression guard against reintroducing this specific bug's literal pattern, but it doesn't exercise the actual channel-list command execution or failure-surfacing behavior, and would pass even if the shell invocation were functionally rewritten with different wording, or fail on a harmless textual change.

If practical, prefer asserting the observable behavior (e.g., that a failing/empty-stdout shell result surfaces a diagnostic error) via the exported runSandboxShell/proof-parsing helpers rather than string-matching the other test file's source.

🤖 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/messaging-providers-runtime-proofs.test.ts` around lines 205
- 219, The current check in messaging-providers-runtime-proofs.test.ts is a
source-text assertion that only matches literals in another file, so it should
be replaced with a behavior-focused test. Update the proof using the exported
runSandboxShell and proof-parsing helpers to exercise a failing or empty-stdout
OpenClaw channels list invocation and assert the diagnostic/error surfaces as
expected, rather than inspecting LIVE_MESSAGING_PROVIDERS_SOURCE for specific
strings.

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.

Nitpick comments:
In `@src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts`:
- Around line 197-207: Add test coverage for the remaining decode branches in
decodeOpenClawQrTerminalSource: the current rewrite test only passes a Buffer
through createOpenClawQrTerminalLoadHook. Extend the whatsapp-qr-compact.test.ts
suite with cases that feed a Uint8Array and an ArrayBuffer into the load hook
(or directly into decodeOpenClawQrTerminalSource if that is the best entry
point) and assert they are rewritten the same way as the Buffer case.

In `@test/e2e/support/messaging-providers-runtime-proofs.test.ts`:
- Around line 205-219: The current check in
messaging-providers-runtime-proofs.test.ts is a source-text assertion that only
matches literals in another file, so it should be replaced with a
behavior-focused test. Update the proof using the exported runSandboxShell and
proof-parsing helpers to exercise a failing or empty-stdout OpenClaw channels
list invocation and assert the diagnostic/error surfaces as expected, rather
than inspecting LIVE_MESSAGING_PROVIDERS_SOURCE for specific strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2e8e0c13-ad33-452e-91e4-8c8244e4d7e9

📥 Commits

Reviewing files that changed from the base of the PR and between d50338a and 6542eaa.

📒 Files selected for processing (6)
  • src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.test.ts
  • src/lib/messaging/channels/whatsapp/runtime/whatsapp-qr-compact.ts
  • test/e2e/live/messaging-providers-slack-runtime-proof.ts
  • test/e2e/live/messaging-providers.test.ts
  • test/e2e/support/messaging-providers-runtime-proofs.test.ts
  • test/openclaw-slack-deny-feedback-patch.test.ts

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28961025481
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28961955814
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@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/openclaw-slack-deny-feedback-patch.test.ts (1)

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

Good coverage of both preload orderings.

The two ESM composition tests (before-slack / after-slack) directly exercise the ordering contract described in whatsapp-qr-compact.ts's loader registration comment, which is the actual root cause referenced in the linked issue (this[#customizations].loadSync is not a function). This is solid regression coverage for #6467.

One optional thought: the two tests share nearly identical setup/assertion structure differing only by whatsappPreloadOrder. Consider parameterizing with it.each to reduce duplication, though this is purely a maintainability nice-to-have given the current size.

Also applies to: 282-301

🤖 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/openclaw-slack-deny-feedback-patch.test.ts` around lines 260 - 263, The
two ESM composition tests duplicate the same setup and assertions while only
changing whatsappPreloadOrder. Refactor the coverage in the test file to use a
parameterized table-driven form such as it.each around runGuardProbe, so both
"before-slack" and "after-slack" cases share one test body. Keep the existing
assertions and inputs intact, just drive the preload-order variation through the
parameter list to reduce duplication.
🤖 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/openclaw-slack-deny-feedback-patch.test.ts`:
- Around line 260-263: The two ESM composition tests duplicate the same setup
and assertions while only changing whatsappPreloadOrder. Refactor the coverage
in the test file to use a parameterized table-driven form such as it.each around
runGuardProbe, so both "before-slack" and "after-slack" cases share one test
body. Keep the existing assertions and inputs intact, just drive the
preload-order variation through the parameter list to reduce duplication.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f4efef0e-bdfd-4481-a22e-a551739be307

📥 Commits

Reviewing files that changed from the base of the PR and between 86c5012 and f45b093.

📒 Files selected for processing (1)
  • test/openclaw-slack-deny-feedback-patch.test.ts

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28964320005
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 28965663864
Workflow ref: fix/6467-openclaw-slack-runtime-proof
Requested targets: (default — all supported)
Requested jobs: messaging-providers
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
messaging-providers ✅ success

@apurvvkumaria
apurvvkumaria enabled auto-merge (squash) July 8, 2026 18:49
@apurvvkumaria
apurvvkumaria requested a review from cv July 8, 2026 20:54
@apurvvkumaria
apurvvkumaria merged commit 0b11898 into main Jul 8, 2026
124 of 125 checks passed
@apurvvkumaria
apurvvkumaria deleted the fix/6467-openclaw-slack-runtime-proof branch July 8, 2026 20:55
cv added a commit that referenced this pull request Jul 8, 2026
…6431)

<!-- markdownlint-disable MD041 -->
## Summary
Replaces NemoClaw's build-time mutation of the released Deep Agents
bootstrap with a first-party `deepagents.harness_profiles` plugin for
`deepagents-code==0.1.34` / `deepagents==0.7.0a6`. The two managed
OpenAI-compatible model keys continue to receive the released native
Nemotron 3 Ultra profile, with exact version/source gates and no
third-party source changes.

## Related Issue
Fixes #6424

## Changes
- Add and install `nemoclaw-deepagents-profile==0.1.0` through Deep
Agents' supported profile entry-point lifecycle.
- Register only the two NemoClaw-managed aliases against the released
canonical Ultra profile, atomically and idempotently.
- Fail the image build on missing or unimportable dependencies,
mismatched distribution/package roots, copied/installed adapter-source
drift, or released-profile/bootstrap drift.
- Run a DCode-only negative Docker build from the current hash-locked
base, strip both upstream distributions, and prove failure occurs at the
isolated import gate before the later dependency check.
- Build and install a real unreviewed-version plugin wheel and prove the
actual validator rejects it.
- Verify entry-point discovery, all 12 middleware entries,
unrelated-model isolation, graph compilation, and allowed/denied
execute-dispatch parity against the official wheels.
- Split image/runtime and credential-boundary contracts into balanced
756/755-line suites with a 113-line shared helper, preserving all 75
original tests and substantial per-file size headroom.
- Remove the installed-bootstrap patcher and document that the adapter
must be removed, not rehashed, once reviewed dependencies provide both
exact aliases.
- Preserve the merged DCode hardening and paced `/agents` first-run TUI
behavior from #6410 / #6418.

## Automated review dispositions

- **License metadata:** the production package keeps the PEP 639 SPDX
string and builds unchanged with lock-pinned `setuptools==82.0.1`; the
production validator now requires exact installed-wheel
`License-Expression: Apache-2.0` metadata, with a negative
metadata-drift test. The legacy conversion is a localized offline
wrong-version fixture with explicit source-boundary and
removal-condition documentation. Remove the fixture-only conversion once
runner setuptools accepts PEP 639 strings; production never uses it.
- **Plain-progress build output:** plain progress remains necessary to
prove the exact import-failure marker. Before Docker runs, the gate now
rejects every Docker `ARG` name outside a complete reviewed allowlist,
while tests pin the only passed build arguments to the two public
`BASE_IMAGE` references. Behavior tests inject unreviewed uppercase,
lowercase, and continued ARG declarations across all three Dockerfiles
and prove rejection occurs before any build; the targeted DCode E2E job
runs the same script with real Docker before live tests.
- **Adapter build-layer retention:** Docker can retain the copied
project tree in an image layer or failed local build cache. This is
accepted because it contains only public, first-party Apache-2.0 source
and metadata, while the installed Python module necessarily ships the
same source; revisit if any adapter input becomes secret-bearing or
non-public.
- **Credential redaction parity:** `PASS`/`PASSWD`,
quoted/space-separated assignments, punctuation-bearing values, and
bounded camel/acronym aliases now share the same fail-closed policy
across the Bash wrapper, managed Python runtime, observability scrubber,
config filter, full/sensitive-text redactors, structured-log classifier,
TUI sanitizer, and E2E redactors. The separator lookbehind is capped at
32 horizontal characters to prevent attacker-controlled scans;
private-key blocks are scrubbed before assignment matching. Positive
tests cover `customPass`, `DBPass`, and known secret `*Key` families,
while `COMPASS`/`BYPASS`, `TOPSECRET`/`SUBTOKEN`, pass-rate fields,
`publicKey`, and `customKey` remain untouched.
- **OpenShell TLS key provenance:** the canonical mounted path is
intentionally accepted only from the supervisor-owned runtime
environment and rejected from the mutable DCode `.env`. The split
credential suite now proves both sides explicitly, matching the existing
wrapper-identity coverage; allowing it in `.env` would weaken the
boundary.
- **Docker auth cleanup:** the shared workflow validator requires
exactly one canonical cleanup with `if: always()` as the final job step.
A DCode-specific mutation test now also rejects moving cleanup before
the import gate.
- **Private-key and fixture helpers:** multiline private-key matching is
consolidated into the live generic matcher with a required-newline mode,
preserving comment behavior while removing 12 lines. Profile-hash
fixture replacement is now whitespace/quote tolerant while still
requiring one exact reviewed constant and digest. A focused regression
covers both formatting variants and duplicate-definition rejection; use
an AST transform only if the current two-constant scope grows.

## 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
- [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: public CLI, configuration,
model IDs, and user-visible behavior are unchanged; the existing DCode
quickstart is implementation-neutral.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — the prior security and supply-chain approval is
#6431 (review);
fresh exact-head re-review will be requested after the current full
fan-out because the head changed.
- [x] Non-success, skipped, or missing CI check accepted by maintainer —
`e2e-all` baseline failures accepted in
#6431 (review);
follow-ups #6381/#6384 and #6467/#6474.

## Verification
- [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 — `npm run check:diff` passed on
`e78d3ef7`; the independently runnable image/runtime and
credential-boundary suites passed 18 of 18 and 123 of 123; the final
cross-surface security/parity audit passed 96 of 96; fresh-cache
real-wheel validation and the isolated three-package import probe
passed.
- [ ] Applicable broad gate passed — exact-head focused DCode run
[28971629788](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629788)
and cloud-onboard run
[28971629739](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629739)
passed on `e78d3ef7`; full fan-out run
[28972330045](https://github.com/NVIDIA/NemoClaw/actions/runs/28972330045)
is in progress. The prior full run
[28919686103](https://github.com/NVIDIA/NemoClaw/actions/runs/28919686103)
passed 77 of 79 applicable jobs; its two failures reproduced identically
on retry and `main` run
[28911441118](https://github.com/NVIDIA/NemoClaw/actions/runs/28911441118),
with the prior maintainer waiver recorded
[here](#6431 (review)).
- [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)

## Exact-head advisor evidence

- [`ubuntu-repo-cloud-langchain-deepagents-code` run
28971629788](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629788)
passed on exact head `e78d3ef7f43f3242486dedc4b5b2b42e0585d041`. The
production-image validator covered plugin discovery and
installed-distribution binding, official source hashes, both aliases and
all 12 middleware entries, unrelated-model isolation, graph compilation,
and allowed/denied execute dispatch parity.
- The same exact-head run passed the real-Docker stripped-dependency
import gate before live E2E, then passed image version checks
(`deepagents-code==0.1.34`, `deepagents==0.7.0a6`), direct and
login-shell headless `PONG`, and interactive TUI acceptance with the
optional name prompt and no model picker.
- The advisor-required [`cloud-onboard` run
28971629739](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629739)
also passed on that exact SHA.
- CodeRabbit is green with no unresolved threads. Exact-head advisor run
28971565095 reported zero GPT findings but requested the runtime
evidence above; Nemotron's two attempts were non-advisory JSON-parse
failures. Both advisors will be rerun against this updated evidence.
- The localized import-gate removal condition is tracked in #6424 rather
than a new cleanup issue.
- Exact-head full fan-out run
[28972330045](https://github.com/NVIDIA/NemoClaw/actions/runs/28972330045)
is in progress; the prior baseline waiver remains applicable only if the
same two unrelated failures recur.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

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

* **New Features**
* Added a first-party Nemotron 3 Ultra profile plugin that registers
managed model aliases.
* **Bug Fixes / Security**
* Strengthened fail-closed validation for the released profile,
including integrity checks and managed vs native dispatch parity (with
denied-shell behavior).
* Hardened secret/credential detection and redaction so `PASS`-keyed
values are treated as sensitive.
* **CI / Quality**
* Added build-time and workflow-boundary checks ensuring images reject
missing base dependencies.
* **Tests**
* Expanded plugin/profile-contract, image behavior, and
end-to-end/workflow coverage.
* **Chores**
* Updated the container build flow to install and validate the plugin
artifact at build time, removing the standalone patch approach.
<!-- 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>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
@cjagwani cjagwani mentioned this pull request Jul 9, 2026
21 tasks
cv pushed a commit that referenced this pull request Jul 9, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Prepares the user documentation for NemoClaw v0.0.78 by replacing the
unreleased section with release highlights and synchronizing the
affected inference, lifecycle, messaging, and CLI reference pages with
merged behavior.

## Changes

- Publish the v0.0.78 release-notes section with links to the most
specific user guides for each shipped behavior.
- Document authoritative Deep Agents route health, Nemotron Ultra
profile behavior, and Hermes compatible-endpoint context metadata.
- Document forced rebuild recovery after total backup failure and the
ownership-safe tunnel/full-stop behavior.
- Keep command examples and shared agent variants aligned with the
current OpenClaw, Hermes, and Deep Agents interfaces.

Source mapping:

- [#3787](#3787) ->
`docs/about/release-notes.mdx`: Record reliable workspace template
seeding during sandbox startup.
- [#4960](#4960) ->
`docs/about/release-notes.mdx`: Record safer detection of rewritten
OpenClaw gateway processes.
- [#5676](#5676) ->
`docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON
handling.
- [#5857](#5857) ->
`docs/about/release-notes.mdx`: Record synchronization of explicit
OpenClaw main-agent model state.
- [#5929](#5929) ->
`docs/about/release-notes.mdx`: Record copyable SSH port-forward
guidance for remote dashboards.
- [#6068](#6068) ->
`docs/about/release-notes.mdx`: Record custom-image plugin provenance
reconciliation.
- [#6116](#6116) ->
`docs/about/release-notes.mdx`: Record live-loopback dashboard-forward
recovery.
- [#6122](#6122) ->
`docs/about/release-notes.mdx`: Announce validated, round-trippable
policy YAML output.
- [#6211](#6211) ->
`docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`,
`docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild
--force` recovery boundary.
- [#6283](#6283) ->
`docs/about/release-notes.mdx`: Record Hermes WebUI port alignment.
- [#6293](#6293) ->
`docs/inference/switch-inference-providers.mdx`,
`docs/about/release-notes.mdx`: Document compatible-endpoint
context-length probing for Hermes.
- [#6320](#6320) ->
`docs/about/release-notes.mdx`: Record bounded gateway-recovery waits.
- [#6377](#6377) ->
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain
rebuild diagnostics and prepared MCP-destroy recovery.
- [#6412](#6412) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document authoritative agent-visible
inference route health.
- [#6421](#6421) ->
`docs/about/release-notes.mdx`: Record the longer quiet-pull window for
managed vLLM images.
- [#6431](#6431) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document the version-pinned Nemotron
Ultra profile plugin.
- [#6439](#6439) ->
`docs/about/release-notes.mdx`: Summarize the authenticated, pinned
credential-capture helper boundary.
- [#6450](#6450) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document
host-forward cleanup and ownership-safe gateway-port release.
- [#6474](#6474) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/about/release-notes.mdx`: Record composable OpenClaw messaging
runtime loaders.
- [#6475](#6475) ->
`docs/about/release-notes.mdx`: Record removal of the unavailable Kimi
K2.6 production endpoint option.
- [#6480](#6480) ->
`docs/about/release-notes.mdx`: Record stderr routing for the plugin
registration banner.
- [#6481](#6481) ->
`docs/about/release-notes.mdx`: Record post-pull Ollama model discovery
checks.
- [#6482](#6482) ->
`docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon
restart.
- [#6486](#6486) ->
`docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep
Agents auto-approval boundary.
- [#6490](#6490) ->
`docs/about/release-notes.mdx`: Record diagnostics for custom images
missing the managed runtime.
- [#6494](#6494) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document nonempty tool-call content
preservation and placeholder rejection.
- [#6497](#6497) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document isolated Deep Agents
route-probe output.
- [#6506](#6506) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document observability-preserving
managed route probes.
- [#6508](#6508) ->
`docs/about/release-notes.mdx`: Link the new extension taxonomy and
SDK-readiness reference from the release summary.

Release-source verification: GitHub reports all 29 cited source PRs as
merged with base `main`, and every merge commit is an ancestor of
`origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No
source-mapping mismatches were found.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: Documentation-only
release-prep changes; `npm run docs` validates variants, routes, and
Fern content.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:
- [ ] 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: Tests
are not applicable to this documentation-only change set.
- [ ] 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) — exited
0 with zero errors; Fern reported the existing unauthenticated
redirect-check and light-mode contrast warnings.
- [x] 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: Charan Jagwani <cjagwani@nvidia.com>

---------

Signed-off-by: cjagwani <cjagwani@nvidia.com>
@wscurran wscurran added area: messaging Messaging channels, bridges, manifests, or channel lifecycle bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior integration: slack Slack integration or channel behavior integration: whatsapp WhatsApp integration or channel behavior labels Jul 12, 2026
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Replaces the WhatsApp compact-QR preload's asynchronous Node loader with
a synchronous hook so it composes safely with the Slack runtime guard.
It also makes an empty installed-runtime proof fail with
already-redacted loader diagnostics instead of hiding the cause.
Reported by @ericksoa.

## Related Issue
Fixes NVIDIA#6467.

## Changes
- Register the reviewed WhatsApp QR source rewrite through
`Module.registerHooks()` while preserving its SHA-256/preimage checks,
supported source-type boundary, and fail-closed behavior.
- Require OpenClaw channel-list evidence and include redacted stderr
when the Slack proof emits no valid stdout record.
- Cover Slack-plus-WhatsApp loader composition, stdout-only proof
parsing, and mandatory channel-list output.

## 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. -->
- [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: This restores the existing
documented Slack and WhatsApp behavior; only internal loader composition
and CI diagnostics change. An independent documentation review found the
existing channel docs 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: Independent diff review
and the GPT-5.5 PR Review Advisor found no blocking correctness or
security issues. The reviewed-source integrity check, fail-closed
behavior, credential assertions, policy assertions, and redaction
boundary remain intact. A thrown `registerHooks()` call is intentionally
reported by failure class without reflecting arbitrary upstream
exception text into logs; Node owns that source boundary, the regression
test distinguishes unavailable from failed registration, and the
fallback can be removed if the source rewrite is no longer needed.
- [ ] 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: 29
CLI QR-loader tests, 7 Slack composition integration tests, and 23
E2E-support proof tests passed (59/59). Compiled production preloads
also loaded successfully in both orders on Node 22.16.0. Exact-head
Ubuntu `messaging-providers` passed consecutively in [run
1](https://github.com/NVIDIA/NemoClaw/actions/runs/28964320005/job/85943544749)
and [run
2](https://github.com/NVIDIA/NemoClaw/actions/runs/28965663864/job/85948136320);
the real-renderer
[`whatsapp-qr-compact-e2e`](https://github.com/NVIDIA/NemoClaw/actions/runs/28964320469/job/85943482607)
also passed.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: `npm test` was attempted
with sandbox restrictions removed, but this macOS host cannot satisfy
unrelated Linux ownership/capability, Docker, GNU `stat -c`, and Python
>=3.10 test prerequisites. Ubuntu PR CI passed its build, type-check,
test shards, installer, WSL, and macOS jobs; the exact-head Ubuntu live
acceptance lane passed twice as linked above.
- [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: Apurv Kumaria <akumaria@nvidia.com>

---------

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…VIDIA#6431)

<!-- markdownlint-disable MD041 -->
## Summary
Replaces NemoClaw's build-time mutation of the released Deep Agents
bootstrap with a first-party `deepagents.harness_profiles` plugin for
`deepagents-code==0.1.34` / `deepagents==0.7.0a6`. The two managed
OpenAI-compatible model keys continue to receive the released native
Nemotron 3 Ultra profile, with exact version/source gates and no
third-party source changes.

## Related Issue
Fixes NVIDIA#6424

## Changes
- Add and install `nemoclaw-deepagents-profile==0.1.0` through Deep
Agents' supported profile entry-point lifecycle.
- Register only the two NemoClaw-managed aliases against the released
canonical Ultra profile, atomically and idempotently.
- Fail the image build on missing or unimportable dependencies,
mismatched distribution/package roots, copied/installed adapter-source
drift, or released-profile/bootstrap drift.
- Run a DCode-only negative Docker build from the current hash-locked
base, strip both upstream distributions, and prove failure occurs at the
isolated import gate before the later dependency check.
- Build and install a real unreviewed-version plugin wheel and prove the
actual validator rejects it.
- Verify entry-point discovery, all 12 middleware entries,
unrelated-model isolation, graph compilation, and allowed/denied
execute-dispatch parity against the official wheels.
- Split image/runtime and credential-boundary contracts into balanced
756/755-line suites with a 113-line shared helper, preserving all 75
original tests and substantial per-file size headroom.
- Remove the installed-bootstrap patcher and document that the adapter
must be removed, not rehashed, once reviewed dependencies provide both
exact aliases.
- Preserve the merged DCode hardening and paced `/agents` first-run TUI
behavior from NVIDIA#6410 / NVIDIA#6418.

## Automated review dispositions

- **License metadata:** the production package keeps the PEP 639 SPDX
string and builds unchanged with lock-pinned `setuptools==82.0.1`; the
production validator now requires exact installed-wheel
`License-Expression: Apache-2.0` metadata, with a negative
metadata-drift test. The legacy conversion is a localized offline
wrong-version fixture with explicit source-boundary and
removal-condition documentation. Remove the fixture-only conversion once
runner setuptools accepts PEP 639 strings; production never uses it.
- **Plain-progress build output:** plain progress remains necessary to
prove the exact import-failure marker. Before Docker runs, the gate now
rejects every Docker `ARG` name outside a complete reviewed allowlist,
while tests pin the only passed build arguments to the two public
`BASE_IMAGE` references. Behavior tests inject unreviewed uppercase,
lowercase, and continued ARG declarations across all three Dockerfiles
and prove rejection occurs before any build; the targeted DCode E2E job
runs the same script with real Docker before live tests.
- **Adapter build-layer retention:** Docker can retain the copied
project tree in an image layer or failed local build cache. This is
accepted because it contains only public, first-party Apache-2.0 source
and metadata, while the installed Python module necessarily ships the
same source; revisit if any adapter input becomes secret-bearing or
non-public.
- **Credential redaction parity:** `PASS`/`PASSWD`,
quoted/space-separated assignments, punctuation-bearing values, and
bounded camel/acronym aliases now share the same fail-closed policy
across the Bash wrapper, managed Python runtime, observability scrubber,
config filter, full/sensitive-text redactors, structured-log classifier,
TUI sanitizer, and E2E redactors. The separator lookbehind is capped at
32 horizontal characters to prevent attacker-controlled scans;
private-key blocks are scrubbed before assignment matching. Positive
tests cover `customPass`, `DBPass`, and known secret `*Key` families,
while `COMPASS`/`BYPASS`, `TOPSECRET`/`SUBTOKEN`, pass-rate fields,
`publicKey`, and `customKey` remain untouched.
- **OpenShell TLS key provenance:** the canonical mounted path is
intentionally accepted only from the supervisor-owned runtime
environment and rejected from the mutable DCode `.env`. The split
credential suite now proves both sides explicitly, matching the existing
wrapper-identity coverage; allowing it in `.env` would weaken the
boundary.
- **Docker auth cleanup:** the shared workflow validator requires
exactly one canonical cleanup with `if: always()` as the final job step.
A DCode-specific mutation test now also rejects moving cleanup before
the import gate.
- **Private-key and fixture helpers:** multiline private-key matching is
consolidated into the live generic matcher with a required-newline mode,
preserving comment behavior while removing 12 lines. Profile-hash
fixture replacement is now whitespace/quote tolerant while still
requiring one exact reviewed constant and digest. A focused regression
covers both formatting variants and duplicate-definition rejection; use
an AST transform only if the current two-constant scope grows.

## 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
- [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: public CLI, configuration,
model IDs, and user-visible behavior are unchanged; the existing DCode
quickstart is implementation-neutral.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — the prior security and supply-chain approval is
NVIDIA#6431 (review);
fresh exact-head re-review will be requested after the current full
fan-out because the head changed.
- [x] Non-success, skipped, or missing CI check accepted by maintainer —
`e2e-all` baseline failures accepted in
NVIDIA#6431 (review);
follow-ups NVIDIA#6381/NVIDIA#6384 and NVIDIA#6467/NVIDIA#6474.

## Verification
- [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 — `npm run check:diff` passed on
`e78d3ef7`; the independently runnable image/runtime and
credential-boundary suites passed 18 of 18 and 123 of 123; the final
cross-surface security/parity audit passed 96 of 96; fresh-cache
real-wheel validation and the isolated three-package import probe
passed.
- [ ] Applicable broad gate passed — exact-head focused DCode run
[28971629788](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629788)
and cloud-onboard run
[28971629739](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629739)
passed on `e78d3ef7`; full fan-out run
[28972330045](https://github.com/NVIDIA/NemoClaw/actions/runs/28972330045)
is in progress. The prior full run
[28919686103](https://github.com/NVIDIA/NemoClaw/actions/runs/28919686103)
passed 77 of 79 applicable jobs; its two failures reproduced identically
on retry and `main` run
[28911441118](https://github.com/NVIDIA/NemoClaw/actions/runs/28911441118),
with the prior maintainer waiver recorded
[here](NVIDIA#6431 (review)).
- [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)

## Exact-head advisor evidence

- [`ubuntu-repo-cloud-langchain-deepagents-code` run
28971629788](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629788)
passed on exact head `e78d3ef7f43f3242486dedc4b5b2b42e0585d041`. The
production-image validator covered plugin discovery and
installed-distribution binding, official source hashes, both aliases and
all 12 middleware entries, unrelated-model isolation, graph compilation,
and allowed/denied execute dispatch parity.
- The same exact-head run passed the real-Docker stripped-dependency
import gate before live E2E, then passed image version checks
(`deepagents-code==0.1.34`, `deepagents==0.7.0a6`), direct and
login-shell headless `PONG`, and interactive TUI acceptance with the
optional name prompt and no model picker.
- The advisor-required [`cloud-onboard` run
28971629739](https://github.com/NVIDIA/NemoClaw/actions/runs/28971629739)
also passed on that exact SHA.
- CodeRabbit is green with no unresolved threads. Exact-head advisor run
28971565095 reported zero GPT findings but requested the runtime
evidence above; Nemotron's two attempts were non-advisory JSON-parse
failures. Both advisors will be rerun against this updated evidence.
- The localized import-gate removal condition is tracked in NVIDIA#6424 rather
than a new cleanup issue.
- Exact-head full fan-out run
[28972330045](https://github.com/NVIDIA/NemoClaw/actions/runs/28972330045)
is in progress; the prior baseline waiver remains applicable only if the
same two unrelated failures recur.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

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

* **New Features**
* Added a first-party Nemotron 3 Ultra profile plugin that registers
managed model aliases.
* **Bug Fixes / Security**
* Strengthened fail-closed validation for the released profile,
including integrity checks and managed vs native dispatch parity (with
denied-shell behavior).
* Hardened secret/credential detection and redaction so `PASS`-keyed
values are treated as sensitive.
* **CI / Quality**
* Added build-time and workflow-boundary checks ensuring images reject
missing base dependencies.
* **Tests**
* Expanded plugin/profile-contract, image behavior, and
end-to-end/workflow coverage.
* **Chores**
* Updated the container build flow to install and validate the plugin
artifact at build time, removing the standalone patch approach.
<!-- 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>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@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

Prepares the user documentation for NemoClaw v0.0.78 by replacing the
unreleased section with release highlights and synchronizing the
affected inference, lifecycle, messaging, and CLI reference pages with
merged behavior.

## Changes

- Publish the v0.0.78 release-notes section with links to the most
specific user guides for each shipped behavior.
- Document authoritative Deep Agents route health, Nemotron Ultra
profile behavior, and Hermes compatible-endpoint context metadata.
- Document forced rebuild recovery after total backup failure and the
ownership-safe tunnel/full-stop behavior.
- Keep command examples and shared agent variants aligned with the
current OpenClaw, Hermes, and Deep Agents interfaces.

Source mapping:

- [NVIDIA#3787](NVIDIA#3787) ->
`docs/about/release-notes.mdx`: Record reliable workspace template
seeding during sandbox startup.
- [NVIDIA#4960](NVIDIA#4960) ->
`docs/about/release-notes.mdx`: Record safer detection of rewritten
OpenClaw gateway processes.
- [NVIDIA#5676](NVIDIA#5676) ->
`docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON
handling.
- [NVIDIA#5857](NVIDIA#5857) ->
`docs/about/release-notes.mdx`: Record synchronization of explicit
OpenClaw main-agent model state.
- [NVIDIA#5929](NVIDIA#5929) ->
`docs/about/release-notes.mdx`: Record copyable SSH port-forward
guidance for remote dashboards.
- [NVIDIA#6068](NVIDIA#6068) ->
`docs/about/release-notes.mdx`: Record custom-image plugin provenance
reconciliation.
- [NVIDIA#6116](NVIDIA#6116) ->
`docs/about/release-notes.mdx`: Record live-loopback dashboard-forward
recovery.
- [NVIDIA#6122](NVIDIA#6122) ->
`docs/about/release-notes.mdx`: Announce validated, round-trippable
policy YAML output.
- [NVIDIA#6211](NVIDIA#6211) ->
`docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`,
`docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild
--force` recovery boundary.
- [NVIDIA#6283](NVIDIA#6283) ->
`docs/about/release-notes.mdx`: Record Hermes WebUI port alignment.
- [NVIDIA#6293](NVIDIA#6293) ->
`docs/inference/switch-inference-providers.mdx`,
`docs/about/release-notes.mdx`: Document compatible-endpoint
context-length probing for Hermes.
- [NVIDIA#6320](NVIDIA#6320) ->
`docs/about/release-notes.mdx`: Record bounded gateway-recovery waits.
- [NVIDIA#6377](NVIDIA#6377) ->
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain
rebuild diagnostics and prepared MCP-destroy recovery.
- [NVIDIA#6412](NVIDIA#6412) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document authoritative agent-visible
inference route health.
- [NVIDIA#6421](NVIDIA#6421) ->
`docs/about/release-notes.mdx`: Record the longer quiet-pull window for
managed vLLM images.
- [NVIDIA#6431](NVIDIA#6431) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document the version-pinned Nemotron
Ultra profile plugin.
- [NVIDIA#6439](NVIDIA#6439) ->
`docs/about/release-notes.mdx`: Summarize the authenticated, pinned
credential-capture helper boundary.
- [NVIDIA#6450](NVIDIA#6450) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document
host-forward cleanup and ownership-safe gateway-port release.
- [NVIDIA#6474](NVIDIA#6474) ->
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/about/release-notes.mdx`: Record composable OpenClaw messaging
runtime loaders.
- [NVIDIA#6475](NVIDIA#6475) ->
`docs/about/release-notes.mdx`: Record removal of the unavailable Kimi
K2.6 production endpoint option.
- [NVIDIA#6480](NVIDIA#6480) ->
`docs/about/release-notes.mdx`: Record stderr routing for the plugin
registration banner.
- [NVIDIA#6481](NVIDIA#6481) ->
`docs/about/release-notes.mdx`: Record post-pull Ollama model discovery
checks.
- [NVIDIA#6482](NVIDIA#6482) ->
`docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon
restart.
- [NVIDIA#6486](NVIDIA#6486) ->
`docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep
Agents auto-approval boundary.
- [NVIDIA#6490](NVIDIA#6490) ->
`docs/about/release-notes.mdx`: Record diagnostics for custom images
missing the managed runtime.
- [NVIDIA#6494](NVIDIA#6494) ->
`docs/inference/model-capability-audit.mdx`,
`docs/about/release-notes.mdx`: Document nonempty tool-call content
preservation and placeholder rejection.
- [NVIDIA#6497](NVIDIA#6497) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document isolated Deep Agents
route-probe output.
- [NVIDIA#6506](NVIDIA#6506) ->
`docs/get-started/quickstart-langchain-deepagents-code.mdx`,
`docs/about/release-notes.mdx`: Document observability-preserving
managed route probes.
- [NVIDIA#6508](NVIDIA#6508) ->
`docs/about/release-notes.mdx`: Link the new extension taxonomy and
SDK-readiness reference from the release summary.

Release-source verification: GitHub reports all 29 cited source PRs as
merged with base `main`, and every merge commit is an ancestor of
`origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No
source-mapping mismatches were found.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: Documentation-only
release-prep changes; `npm run docs` validates variants, routes, and
Fern content.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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:
- [ ] 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: Tests
are not applicable to this documentation-only change set.
- [ ] 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) — exited
0 with zero errors; Fern reported the existing unauthenticated
redirect-check and light-mode contrast warnings.
- [x] 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: Charan Jagwani <cjagwani@nvidia.com>

---------

Signed-off-by: cjagwani <cjagwani@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: messaging Messaging channels, bridges, manifests, or channel lifecycle bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior integration: slack Slack integration or channel behavior integration: whatsapp WhatsApp integration or channel behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(e2e): fix empty OpenClaw Slack runtime proof

3 participants