Skip to content

feat(self-diagnose): cross-node gather + diff via SSH (closes #421) - #768

Merged
qingyun-wu merged 3 commits into
mainfrom
feat/cross-node-diagnose
May 17, 2026
Merged

qingyun-wu merged 3 commits into
mainfrom
feat/cross-node-diagnose

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Summary

Closes #421. When Sutando works on one machine but is broken on another, the existing single-host self-diagnose + stage-readiness.sh can't surface the gap. This PR adds gather-remote.sh — run gather.sh on the failing node via SSH, pull the result back, build a structured comparison report.

What's new

  • skills/self-diagnose/scripts/gather-remote.sh <ssh-target> [window] — orchestrator
  • Honors ~/.ssh/config aliases (same convention as skills/cross-node-sync/setup-rsync-sync.sh)
  • Override remote sutando path via SUTANDO_REMOTE_REPO=/path/on/peer
  • Persists comparison to notes/diagnose-cross-node-<YYYY-MM-DD>.md

Comparison report covers

  • Repo commit alignment (heads + list of commits the remote may be missing)
  • Health check side-by-side per side
  • Voice-agent error signal counts per side
  • Quota state per side
  • Open-PR view drift (which #N each side sees but not the other)
  • Files present in only one gather (recursive diff -rq)

Security posture (per #421 constraints)

  • Read-only on the remote — only gather.sh runs there. No mutation, no codex exec, no shell-out beyond gather.sh.
  • Allowlist via gather.sh's scope — gather already enumerates what gets collected; no .env, no tokens. This script rsyncs only that dir.
  • Per-session SSH — no daemon, no persistent state on the remote.
  • Inherits gather.sh's redaction posture — TODO at end of script flags a redaction audit before broad use.

Out of scope (future work)

  • Real-time Q&A on the failing node via codex exec --sandbox read-only. Wait until diff.md output shows we need free-form remote inspection.
  • Secret-redaction audit pass on gather.sh outputs.
  • Three-way diff (when more than two nodes coordinate).

Test plan

  • bash tests/gather-remote.test.sh — 5/5 green:
    • --help prints usage
    • No-arg invocation prints usage + exits non-zero
    • Unreachable peer fails fast with clear cannot reach stderr + manual ssh hint
    • Default window = 24h
    • 2nd positional arg overrides window
  • End-to-end smoke (manual, not in CI — requires a reachable peer with a sutando checkout): documented in SKILL.md for the operator to run when a real cross-node bug appears.

🤖 Generated with Claude Code

qingyun-wu and others added 2 commits May 16, 2026 09:11
When Sutando works on one machine but fails on another, you previously had
no easy way to find the gap. `scripts/stage-readiness.sh` and the existing
`self-diagnose` skill cover the single-host case; cross-node sync ships
data, not state introspection. This PR fills the gap.

New: `skills/self-diagnose/scripts/gather-remote.sh <ssh-target> [window]`

Flow:
  1. SSH into the failing node (uses ~/.ssh/config — same convention as
     skills/cross-node-sync/setup-rsync-sync.sh).
  2. Run `gather.sh` on the remote (only thing executed there — read-only
     by construction; no mutation, no codex, no shell-exec beyond gather).
  3. rsync the gathered dir back locally.
  4. Run `gather.sh` locally (parallel ground truth).
  5. Build a structured `diff.md` covering: repo commit alignment
     (with the list of commits the remote may be missing), health-check
     side-by-side, voice-agent error signal counts, quota state, open-PR
     view drift (which numbers each side sees), and a recursive file diff
     for anything one side has and the other doesn't.
  6. Persist a copy at `notes/diagnose-cross-node-<YYYY-MM-DD>.md`.

Output paths:
  /tmp/sutando-diagnose-cross-<ts>/local/   — gather output for this node
  /tmp/sutando-diagnose-cross-<ts>/remote/  — pulled-back gather output
  /tmp/sutando-diagnose-cross-<ts>/diff.md  — structured comparison
  notes/diagnose-cross-node-<YYYY-MM-DD>.md — persisted copy

Security posture (per #421 constraints):
  - Read-only on the remote — only gather.sh runs, no mutation
  - Allowlist enforced by gather.sh's existing scope (no .env, no tokens,
    no credentials in the rsynced payload)
  - Per-session SSH; no daemon, no persistent state on the remote
  - Inherits gather.sh's secret-redaction posture — TODO at the bottom of
    the script flags a redaction audit before the feature sees broad use

SKILL.md updated with a "Cross-node mode (resolves #421)" section so the
existing /self-diagnose flow knows about the new sub-command.

Tests (5 cases, bash, all green):
  - --help prints usage
  - no-arg invocation prints usage + exits non-zero
  - unreachable peer fails fast with clear 'cannot reach' stderr + manual ssh hint
  - default window = 24h
  - 2nd positional arg overrides window

End-to-end coverage (actual ssh+rsync) is a manual smoke documented in
SKILL.md — requires a reachable peer with a sutando checkout, which CI
doesn't have.

Out of scope (future):
  - Real-time Q&A on the remote via codex exec --sandbox read-only.
    Wait until the diff.md output's findings suggest free-form remote
    inspection is needed; today's static gather covers the 80% case.
  - Secret-redaction audit of gather.sh's outputs (TODO at end of script).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bash test added in the prior commit was silently dropped by .gitignore's
tests/* allowlist which only un-ignored *.test.ts and *.test.py. Add
*.test.sh to the allowlist (same change as the parallel PR #764 makes for
the memory-sync migration test) so this PR is self-contained.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed: new gather-remote.sh for cross-node diagnose (closes #421). Read-only on remote (gather.sh only), allowlist via gather.sh's existing scope, per-session SSH. Structured local/remote/diff.md output + persisted notes copy. CI green. — Lucy (Mac Studio bot)

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — closes #421 with a focused two-side gather + structured diff, and the security posture is explicit + correct (read-only on remote, allowlist enforced by gather.sh's own scope, per-session SSH, no daemon). The TODO at the bottom about a redaction-audit-before-broad-use is exactly the right caveat to leave loud.

Things I like:

  1. Security posture explicit + load-bearing (gather-remote.sh:25-43 + SKILL.md:42-46) — four bullet points: read-only on remote, allowlist via gather.sh scope, per-session SSH, inherits gather.sh redaction. Each constraint named with reasoning. Without this, the script would look like "arbitrary remote shell exec." With this, it's "I run one specific allowlisted script on the remote and rsync its output."

  2. Honors ~/.ssh/config aliases (gather-remote.sh:91-94) — gather-remote.sh mac-mini works the same as gather-remote.sh user@macbook.local. Matches the convention already established in skills/cross-node-sync/setup-rsync-sync.sh, so an operator who set up the sync alias gets the diagnostic for free.

  3. Quick reachability check before heavy lift (gather-remote.sh:127-131) — ssh -o BatchMode=yes -o ConnectTimeout=5 "$PEER" true first. Bails immediately with a helpful error message (manual test command included) if SSH is broken. Without this, the user would wait for gather.sh to time out across an unreachable peer.

  4. REMOTE_REPO="${SUTANDO_REMOTE_REPO:-...}" override (gather-remote.sh:96) — env-var escape hatch for the case where the remote's checkout isn't at the default ~/Desktop/sutando. Per-invocation, no config-file edits needed.

  5. Local repo auto-resolve (gather-remote.sh:108-122) — SUTANDO_ROOT env > $PWD/build_log.md > walk-up from script dir. Mirrors gather.sh's own resolution. So the script works from any cwd without forcing cd first.

  6. Stream remote gather output, capture path via tail -1 (gather-remote.sh:136-140) — gather.sh's contract is "last line is the output path." This script consumes that contract cleanly via tail. If the contract ever changes (e.g. gather.sh adds a trailing summary), this line breaks loudly + greppably.

  7. Structured diff sections (PR body) — commit alignment, health check, voice-agent errors, quota, PR-view drift, file-tree diff. Each section is a focused comparison; no monolithic "diff everything" output.

  8. set -euo pipefail (gather-remote.sh:65) — bash strict mode. Any failure exits with non-zero immediately + the missing-var check catches typos.

  9. Persisted copy at notes/diagnose-cross-node-<date>.md — /tmp ephemeral copy for immediate use; notes/ archive for "what did the gap look like last week?" Same pattern as gather.sh itself.

  10. .gitignore allows tests/*.test.sh (.gitignore:45) — needed for the test to be tracked.

Non-blocking observations:

  1. REMOTE_REPO quoting (gather-remote.sh:96) — REMOTE_REPO="${SUTANDO_REMOTE_REPO:-\$HOME/Desktop/sutando}" — the \$HOME escapes locally so the literal $HOME is passed to ssh, which expands on the remote side. Correct (because the local user's $HOME != the remote user's $HOME). Worth a comment naming this so a future "fix" doesn't unescape it.

  2. TODO at bottom about redaction audit (gather-remote.sh end + SKILL.md) — gather.sh's existing scope is the allowlist, but if any of those collected items happen to contain secrets (e.g. a log line that prints an API key), they'd land in the local /tmp/ + persisted notes. The TODO is the right place to flag this; an audit pass before broader rollout is the right next step.

  3. No bandwidth limit on rsync (gather-remote.sh:146) — gather.sh output is typically <10MB so unbounded is fine. If it ever grows (full log archives), --bwlimit would prevent saturating a low-bandwidth link.

  4. Local gather runs serially after remote (gather-remote.sh:152-156 + comment) — comment notes parallelization is a future optim. Fine for today; if cross-node diagnose becomes interactive (operator waiting), parallelizing would halve wall-clock.

  5. if [ -d "$LOCAL_OUT" ] (gather-remote.sh:158) — silently skips local copy if gather.sh failed locally. Worth a warn so the diff doesn't render against an empty local side without explanation.

  6. SUTANDO_REMOTE_REPO not validated — if the env var points to a path that exists but isn't a sutando checkout, the remote cd succeeds and gather.sh fails with a different error. Worth a remote-side sanity check (e.g. [ -f build_log.md ]).

  7. Test file mentioned but I didn't read it (tests/gather-remote.test.sh) — worth confirming coverage for: SSH-unreachable bail, missing-remote-repo bail, basic diff sections present.

  8. No cleanup of /tmp/sutando-diagnose-cross-<ts>/ — accumulates over time. The notes/ copy is the durable record; the /tmp/ copy could be auto-cleared after N days.

Cross-PR linkages:

  • Closes #421 — the long-running "we can't tell what's broken on the OTHER node" issue.
  • Builds on existing gather.sh — no new collection logic; just a wrapper that runs gather on both sides + diffs them.
  • Pairs with #767 core-heartbeat — both PRs answer "is the other node alive/working?" #767 is the always-on liveness signal; this PR is the deeper on-demand diagnostic.
  • #sutando-resources/skills/self-diagnose is the natural home — fits cleanly in the existing skill, no new top-level dir.
  • SSH-alias convention shared with cross-node-sync — operators who set up one get the other for free.

Tests: tests/gather-remote.test.sh ships. Worth confirming coverage for the failure modes (SSH-unreachable, REMOTE_REPO missing, gather.sh failing on remote). Happy-path tests need a real peer; CI would need a localhost-loopback setup.

Ready to merge after a quick redaction audit pass on gather.sh's output (the TODO at bottom). The script itself is solid; the security concern is about what gather.sh emits, which is upstream of this PR.

— Lucy (Mac Studio bot)

@qingyun-wu
qingyun-wu merged commit aaaeb8a into main May 17, 2026
2 checks passed
@qingyun-wu
qingyun-wu deleted the feat/cross-node-diagnose branch May 17, 2026 01:09
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…#421) (sonichi#768)

* feat(self-diagnose): cross-node gather + diff via SSH (closes sonichi#421)

When Sutando works on one machine but fails on another, you previously had
no easy way to find the gap. `scripts/stage-readiness.sh` and the existing
`self-diagnose` skill cover the single-host case; cross-node sync ships
data, not state introspection. This PR fills the gap.

New: `skills/self-diagnose/scripts/gather-remote.sh <ssh-target> [window]`

Flow:
  1. SSH into the failing node (uses ~/.ssh/config — same convention as
     skills/cross-node-sync/setup-rsync-sync.sh).
  2. Run `gather.sh` on the remote (only thing executed there — read-only
     by construction; no mutation, no codex, no shell-exec beyond gather).
  3. rsync the gathered dir back locally.
  4. Run `gather.sh` locally (parallel ground truth).
  5. Build a structured `diff.md` covering: repo commit alignment
     (with the list of commits the remote may be missing), health-check
     side-by-side, voice-agent error signal counts, quota state, open-PR
     view drift (which numbers each side sees), and a recursive file diff
     for anything one side has and the other doesn't.
  6. Persist a copy at `notes/diagnose-cross-node-<YYYY-MM-DD>.md`.

Output paths:
  /tmp/sutando-diagnose-cross-<ts>/local/   — gather output for this node
  /tmp/sutando-diagnose-cross-<ts>/remote/  — pulled-back gather output
  /tmp/sutando-diagnose-cross-<ts>/diff.md  — structured comparison
  notes/diagnose-cross-node-<YYYY-MM-DD>.md — persisted copy

Security posture (per sonichi#421 constraints):
  - Read-only on the remote — only gather.sh runs, no mutation
  - Allowlist enforced by gather.sh's existing scope (no .env, no tokens,
    no credentials in the rsynced payload)
  - Per-session SSH; no daemon, no persistent state on the remote
  - Inherits gather.sh's secret-redaction posture — TODO at the bottom of
    the script flags a redaction audit before the feature sees broad use

SKILL.md updated with a "Cross-node mode (resolves sonichi#421)" section so the
existing /self-diagnose flow knows about the new sub-command.

Tests (5 cases, bash, all green):
  - --help prints usage
  - no-arg invocation prints usage + exits non-zero
  - unreachable peer fails fast with clear 'cannot reach' stderr + manual ssh hint
  - default window = 24h
  - 2nd positional arg overrides window

End-to-end coverage (actual ssh+rsync) is a manual smoke documented in
SKILL.md — requires a reachable peer with a sutando checkout, which CI
doesn't have.

Out of scope (future):
  - Real-time Q&A on the remote via codex exec --sandbox read-only.
    Wait until the diff.md output's findings suggest free-form remote
    inspection is needed; today's static gather covers the 80% case.
  - Secret-redaction audit of gather.sh's outputs (TODO at end of script).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: include cross-node gather-remote test + allow tests/*.test.sh

The bash test added in the prior commit was silently dropped by .gitignore's
tests/* allowlist which only un-ignored *.test.ts and *.test.py. Add
*.test.sh to the allowlist (same change as the parallel PR sonichi#764 makes for
the memory-sync migration test) so this PR is self-contained.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Help users debug something that's already working on my end

2 participants