Skip to content

feat(discovery): add agentsync ls and status --machine for cross-machine browse - #187

Merged
chrisleekr merged 2 commits into
mainfrom
feat/discovery
Jun 20, 2026
Merged

feat(discovery): add agentsync ls and status --machine for cross-machine browse#187
chrisleekr merged 2 commits into
mainfrom
feat/discovery

Conversation

@chrisleekr

@chrisleekr chrisleekr commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Why

copy needs an exact logical path (copy work-laptop claude/CLAUDE.md.age), but there was no way to discover those paths. status only ever inspected this machine's namespace, and there was no CLI command to list another machine's artifacts — so on a fresh machine you had to guess paths or open the TUI. The "canonical scripting surface" couldn't enumerate what was copyable.

Fourth PR in the series (follows #184, #185, #186).

What

  • agentsync ls [machine] [path] — list machine namespaces, or the copyable .age paths within one. Read-only and key-free: it lists which encrypted files exist without decrypting, so a machine that is not yet a recipient can still discover what copy accepts. ls self browses your own backup; a path prefix narrows the listing.
  • agentsync status --machine <name> — compare local config against another machine's namespace for a pre-copy diff (synced/local-changed/local-only/vault-only). Validates the name against known machines; self resolves to this machine.
  • Refactor: exported the shared enumerateArtifacts (fixing a leading-slash bug for whole-namespace enumeration); computeSyncStatus gained an optional sourceMachine.

Security (caught in review)

The review panel found a path-traversal info disclosure: ls work-laptop ../../../../etc would have enumerated .age files outside the namespace (copy blocks this downstream, but ls printed raw paths). Fixed by a containment guard in the shared enumerateArtifacts — any relDir that resolves outside machineRoot returns [], covering both ls and copy's dir-sweep. Regression test included.

Tests

ls.test.ts (10): all LsResult kinds, self, path prefix, traversal guard, reconcile-error (diverged history), and the CLI wrapper exit code. status.test.ts (+2): cross-machine synced comparison and unknown-machine error. Full suite: 947 pass / 0 fail.

CI note

bun test exits non-zero on the per-file coverage floor; CI treats 0-fail as success.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added ls subcommand for read-only discovery of machines and copyable artifacts in the vault without requiring decryption.
    • Enhanced status command with --machine option to compare agent synchronization status against other machines.
  • Bug Fixes

    • Improved security: hardened artifact enumeration against path traversal attacks.
  • Documentation

    • Updated command documentation and README to reflect new and enhanced functionality.

…machine browse

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011L7s33UCjpQkreXW7amAw2
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chrisleekr, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 5 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 09f944cb-5055-4d44-8b86-3f8c2774dc49

📥 Commits

Reviewing files that changed from the base of the PR and between cf58801 and 6cfb103.

📒 Files selected for processing (3)
  • docs/commands.md
  • src/commands/__tests__/status.test.ts
  • src/commands/copy.ts
📝 Walkthrough

Walkthrough

Adds a new agentsync ls subcommand for read-only, key-free vault namespace and artifact discovery. Extends agentsync status with a --machine flag for cross-machine comparison. Exports enumerateArtifacts from copy.ts with path-traversal containment guards for shared use. Updates documentation and adds tests for both commands.

Changes

ls command, status --machine, and shared traversal hardening

Layer / File(s) Summary
enumerateArtifacts export and traversal guard
src/commands/copy.ts
enumerateArtifacts is exported and gains isAbsolute/resolve/relative containment logic that returns an empty array when relDir resolves outside machineRoot. Also fixes childRel to omit a leading slash during whole-namespace enumeration.
ls command: LsResult type, performLs, lsCommand, CLI wiring
src/commands/ls.ts, src/cli.ts
Defines LsResult discriminated union; implements performLs (vault reconcile, machine listing, self-alias, unknown-machine guard, artifact enumeration); exports lsCommand with machine and path args; registers lsCommand as the ls subcommand in the root CLI.
status --machine cross-machine comparison
src/commands/status.ts
Adds sourceMachine? to ComputeSyncStatusOptions and uses it in computeSyncStatus for machineRoot selection. Extends the status CLI with --machine, validates it against listMachines, sets process.exitCode = 1 on unknown input, and passes sourceMachine into computeSyncStatus.
ls command tests
src/commands/__tests__/ls.test.ts
Git-based fixture harness seeds two machine namespaces and verifies all performLs result kinds: machines, artifacts, path-prefix narrowing, self-alias, unknown-machine, empty, traversal-blocked, reconcile-error, and CLI exit code on unknown machine.
status --machine tests
src/commands/__tests__/status.test.ts
Adds a positive --machine test (peer skill shows as synced) and a negative test (unknown machine sets process.exitCode = 1 with an error message).
Documentation
README.md, docs/commands.md
Adds ls to the README commands table. Adds ls to the command index, documents its usage, arguments, key-free behavior, and fast-forward reconciliation. Expands status docs with --machine, new status strings (vault-only, unknown, error), and updated caveats.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • chrisleekr/agentsync#12: The ls command calls GitClient.reconcileWithRemote with allowMissingRemote; the fast-forward-only reconciliation logic introduced or modified in this PR is a direct code-level dependency of the new performLs implementation.

Suggested labels

bot:resolve

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: adding the agentsync ls command for discovery and status --machine for cross-machine comparison.
Description check ✅ Passed The description comprehensively covers all required sections: Why (motivation), What (features added), Security (vulnerability and fix), Tests (coverage and results), and includes appropriate context about the PR series.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/commands.md`:
- Line 248: The recovery command documentation for `vault-only` status is
incomplete for cross-machine scenarios. When using `status --machine <peer>`,
the source namespace is the peer machine's namespace, not the local disk, making
the `copy self <path>` instruction incorrect. Update line 248 to clarify that
the recovery command should reference the compared peer machine instead of using
`self`, so users can properly copy content from the peer namespace to the local
disk when comparing across machines.

In `@src/commands/__tests__/status.test.ts`:
- Around line 285-287: Replace the order-dependent artifact selection in the
test with an explicit lookup of the "peer-skill" artifact from snap.artifacts.
Instead of accessing snap.artifacts[0], find the artifact that matches the
"peer-skill" identifier by filtering or searching through snap.artifacts. Add an
assertion or check immediately after to fail fast if the peer-skill artifact is
not found, ensuring the test fails with a clear error rather than proceeding
with an incorrect artifact or undefined plaintext value.

In `@src/commands/copy.ts`:
- Around line 45-47: The containment validation check in the copy command is too
broad in its rejection logic. The condition checking
`containment.startsWith("..")` catches valid paths like `..cache/foo.age` that
aren't actually escaping the directory. Instead of checking if the containment
path simply starts with "..", change it to check for "../" which specifically
indicates directory traversal escaping. This allows valid artifact names that
happen to start with two dots while still properly blocking actual directory
escape attempts.

In `@src/commands/ls.ts`:
- Around line 34-36: The error handling in the reconcile-error catch block
(lines 34-36) and the similar error handling at lines 110-112 currently return
only the raw error message without actionable recovery guidance. Enhance both
catch blocks to include specific next-step instructions that guide users on how
to recover, such as directing them to resolve divergence or run a recovery
command. Modify the error message returned in the reconcile-error object to
combine the descriptive error message with clear, actionable guidance so users
know what to do immediately when reconciliation fails.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 916bdaea-406d-4041-90ef-24322787aa18

📥 Commits

Reviewing files that changed from the base of the PR and between 4e4d1df and cf58801.

📒 Files selected for processing (8)
  • README.md
  • docs/commands.md
  • src/cli.ts
  • src/commands/__tests__/ls.test.ts
  • src/commands/__tests__/status.test.ts
  • src/commands/copy.ts
  • src/commands/ls.ts
  • src/commands/status.ts

Comment thread docs/commands.md Outdated
Comment thread src/commands/__tests__/status.test.ts Outdated
Comment thread src/commands/copy.ts Outdated
Comment thread src/commands/ls.ts
…ne vault-only doc, deterministic fixture

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011L7s33UCjpQkreXW7amAw2
@chrisleekr
chrisleekr merged commit 8fc2009 into main Jun 20, 2026
24 checks passed
@chrisleekr
chrisleekr deleted the feat/discovery branch June 20, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant