Add diagnostics OTEL capability contract tests#92045
Merged
vincentkoc merged 1 commit intoJun 14, 2026
Merged
Conversation
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 11:25 PM ET / 03:25 UTC. Summary PR surface: Tests +49. Total +49 across 2 files. Reproducibility: not applicable. This PR adds contract coverage and does not report a failing current-main user workflow. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the focused contract tests after normal maintainer review while keeping the existing runtime trust policy unchanged. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds contract coverage and does not report a failing current-main user workflow. Is this the best way to solve the issue? Yes. Separate registry and service-gate tests are a narrow maintainable way to protect the official-install trust classification and downstream capability grant. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against bd10e1998be0. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Tests +49. Total +49 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
b1ffc2c to
a4a2807
Compare
Member
|
Maintainer verification for
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
diagnostics-otelpackage when it is selected from config and matches an installed official package record.diagnostics-otelreceivesinternalDiagnosticswhen the registry classifies the config-selected official install as trusted, while untrusted/spoofed services remain denied.Verification
git diff --checkpnpm exec oxfmt --check --threads=1 src/plugins/services.test.ts src/plugins/manifest-registry.test.tsnode scripts/run-vitest.mjs run --config test/vitest/vitest.plugins.config.ts src/plugins/services.test.ts -t "grants internal diagnostics only to trusted diagnostics exporter services"node scripts/run-vitest.mjs run --config test/vitest/vitest.plugins.config.ts src/plugins/manifest-registry.test.ts -t "marks official diagnostics-otel config paths trusted when the install record matches"Notes:
origin/mainalready has part of the service-side diagnostics coverage, but not the config-pathdiagnostics-oteltrusted official install case added here.vitest.unit.config.tsfocused command now excludessrc/plugins/**; the plugin-scoped wrapper config above is the current repo-approved targeted command for these files.Real behavior proof
Behavior addressed: Official
diagnostics-otelinstalls selected from config should be classified as trusted when their install record matches, so the diagnostics exporter service receives the internal diagnostics hooks.Real environment tested: Local OpenClaw source checkout on the PR branch with repository runtime modules loaded through
node --import tsx; no Vitest harness or mocks in this proof command.Exact steps or command run after this patch: Ran a source-checkout Node command that writes a temporary
diagnostics-otelplugin manifest, callsloadPluginManifestRegistrywith a matching official npm install record, then starts adiagnostics-otelplugin service throughstartPluginServicesusing the registry record'strustedOfficialInstallclassification.Evidence after fix: Terminal output from the local source-checkout proof command:
{ "manifestId": "diagnostics-otel", "manifestOrigin": "config", "manifestTrustedOfficialInstall": true, "serviceInternalDiagnosticsOnEvent": "function", "serviceInternalDiagnosticsEmit": "function" }Observed result after fix: The config-selected
diagnostics-otelmanifest record was trusted (manifestTrustedOfficialInstall: true), and the started service context exposed both internal diagnostics hooks as functions.What was not tested: Full packaged install in a user runtime and live end-to-end OTEL export were not run for this test-only PR; the branch adds focused contract tests plus the source-checkout proof above.