Skip to content

fix: handle circular arrays in example output redaction - #22

Merged
steipete merged 2 commits into
openclaw:mainfrom
embrace897:codex/fix-circular-array-output
Nov 5, 2026
Merged

steipete merged 2 commits into
openclaw:mainfrom
embrace897:codex/fix-circular-array-output

Conversation

@embrace897

@embrace897 embrace897 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Fix self-referencing arrays in the existing standalone output redactors. Arrays now enter the existing visited-object set before traversal, so JSON output retains sensitive-field masking and emits [Circular] instead of overflowing the stack. Recipe, Quickstart, Coding Agent CLI, and Workbench copies stay synchronized.

Also patch Vitest to 4.1.12 to resolve both open mocker advisories and refresh compatible React/Vite, icon, type, and lint dependencies after the two-day cooldown. Preserve the pinned package manager, TypeScript tooling, and security overrides.

Validation:

  • All four original helpers reproduced RangeError; all four fixed helpers produced the expected serializable, redacted JSON. The recipe runMain output entrypoint passed with synthetic cyclic input.
  • Full Linux pnpm check passed: 27 tests, formatting, typecheck, lint, docs, every starter build/typecheck, and offline CLI smoke commands.
  • Independent review found no actionable P0–P2 issues.
  • The original Windows environment had two spawn pnpm ENOENT test failures and a symlink EPERM; this work does not claim those host limitations are fixed. No live Gateway or browser proof is claimed.

No project version bump, tag, or release.

@clawsweeper

clawsweeper Bot commented Nov 4, 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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 Nov 4, 2026
@clawsweeper

clawsweeper Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

The PR registers arrays in the existing visited-object set before recursively redacting their elements.

Example: An output array contains { token: "synthetic-secret", text: "visible" } followed by itself.

  • Before: Redaction recurses until a RangeError prevents JSON output.
  • After: Output is [{"token":"[REDACTED]","text":"visible"},"[Circular]"].

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The narrow repair has concrete output-path proof and meaningful regression coverage; full Linux validation remains unreported.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The updated body records head's production runMain output path on Node v24.15.0 receiving the synthetic self-referencing array and printing serializable output with the token masked, a circular marker, and exit code zero; historical Workbench screenshots provide only prior-design context.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A self-referencing array passed to an exported output helper prevents printable, redacted JSON.
Reason: The small repair extends existing object-cycle handling to arrays without new configuration, dependencies, or machinery.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: main still has the circular-array defect, and this focused patch fixes it without an identified correctness regression.

Priority: P3
Reviewed head: f935324b79c34f15f359292bec52cb9222954c7c

Before merge

None.

Findings

None.

Agent review details

How this fits together

Recipe results, CLI structured output, and Workbench results pass through local redactors before JSON reaches the console or Result panel.

flowchart TD
  A[Recipe results] --> D[Local redactors]
  B[CLI structured output] --> D
  C[Workbench results] --> D
  D --> E[Visited object tracking]
  E --> F[Sensitive field masking]
  F --> G[JSON output]
Loading

Technical review

Best possible solution:

Retain the focused change applying existing cycle tracking to arrays in each intentionally standalone redactor.

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

Main deterministically recurses on an array containing itself; the contributor also reports base-failing regressions, while this read-only review executed no target code.

Is this the best way to solve the issue?

Registering arrays before traversal fixes the cause using the existing WeakSet and preserves the established standalone ownership and masking policy.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 6dc6b2349474.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

Likely related people:

  • Vincent Koc: Raw commit ed63cf9 adds recipes/_shared/run-main.ts:21 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: ed63cf9d483b; files: recipes/_shared/run-main.ts)
  • Sebastien Tardif: Raw commit 34c2c26 adds sdk/agent-workbench/src/redact-sensitive-output.ts:2 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 34c2c26c000e; files: sdk/agent-workbench/src/redact-sensitive-output.ts)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Review metrics

Metric Value Why it matters
Patch size Production +12/-12; tests +15/-0 Production changes reorder existing traversal; regression coverage checks the distinct failure across four intentional copies.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P3: This is a small robustness repair for synthetic cyclic input, with no observed cyclic SDK response or affected live workflow.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Obtain a successful Cookbook and examples check for the reviewed head.

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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-11-04T08:39:14.059Z sha f935324 :: needs real behavior proof before merge. :: none

Reviewed November 5, 2026, 3:21 AM ET / 08:21 UTC (Revision 2).

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Nov 5, 2026
@steipete

steipete commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Maintainer validation at 9adec5d: the circular-array input reproduces RangeError in all four original helpers; all four corrected helpers return serializable JSON with nested token redaction and a circular marker. The recipe runMain output entrypoint also passed with the synthetic cyclic result.

The full Linux pnpm check passed on an isolated lease: 27 tests, formatting, typecheck, lint (zero warnings/errors), docs, every starter build/typecheck, and both offline CLI smoke commands. Independent review found no actionable P0–P2 issues. This adds Linux validation; it does not claim the originally reported Windows environment failures are fixed or that a live Gateway was tested.

Also update Vitest to 4.1.12 to fix both open mocker advisories and refresh compatible starter dependencies after the configured two-day cooldown. Retain pinned pnpm/TypeScript tooling, esbuild security override, and Vite override (updated to 8.3.5). No project version bump or publication.

@steipete
steipete merged commit 1dec557 into openclaw:main Nov 5, 2026
3 of 4 checks passed
@steipete

steipete commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Merged as 1dec557 after all three exact-head checks passed, including Cookbook and examples. Maintainer proof also passed all 27 tests, lint, formatting, typecheck, docs, builds, offline CLI smokes and synthetic circular-output entrypoint checks on Linux. Independent review was clean through P2. The dispatch HTTP 500 cleared on its one diagnosed infrastructure retry.

Contributor credit is preserved in the changelog and squash commit. Main CI and the current-version non-publishing release dry run are now verifying the landed commit.

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

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. 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.

2 participants