Skip to content

fix: prove scalar console diagnostic templates - #128

Merged
Patrick-Erichsen merged 1 commit into
mainfrom
fix-native-diagnostic-templates
Oct 30, 2026
Merged

Patrick-Erichsen merged 1 commit into
mainfrom
fix-native-diagnostic-templates

Conversation

@Patrick-Erichsen

Copy link
Copy Markdown
Collaborator

SkillSpector leaves scalar console diagnostics opaque when a module also writes an unrelated report, preventing the retained WCAG 2.0.22 upload from completing its required publication scan. Add affirmative scalar provenance and use evidence for a direct single-template native console argument. The proof reuses the existing bounded semantic grammar and runs lazily only for that diagnostic shape.

The diagnostic proof grants no ownership to file writes, JSON output, process arguments, or a source value’s other consumers. Unknown effects, coercion, mutation, shadowing, callbacks, unsupported grammar and executable consumers stay incomplete. The earlier FIFO/polling/later/nested file-consumer counterexamples remain covered.

Unchanged original, 21 files / 86,004 bytes Baseline 3308fba Candidate
Fully inspected files, macOS and Linux arm64 static runs 20/21 21/21
Substantive security finding fields and suppressions Preserved Identical
Missing-reference caveats 7 7

All original hashes match. Only the proven diagnostic completeness exception and its AE1 disappear. These runs use no models and execute no publisher JavaScript; production recovery will still go through the ordinary scanners, judge and publication gates after ClawHub adopts the source.

Validation: 183 focused/native/JSON controls, seven prior file-consumer controls, all 1,380 Linux adapter tests, go test -count=1 ./... and go vet ./... pass. Regenerating the transport patch reproduces all 123 source modules byte for byte. Complete frozen-module independent P0–P3 review passed with no actionable findings on the repaired V2 (Codex GPT-6.1 Sol, high/Standard). The first review caught mutable loop provenance; immutable unwritten loop/destructuring controls now reject that exact counterexample and its sibling cases.

@clawsweeper

clawsweeper Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 30, 2026
@clawsweeper

clawsweeper Bot commented Oct 30, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

The PR recognizes proven scalar JavaScript console templates without extending that proof to file output or other consumers.

Example: A module logs value: ${n} with n=1 and separately writes "{}" to report.json.

  • Before: The unrelated file write prevents the existing closed-module proof from establishing diagnostic completeness.
  • After: The diagnostic receives its own bounded scalar proof while the file write retains its separate ownership checks.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation and behavioral controls are coherent and reviewed-head CI passes; directly inspectable runtime evidence remains limited, and publication recovery is separate.
Proof confidence 🦐 gold shrimp (3/6) The collaborator-authored PR is outside the ordinary contributor proof gate; its body reports installed macOS and Linux static runs improving WCAG coverage from 20/21 to 21/21 with unchanged substantive findings, but supplies no linked logs or CLI trace.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Partial
User problem: Valid JavaScript console diagnostics remain incompletely inspected when their module also writes an unrelated report.
Reason: The source repair has clear scanner value and maintainer-authored direction; it addresses a prerequisite for the linked publication report rather than completing operational recovery.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR addresses a concrete inspection gap still present on main; source review found no actionable defect.

Priority: P2
Reviewed head: b1872c98d43c93718cc513b6ef98879ebba50482

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The reported unchanged-artifact comparison has no attached shipped-CLI trace or raw baseline-versus-candidate output.
Agent review details

How this fits together

ClawScan's Docker runtime applies an integration patch to SkillSpector, whose JavaScript ownership analysis feeds static inspection completeness; ClawScan preserves the raw report and rejects incomplete scanner results.

flowchart LR
A[Skill source] --> B[SkillSpector static scanner]
B --> C[Bounded JavaScript analysis]
C --> D[Scalar diagnostic proof]
D --> E[Inspection ledger and raw findings]
E --> F[ClawScan report validation]
Loading

Technical review

Best possible solution:

Land the bounded diagnostic proof while preserving independent consumer checks and handling retained-upload recovery through the existing operational path.

Do we have a high-confidence way to reproduce the issue?

Main's closed-module proof rejects the unrelated writeFileSync effect and lacks this diagnostic exception; the added fixture supplies a concrete input, but this review executed no target code.

Is this the best way to solve the issue?

The separate consumer-specific proof appropriately reuses the existing grammar, binding resolution, and budget without relaxing the original closed-value proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3308fba908e3.

Provenance checked

  • Individual JavaScript template ownership keeps the original intent (c0731bf: Bound shell parsing only after an individual data-use proof while preserving raw findings and conservative unsupported coverage.)
  • Closed native-value and effect proof keeps the original intent (aff544d: Require affirmative native identity, scalar input, and source-effect evidence before accepting constructed validator values.)
  • Runtime build regression registration keeps the original intent (3308fba: Run integration regression controls against the installed patched scanner during runtime construction.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: A bounded false-incompleteness case blocks valid scanner input, without evidence of a service-wide outage or urgent regression.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: A bounded false-incompleteness case blocks valid scanner input, without evidence of a service-wide outage or urgent regression.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 30, 2026, 2:11 PM ET / 18:11 UTC.

@Patrick-Erichsen

Copy link
Copy Markdown
Collaborator Author

Direct before/after CLI evidence for reviewed head b1872c98d43c93718cc513b6ef98879ebba50482:

The real Linux arm64 skillspector CLI ran against the same unchanged 21-file /86,004-byte original in an isolated, network-disabled container. The published baseline image supplied dependencies; each invocation mounted the corresponding pristine reconstructed source read-only. These are static source comparisons, rather than a claim that the new image was already published.

skillspector scan /original --no-llm --format json --output /output/wcag-linux-v2-baseline.json
Report saved to: /output/wcag-linux-v2-baseline.json
skillspector scan /original --no-llm --format json --output /output/wcag-linux-v2-candidate.json
Report saved to: /output/wcag-linux-v2-candidate.json

Projected directly from the resulting raw reports:

[
  {
    "variant": "baseline",
    "rawReportSha256": "78b0cf5454f2ed23eb01cc0343868bd45d3ee12cffb3a838d6859d297afb1f67",
    "fullyInspectedFiles": 20,
    "totalComponents": 21,
    "ledgerExceptionsByReason": {
      "opaque_content": 1,
      "reference_missing": 7
    },
    "rawIssues": 2,
    "modelsRequested": false
  },
  {
    "variant": "candidate",
    "rawReportSha256": "f368d2e3f0af5edc546223139bb9e9575774d69058a9b98d3e78813965395a9a",
    "fullyInspectedFiles": 21,
    "totalComponents": 21,
    "ledgerExceptionsByReason": {
      "reference_missing": 7
    },
    "rawIssues": 1,
    "modelsRequested": false
  }
]

Every original path/size/SHA-256 matches before and after. All non-AE1 finding fields and suppressions compare equal; seven reference-missing caveats remain. Only the diagnostic completeness exception and corresponding AE1 are removed. No publisher JavaScript or models were run. The independently reviewed product patch reconstructs all 123 candidate source modules byte for byte.

Installed runtime builds also pass on amd64 and arm64, including all 1,380 adapter controls and CLI smoke tests. Immutable runtime publication, ClawHub adoption and the original's normal production scanner/judge/publication/download/install proof remain separate follow-up gates. The linked publishing issue stays open.

@Patrick-Erichsen
Patrick-Erichsen merged commit c0b813f into main Oct 30, 2026
19 checks passed
@Patrick-Erichsen
Patrick-Erichsen deleted the fix-native-diagnostic-templates branch October 30, 2026 18:13
@Patrick-Erichsen

Copy link
Copy Markdown
Collaborator Author

Published-runtime proof for merged source c0b813f1001062257758f079795cdd176b7e31a4 (reviewed tree 70286334bc1ca0b4575fca5795eed5ec288985d1):

Normal runtime publication completed. The immutable index is ghcr.io/openclaw/clawscan-runtime@sha256:21d3caa3ee9125877ee02c9a8c2002b9c0a90886f1b06a961569ae6272482e27. Anonymous registry readback verifies the raw index digest, both native manifests, both smoke-tested config digests and the exact source/tree.

Actual Linux arm64 execution of that published image uses its installed scanner, with no source overlay:

skillspector scan /original --no-llm --format json --output /output/wcag-published-image-static.json
21/21 files fully inspected
123 installed Python source modules byte-identical to reviewed candidate
21 original file hashes preserved
raw findings and suppressions identical to qualified candidate
models invoked: false; publisher code executed: false

The first local image pull failed because the isolated VM was full. Its failure is retained. The unused baseline image was archived with its full hash and all 20 content-addressed blobs verified, then only that unused cache reference was removed without force. All ten protected running containers were verified unchanged before/after; the identical published-image retry passed.

This completes the native source/runtime prerequisite. ClawHub adoption #4157 is still under review/CI. The original's normal semantic scanners, judge, publication and anonymous download/install proof remain required; #4119 remains open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant