Skip to content

fix(scripts): salience_phase0.sh — flyctl ssh -C is exec, not shell#151

Merged
cipher813 merged 1 commit into
mainfrom
fix/salience-snapshot-vec-cp-quoting
May 22, 2026
Merged

fix(scripts): salience_phase0.sh — flyctl ssh -C is exec, not shell#151
cipher813 merged 1 commit into
mainfrom
fix/salience-snapshot-vec-cp-quoting

Conversation

@cipher813
Copy link
Copy Markdown
Owner

Summary

Two bugs in cmd_snapshot caught on first real prod-snapshot run:

Bug 1: flyctl ssh -C doesn't interpret shell operators

flyctl ssh console -a X -C "cp A B && echo done" execs the command directly without a sh -c wrapper. So && and following tokens become literal args to cp:

cp /data/default.vec.npz /tmp/snap.vec.npz "vec copy done"
   └─ cp tried to use "vec copy done" as a destination directory

The python -c command in Step 1 (sqlite backup) worked fine because it was a single command + single quoted arg — no shell operators involved.

Fix: drop the && echo X confirmation from Step 2's cp invocation. Single command only. If chaining is needed in future, wrap in sh -c "..." explicitly.

Bug 2: success message printed unconditionally even on failure

flyctl ... -C "cp ..." || echo_warn "...failed..."
echo_ok "remote ... written"      # ← runs ALWAYS, including on failure

Resulted in contradictory output during the live run:

⚠ remote vecstore copy failed — embedding signals will be zero
✓ remote /tmp/snap.vec.npz written        ← false!

Fix: switch from cmd || warn (followed by unconditional ok) to if cmd; then ok; else warn; fi. Applied to Step 2 (vec copy) AND Step 4 (remote cleanup) — same pattern bug at both sites.

After this fix

git checkout main && git pull
scripts/salience_phase0.sh snapshot

Expected output:

  • Step 2: ✓ remote /tmp/snap.vec.npz written (and ONLY this line — no contradictory warn)
  • Step 3: downloaded sqlite=8036352B vec.npz=~3000000B
  • Result block: Vectors: ~2433 (matching Live memories: 2433)

Then scripts/salience_phase0.sh score will use the embedding signals (constraint_score, time_penalty) as designed — picks should look fundamentally different from the all-zeros-c-and-tp output you saw.

Verified

  • bash -n clean
  • Locally can't test flyctl interaction directly — but the cp command is now a single execv-style call, matching the working Step 1 python -c pattern

🤖 Generated with Claude Code

Two bugs in cmd_snapshot caught on first real run:

1. `flyctl ssh -C "cmd && echo done"` doesn't interpret `&&` as a
   shell operator. flyctl execs the command directly (no `sh -c`
   wrapper), so `&&` and following tokens get passed as literal
   args to the first command. cp ended up with:

     cp /data/default.vec.npz /tmp/snap.vec.npz "vec copy done"

   …and tried to use "vec copy done" as a destination directory,
   which doesn't exist → error.

   Fix: drop the `&& echo X` confirmation. Single command only.
   If chaining is needed in future, wrap in `sh -c "..."` explicitly.

2. Error handling pattern was wrong — `flyctl ... || echo_warn`
   followed by an unconditional `echo_ok` printed BOTH the warning
   AND the success line on failure:

     ⚠ remote vecstore copy failed — embedding signals will be zero
     ✓ remote /tmp/snap.vec.npz written        ← false!

   Fix: switch from `cmd || warn` (followed by unconditional ok) to
   `if cmd; then ok; else warn; fi`. Applied to both Step 2 (vec
   copy) and Step 4 (remote cleanup) — same pattern bug.

After this fix, re-running `scripts/salience_phase0.sh snapshot`
will:
  - Step 2: cp succeeds cleanly, vec.npz lands in /tmp/snap.vec.npz
  - Step 3: sftp pulls both files
  - Result: snapshot has Vectors: ~2433 (matching live memories)

Then `scripts/salience_phase0.sh score` will use the embedding
signals (constraint_score, time_penalty) as designed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cipher813 cipher813 merged commit 863efa7 into main May 22, 2026
9 checks passed
@cipher813 cipher813 deleted the fix/salience-snapshot-vec-cp-quoting branch May 22, 2026 00:12
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.

1 participant