Skip to content

fix: decode raw JSON locator paths and report field errors - #52

Merged
steipete merged 3 commits into
mainfrom
fix/raw-json-protocol
Aug 29, 2026
Merged

fix: decode raw JSON locator paths and report field errors#52
steipete merged 3 commits into
mainfrom
fix/raw-json-protocol

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

The report in #51 exposes two problems hidden by the generic raw JSON error. The human CLI names tree/find are not protocol commands (collectAll/query), and their app/depth options are named application/max_depth in JSON. Valid query and action payloads decode on both v0.1.6 and current main; this is not an all-command v0.1.6 regression.

There are reproducible locator defects: synthesized decoding requires criteria even for a documented path-only locator, and the CLI's snake-case decoder transforms path_from_root into a key that the locator never reads. A malformed array entry also loses its actual error when the CLI retries the entire input as an object.

This change decodes path-only locators, preserves navigation hints under both default and snake-case decoding, retains the published path_from_root encoding, and reports the failing field path without replacing array-entry errors. README examples and raw help explain the CLI/protocol mapping. It adds no command aliases and does not change action implementations.

Proof:

  • Built and exercised the v0.1.6 tag (dba24f0) and unmodified main (87242ce) with the real CLI across --json, --stdin, and --file. Valid non-ping commands reach dispatch; path-only locators fail and malformed paths are ignored before the fix.
  • Verified the published v0.1.6 universal archive's checksum and signature; both ARM and Intel slices reproduce the same results. Intel execution used Rosetta on macOS 26, not an Intel Sonoma machine.
  • New regression tests fail before the fix and pass afterward. The wire matrix covers all 18 recognized command names through all three input sources, plus nested batch errors, array-entry diagnostics, empty arrays, and BOM-prefixed arrays. Locator tests verify navigation survives command conversion and public Codable round trips.
  • Full swift test -j 4 passed (207 tests reported in the main test target; 5 in the conversion target). Eighteen opt-in UI automation tests were skipped. Independent live Dock query/collectAll reads succeeded before and after the fix, including both slices of the fixed universal binary.
  • make check with the repository-pinned SwiftFormat/SwiftLint, shellcheck scripts/*.sh, and scripts/build-release-artifact.sh 0.1.6 --adhoc passed. Verified archive contents/checksum, both architectures, code signature, version output, and generated Homebrew formula syntax. The artifact was local validation only; no release was published.
  • Codex autoreview completed with no actionable findings at its configured P0 threshold.

Fixes #51

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@cursor

cursor Bot commented Aug 29, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are limited to JSON decoding, CLI error text, and documentation; they fix documented wire behavior without altering accessibility action implementations.

Overview
Fixes raw JSON locator decoding so documented path_from_root-only payloads work: criteria defaults to an empty array when omitted, and path_from_root is read under both default and snake_case key decoding (so the CLI decoder no longer drops navigation hints). Encoding still emits path_from_root.

Command parsing moves into InputHandler.decodeCommands, which accepts a single envelope or an array without re-decoding a bad array entry as a lone object—decode failures now surface field paths (including nested batch/array indices) via DecodingError human-readable messages in axorc raw responses.

README and axorc raw help document the CLI vs JSON mapping (tree/findcollectAll/query, app/depthapplication/max_depth) and locator rules. Regression tests cover the locator wire contract, all protocol commands through --json/--stdin/--file, and test helpers are marked nonisolated for serialized subprocess suites.

Reviewed by Cursor Bugbot for commit 2640158. Bugbot is set up for automated code reviews on this repo. Configure here.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. 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. labels Aug 29, 2026
@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 28, 2026, 11:33 PM ET / August 29, 2026, 03:33 UTC.

ClawSweeper review

What this changes

The branch makes raw JSON locators accept documented path-only navigation hints, preserves detailed decoding errors, clarifies protocol names, and adds regression coverage.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This collaborator-authored PR is a focused candidate fix for the still-open raw JSON decoding report. Source, tests, and the supplied real-CLI evidence support the repair; review the two in-progress checks on this exact head before merging.

Priority: P2
Reviewed head: 2640158c6617f50e6ae5e3de4755ffa71ef9b5d7

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered protocol repair with no discrete correctness finding; exact-head checks remain to be reviewed.
Proof confidence 🌊 off-meta tidepool Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR; independently, its body supplies detailed after-fix real CLI evidence through the changed raw-command entrypoints.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR; independently, its body supplies detailed after-fix real CLI evidence through the changed raw-command entrypoints.
Evidence reviewed 6 items Path-only locator repair: The custom decoder defaults omitted criteria to an empty list and reads the published path hint under both coding-key forms.
Preserved dispatch error path: Raw command decoding now selects the input container once and returns the original DecodingError’s field path.
Path-only behavior is supported downstream: The search implementation explicitly returns a navigated element when criteria are empty but a path hint is present.
Findings None None.
Security None None.

How this fits together

AXorcist’s raw command accepts JSON envelopes from arguments, standard input, or files, decodes them into accessibility commands, and returns structured results. Locator parsing feeds query/action dispatch to the Accessibility library.

flowchart LR
  Input[Raw JSON input] --> Decode[Protocol decoder]
  Decode --> Locator[Locator parsing]
  Locator --> Command[Accessibility command]
  Command --> Dispatch[AXorcist dispatch]
  Dispatch --> Result[JSON result or field error]
Loading

Before merge

  • Resolve merge risk (P1) - The exact head still has build-and-lint and Swift analysis checks in progress; their final outcome is not available in this review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +35 net, tests +216 The production change is bounded to decoding and CLI error handling, with substantially larger focused regression coverage.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the compatible decoder and error-reporting repair after the exact-head checks complete, allowing the linked raw-protocol report to close through the merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the compatible decoder and error-reporting repair after the exact-head checks complete, allowing the linked raw-protocol report to close through the merge.

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

Yes. The supplied evidence records real current-main CLI reproduction across JSON argument, standard-input, and file entrypoints, and the added tests map directly to the decoded locator and error paths.

Is this the best way to solve the issue?

Yes. The patch restores the documented wire format at the decoding boundary and preserves existing command names rather than adding duplicate aliases or changing action behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 87242ce526e4.

Labels

Label justifications:

  • P2: The PR fixes a bounded programmatic raw-protocol compatibility problem without evidence of an urgent availability or security impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR; independently, its body supplies detailed after-fix real CLI evidence through the changed raw-command entrypoints.

Evidence

What I checked:

Likely related people:

  • steipete: Current and historical commits on the affected locator and raw-command paths are authored by Peter Steinberger, including the standalone CLI introduction and this repair. (role: original CLI/protocol author and recent area contributor; confidence: high; commits: 75f56d994947, dfdfba5a32c7, 87242ce526e4; files: Sources/AXorcist/Core/MatchingTypes.swift, Sources/axorc/AXORCMain.swift, Sources/axorc/Core/InputHandler.swift)

Rank-up moves

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

  • Review the completed Swift analysis and build-and-lint results for this exact head before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-29T03:11:29.907Z sha dfdfba5 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-29T03:27:15.708Z sha d1a285e :: needs maintainer review before merge. :: none

@steipete
steipete merged commit 01098e1 into main Aug 29, 2026
9 checks passed
@steipete
steipete deleted the fix/raw-json-protocol branch August 29, 2026 03:34
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: 🐚 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.

raw JSON protocol fails with decode error for all commands except ping (v0.1.6)

1 participant