Skip to content

test(macos): cover root CLI command parsing#93188

Closed
TurboTheTurtle wants to merge 4 commits into
openclaw:mainfrom
TurboTheTurtle:codex/83879-cli-parser-tests
Closed

test(macos): cover root CLI command parsing#93188
TurboTheTurtle wants to merge 4 commits into
openclaw:mainfrom
TurboTheTurtle:codex/83879-cli-parser-tests

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #83879.

Adds a small root-command parser/dispatch helper for the macOS CLI and covers usage/help, known-command argument forwarding, and unknown-command dispatch.

Also refreshes the Swift protocol generator so current literal-union schemas emit the Swift enums required by the macOS package, fixes the exec approvals legacy migration copy path exposed by the macOS IPC shard, and carries the current-main Telegram unused-parameter cleanup needed for check-prod-types while #93185 is still open.

Real behavior proof

Behavior addressed: the macOS root CLI parser and help dispatch path are covered and still produce the expected root usage output.

Real environment tested: local OpenClaw source checkout on macOS, running the Swift package CLI, macOS IPC test shard, protocol guard, and production typecheck.

Exact steps or command run after this patch: ran swift run --package-path apps/macos openclaw-mac --help, focused parser tests, the macOS IPC filter that includes the parser and approvals migration tests, the native protocol guard, and pnpm tsgo:prod.

Evidence after fix:

openclaw-mac

Usage:
  openclaw-mac connect [--url <ws://host:port>] [--token <token>] [--password <password>]
  openclaw-mac configure-remote --ssh-target <user@host[:port]> [--local-port <port>]
  openclaw-mac discover [--timeout <ms>] [--json] [--include-local]
  openclaw-mac wizard [--url <ws://host:port>] [--token <token>] [--password <password>]

✔ Suite RootCommandParserTests passed after 0.001 seconds.
✔ Test run with 587 tests in 114 suites passed after 2.625 seconds.
[test] passed 1 Vitest shard in 6.79s
pnpm tsgo:prod exited 0

Observed result after fix: root help dispatch prints the macOS CLI usage, the new parser tests pass, the broader macOS IPC shard passes with the generated protocol and approvals migration fixes in place, and production TypeScript typechecking is green.

What was not tested: packaged app notarization or installer flows were not run.

@openclaw-barnacle openclaw-barnacle Bot added app: macos App: macos scripts Repository scripts proof: supplied External PR includes structured after-fix real behavior proof. size: S labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 3:31 PM ET / 19:31 UTC.

Summary
The branch extracts macOS root-command parsing into a testable Swift helper with coverage, changes the exec-approvals migration copy flags, and carries protocol-generator/output plus Telegram typecheck cleanup hunks.

PR surface: Source -1, Other +99. Total +98 across 7 files.

Reproducibility: yes. Source inspection on current main shows parseRootCommand is still private and root dispatch lacks focused parser/action coverage, while the PR adds a narrow helper and tests for those branches.

Review metrics: 2 noteworthy metrics.

  • Migration Surface: 1 legacy copy path changed. The exec approvals migration reads and rewrites persisted user approval data during upgrade, so the copy semantics matter before merge.
  • Current-Main Overlap: 2 overlapping surfaces already on main. The protocol literal-union enum fix and Telegram unused-parameter cleanup are already present on current main, explaining the dirty branch state and rebase need.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase or otherwise resolve conflicts against current main while preserving the unique parser and approvals changes.
  • Rerun the focused macOS Swift/parser and approvals migration checks on the final landing head.

Risk before merge

  • [P1] The branch is currently CONFLICTING/DIRTY against current main, so maintainers need a rebase or conflict resolution that preserves current-main protocol and Telegram fixes while keeping the unique parser and approvals changes.
  • [P1] The exec approvals legacy migration path handles persisted user approval data; a wrong copy flag or stale proof could make existing approvals disappear during upgrade even though the code change is small.

Maintainer options:

  1. Rebase And Preserve The Unique Fixes (recommended)
    Resolve the current-main conflicts by keeping the parser tests and COPYFILE_DATA migration change while dropping or preserving the already-main protocol and Telegram fixes as appropriate.
  2. Accept With Migration Proof Intact
    Maintainers may accept the bounded upgrade risk if the macOS IPC migration proof and required checks are fresh for the final landing head.
  3. Pause If Migration Ownership Is Unclear
    If no macOS owner is comfortable with the persisted approvals migration touch, pause this PR and split the parser coverage from the migration repair.

Next step before merge

  • [P2] The remaining action is maintainer conflict resolution and upgrade-sensitive migration review, not a safe automated repair lane on the current dirty branch.

Security
Cleared: No concrete security or supply-chain regression was found; the diff does not change workflows, dependencies, package resolution, or secret handling, and the approvals file remains chmod-hardened after migration.

Review details

Best possible solution:

Land a rebased branch that keeps the root parser coverage and COPYFILE_DATA migration fix, preserves current-main protocol/Telegram implementations, and reruns the macOS Swift/protocol checks after conflict resolution.

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

Yes. Source inspection on current main shows parseRootCommand is still private and root dispatch lacks focused parser/action coverage, while the PR adds a narrow helper and tests for those branches.

Is this the best way to solve the issue?

Yes after rebase. The parser seam is the narrow maintainable test boundary and COPYFILE_DATA matches the copyfile contract, but the already-main protocol and Telegram hunks should not be reintroduced during conflict resolution.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority macOS/protocol repair and test-coverage PR with limited blast radius but real build and upgrade relevance.
  • merge-risk: 🚨 compatibility: The exec approvals legacy migration change can affect existing users during upgrade if the copied approval data or file permissions are wrong.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides structured after-fix terminal proof for the macOS CLI help path, parser tests, macOS IPC shard, protocol guard, and production typecheck.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides structured after-fix terminal proof for the macOS CLI help path, parser tests, macOS IPC shard, protocol guard, and production typecheck.
Evidence reviewed

PR surface:

Source -1, Other +99. Total +98 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 2 -1
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 6 127 28 +99
Total 7 128 30 +98

What I checked:

  • Repository policy applied: Read the full root AGENTS.md plus scoped extensions/scripts guidance and Telegram maintainer notes; this affected the compatibility-sensitive migration review and Telegram proof classification. (AGENTS.md:1, b3128ba93def)
  • Current main still has the root parser gap: OpenClawMacCLI.main still reads CommandLine.arguments, calls a private parseRootCommand helper, switches on command names, and exits on unknown commands; focused rg searches found no RootCommandParserTests on current main. (apps/macos/Sources/OpenClawMacCLI/EntryPoint.swift:11, b3128ba93def)
  • PR adds the narrow parser seam and coverage: At PR head, RootCommandParser.swift exposes rootCommandAction and the new Swift tests cover empty args, help aliases, argument splitting, known command dispatch, and unknown-command nonzero action. (apps/macos/Tests/OpenClawIPCTests/RootCommandParserTests.swift:4, 9e388ce4fdbe)
  • Existing test target can cover the CLI module: The macOS OpenClawIPCTests target already depends on OpenClawMacCLI, so the PR uses an existing package boundary instead of adding a new test target. (apps/macos/Package.swift:79, b3128ba93def)
  • Approvals migration change is dependency-backed and upgrade-sensitive: Current main passes only COPYFILE_EXCL, while PR head uses COPYFILE_DATA | COPYFILE_EXCL and still chmods the target to 0600 after creation. The macOS copyfile(3) contract says COPYFILE_DATA copies source file data and COPYFILE_EXCL fails if the destination exists. (leancrew.com) (apps/macos/Sources/OpenClaw/ExecApprovals.swift:371, 9e388ce4fdbe)
  • Branch now overlaps current main: Current main already has Swift literal-union enum generation/output and the Telegram unused-parameter cleanup, while live GitHub reports this PR as CONFLICTING/DIRTY with no potential merge commit. (scripts/protocol-gen-swift.ts:247, b3128ba93def)

Likely related people:

  • steipete: GitHub history shows Peter Steinberger introduced the renamed OpenClaw macOS CLI entry point and recently landed the current-main Swift literal-union enum generator fix. (role: earlier macOS CLI and protocol-generator contributor; confidence: high; commits: 9a7160786a7d, 7a7165ad22c4; files: apps/macos/Sources/OpenClawMacCLI/EntryPoint.swift, scripts/protocol-gen-swift.ts, apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift)
  • vincentkoc: Recent GitHub history shows Vincent Koc working on exec approval state-dir behavior and tests, and carrying the current-main Telegram typecheck cleanup that overlaps this branch. (role: recent exec approvals and Telegram area contributor; confidence: high; commits: adad27d7448e, 894f521aa5bd, 42759a1b799b; files: apps/macos/Sources/OpenClaw/ExecApprovals.swift, apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift, extensions/telegram/src/rich-message.ts)
  • obviyus: Recent Telegram rich-message history maps the rich markdown/table behavior around the overlapping cleanup to Ayaan Zaidi. (role: recent Telegram rich-message contributor; confidence: high; commits: da92615816b6, b3f315461ba7; files: extensions/telegram/src/rich-message.ts)
  • ngutman: Earlier exec command parity work touched the exec approvals surface, making Nimrod Gutman a useful routing candidate for migration-copy review. (role: adjacent exec approvals contributor; confidence: medium; commits: c4a4050ce48b; files: apps/macos/Sources/OpenClaw/ExecApprovals.swift, apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • 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.

@TurboTheTurtle
TurboTheTurtle force-pushed the codex/83879-cli-parser-tests branch from 3b97a7e to 9e388ce Compare June 15, 2026 06:24
@openclaw-barnacle openclaw-barnacle Bot added the channel: telegram Channel integration: telegram label Jun 15, 2026
@TurboTheTurtle

TurboTheTurtle commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jun 15, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 16, 2026
Merged
@vincentkoc vincentkoc mentioned this pull request Jun 17, 2026
Merged
@vincentkoc vincentkoc self-assigned this Jun 17, 2026
@vincentkoc

vincentkoc commented Jun 17, 2026

Copy link
Copy Markdown
Member

Superseded by #93880, which preserves the approvals migration data fix as an attribution-preserving single-purpose PR.

The remaining commits in this mixed PR are already on main or test-only. Closing in favor of the replacement.

@vincentkoc vincentkoc closed this Jun 17, 2026
@TurboTheTurtle
TurboTheTurtle deleted the codex/83879-cli-parser-tests branch June 17, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

app: macos App: macos channel: telegram Channel integration: telegram merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No tests cover parseRootCommand or the command-dispatch switch in main()

2 participants

@TurboTheTurtle @vincentkoc