fix(remote): keep the binding a re-point replaces instead of destroying it

docs/remote-setup.md

@@ -122,6 +122,16 @@ external tool reads the database file directly, resolve the team's new path
 instead of continuing to use the shared database path. Ask the agent for the
 team's store path, or use the command in [Reference](#reference).
 
+A team already connected somewhere else — say you are trying this server with a
+team you use against the hosted one — can be pointed here with the same
+command. The binding it had is not lost: it is kept under `previous_bindings`
+in the team's `config.json`, and `remote status` lists each replaced server
+(host only, with the time it was replaced — the full endpoint is not printed,
+because for a hosted endpoint the path embeds the access token). The local
+sync state and keys for the old server also stay on disk, so reconnecting to
+the full endpoint stored in the config restores the old binding and picks
+that state back up.
+
 **If you connected with `--e2ee`, export the handoff bundle now**, while you are
 still on the machine that holds the key:
 

scripts/internal/remote-sync.mjs

@@ -438,6 +438,20 @@ function zoneRefusal(host) {
 }
 
 export function validateEndpoint(rawEndpoint) {
+  // The WHATWG parser DELETES ASCII tab and newline (and trims leading and
+  // trailing C0/space) before parsing, so a URL carrying them can validate
+  // while the raw string -- which is what gets stored in the binding and
+  // later read back line-wise by consumers such as `remote status` -- still
+  // contains them. The premise of this validator (stated above) is that what
+  // is written in the URL is where the connection goes; a byte the parser
+  // silently removes breaks that premise, so it is refused rather than
+  // repaired (#849 review).
+  if (/[\u0000-\u001f\u007f]/.test(rawEndpoint)) {
+    return rejected(
+      "--endpoint must not contain control characters " +
+      "(tab, newline, any byte below 0x20, or DEL)",
+    );
+  }
   const authority = rawAuthority(rawEndpoint);
   if (authority === undefined) {
     return rejected("--endpoint must start with https:// (or http:// to a private IP address)");

scripts/remote.sh

@@ -551,13 +551,78 @@ _remote_http_get_json() {
   printf '%s' "$curl_output"
 }
 
+# _remote_archive_replaced_binding <cfg_escaped> <new_server_instance_id> \
+#   <new_remote_team_id> <new_protocol_version> <stamp>
+#
+# Echoes the config document with the current $.remote_binding moved into
+# $.previous_bindings, when — and only when — the binding being written names
+# a DIFFERENT identity (#849). The caller passes the SQL-escaped document and
+# re-escapes what comes back before splicing it into its own write.
+#
+# EVERY site that replaces $.remote_binding wholesale must run its document
+# through this first. There are two such writers — _remote_write_binding
+# below, and cmd_pull's bind-after-bootstrap write — and the non-destruction
+# invariant of #849 holds at the writer boundary only if both archive. (The
+# binding_revision-only touch-ups elsewhere replace nothing and are not
+# writers in this sense.)
+#
+# One entry per (server_instance_id, remote_team_id, protocol_version): an
+# entry for the identity being archived is replaced by the newer copy, and an
+# entry matching the identity being written becomes the live binding again
+# and leaves the archive. The array is therefore bounded by the number of
+# distinct such identity tuples this team has ever been bound to -- one per
+# server in the common case, more if the same server re-registers the team
+# or the protocol version moves -- never by how often the team moved
+# between them.
+#
+# `capabilities` is dropped from the archived copy: it is refetched on every
+# connect, and an archived copy would be the one stale snapshot nobody
+# re-reads. Restoring a previous binding is a reconnect to its endpoint --
+# which refetches -- never a copy of the archived object back into
+# $.remote_binding.
+#
+# A current binding with no server_instance_id never completed a
+# registration; there is no partition behind it to point back to, so it is
+# replaced without being archived, same as before.
+_remote_archive_replaced_binding() {
+  local cfg_escaped="$1" new_instance_sql new_team_sql pv="$4" stamp="$5"
+  new_instance_sql="$(_agmsg_sqlesc "$2")"
+  new_team_sql="$(_agmsg_sqlesc "$3")"
+  agmsg_sqlite_mem \
+    "WITH cfg(doc) AS (SELECT '$cfg_escaped'),
+     cur(b) AS (SELECT json_extract(doc, '\$.remote_binding') FROM cfg),
+     kept(arr) AS (SELECT coalesce((
+       SELECT json_group_array(json(value))
+         FROM cfg, json_each(coalesce(json_extract(cfg.doc, '\$.previous_bindings'), '[]'))
+        WHERE NOT (json_extract(value, '\$.server_instance_id') IS json_extract((SELECT b FROM cur), '\$.server_instance_id')
+               AND json_extract(value, '\$.remote_team_id')     IS json_extract((SELECT b FROM cur), '\$.remote_team_id')
+               AND json_extract(value, '\$.protocol_version')   IS json_extract((SELECT b FROM cur), '\$.protocol_version'))
+          AND NOT (json_extract(value, '\$.server_instance_id') IS '$new_instance_sql'
+               AND json_extract(value, '\$.remote_team_id')     IS '$new_team_sql'
+               AND json_extract(value, '\$.protocol_version')   IS $pv)), '[]'))
+     SELECT CASE
+       WHEN (SELECT b FROM cur) IS NOT NULL
+        AND json_extract((SELECT b FROM cur), '\$.server_instance_id') IS NOT NULL
+        AND NOT (json_extract((SELECT b FROM cur), '\$.server_instance_id') IS '$new_instance_sql'
+             AND json_extract((SELECT b FROM cur), '\$.remote_team_id')     IS '$new_team_sql'
+             AND json_extract((SELECT b FROM cur), '\$.protocol_version')   IS $pv)
+       THEN json_set(doc, '\$.previous_bindings',
+              json_insert((SELECT arr FROM kept), '\$[#]',
+                json(json_set(json_remove((SELECT b FROM cur), '\$.capabilities'),
+                              '\$.replaced_at', '$(_agmsg_sqlesc "$stamp")'))))
+       ELSE doc
+     END FROM cfg;"
+}
+
 # _remote_write_binding <cfg> <endpoint> <binding_cipher> <resp_file>
 # Records the binding on the team config from a capability snapshot. No
 # credential is stored: the snapshot holds nothing that cannot be fetched
 # again, and the team_id is a value we minted ourselves.
 #
-# ONE writer for both the first connect and the adopt path below. Two copies of
-# this object would drift, and the second copy is the one nobody re-reads.
+# ONE writer for both the first connect and the adopt path below — but NOT
+# for every path: cmd_pull binds after its bootstrap with a write of its own,
+# which is why the archive step above is a shared primitive rather than a
+# private step of this function.
 _remote_write_binding() {
   local cfg="$1" endpoint="$2" binding_cipher="$3" resp_file="$4" \
     expected_binding_revision="${5:-}"
@@ -592,6 +657,18 @@ _remote_write_binding() {
     fi
   fi
   cfg_escaped="$(sed "s/'/''/g" "$cfg")"
+  # A write that points the team at a DIFFERENT server must not orphan the
+  # binding it replaces (#849). The local sync rows and keys for the old server
+  # survive this write untouched -- they are keyed on (server_instance_id,
+  # remote_team_id, protocol_version) -- but the endpoint string in the binding
+  # is the only pointer back to them, so overwriting it strands data that is
+  # still on disk. The shared archive primitive above moves the current
+  # binding into $.previous_bindings, a sibling key the wholesale json_set on
+  # $.remote_binding never touches.
+  local archived_doc
+  archived_doc="$(_remote_archive_replaced_binding "$cfg_escaped" \
+    "$server_instance_id" "$remote_team_id" "$protocol_version" "$connected_at")"
+  cfg_escaped="$(printf '%s' "$archived_doc" | sed "s/'/''/g")"
   updated=$(agmsg_sqlite_mem \
     "SELECT json_set('$cfg_escaped', '\$.remote_binding', json_object(
        'endpoint', '$(_agmsg_sqlesc "$endpoint")',
@@ -1112,9 +1189,17 @@ cmd_pull() {
   case "$pulled_protocol" in ''|*[!0-9]*)
     echo "agmsg: server answered with an invalid protocol version" >&2; exit 1 ;; esac
   agmsg_lock_acquire "$TEAMS_DIR/$team" || exit 1
-  local bind_at escaped caps_escaped updated
+  local bind_at escaped caps_escaped updated archived_doc
   bind_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
   escaped=$(sed "s/'/''/g" "$cfg")
+  # This is the second wholesale writer of $.remote_binding (#849): a pull
+  # into a team that already holds a binding to a DIFFERENT server would
+  # otherwise replace it with no way back. Same archive primitive as
+  # _remote_write_binding, so the non-destruction invariant holds at the
+  # writer boundary, not just on the connect path.
+  archived_doc="$(_remote_archive_replaced_binding "$escaped" \
+    "$pulled_sid" "$pulled_id" "$pulled_protocol" "$bind_at")"
+  escaped="$(printf '%s' "$archived_doc" | sed "s/'/''/g")"
   caps_escaped=$(printf '%s' "$pulled_caps" | sed "s/'/''/g")
   updated=$(agmsg_sqlite_mem \
     "SELECT json_set('$escaped', '\$.remote_binding', json_object(
@@ -2436,6 +2521,42 @@ _remote_status_one() {
   else
     echo "		encryption: none"
   fi
+  # What this team was bound to before, and when it was replaced (#849). The
+  # archived binding is the only pointer back to that server's local sync rows
+  # and keys, so a repair must not depend on the operator remembering the URL.
+  #
+  # Displayed through _remote_endpoint_display, which keeps scheme/host/port
+  # and DROPS the path -- for a hosted endpoint the path IS the capability.
+  # That means the printed form is NOT the value to reconnect with; the exact
+  # endpoint stays in the team's config, and the trailing line says so instead
+  # of pretending the display is it.
+  #
+  # One JSON object per row, NOT tab-separated fields: validateEndpoint now
+  # refuses raw control bytes, but a binding written by an OLDER version can
+  # hold an endpoint carrying them, and the archive keeps whatever the binding
+  # held. JSON escapes every byte below 0x20, so a row is one line whatever
+  # the endpoint contains; the per-field extraction below re-reads each row as
+  # JSON, and the printed values are additionally stripped of control bytes so
+  # nothing steers the terminal.
+  local prev_row prev_endpoint prev_replaced prev_any=0
+  while IFS= read -r prev_row; do
+    [ -n "$prev_row" ] || continue
+    prev_endpoint="$(agmsg_sqlite_mem \
+      "SELECT json_extract('$(printf '%s' "$prev_row" | sed "s/'/''/g")', '\$.e');")"
+    prev_replaced="$(agmsg_sqlite_mem \
+      "SELECT json_extract('$(printf '%s' "$prev_row" | sed "s/'/''/g")', '\$.a');")"
+    prev_endpoint="$(_remote_endpoint_display "$prev_endpoint")"
+    prev_endpoint="${prev_endpoint//[[:cntrl:]]/}"
+    prev_replaced="${prev_replaced//[[:cntrl:]]/}"
+    prev_any=1
+    echo "		previous: was bound to $prev_endpoint until $prev_replaced"
+  done < <(agmsg_sqlite_mem \
+    "SELECT json_object('e', json_extract(value, '\$.endpoint'),
+                        'a', coalesce(json_extract(value, '\$.replaced_at'), 'an unrecorded time'))
+       FROM json_each(coalesce(json_extract('$(sed "s/'/''/g" "$cfg")', '\$.previous_bindings'), '[]'));")
+  if [ "$prev_any" -eq 1 ]; then
+    echo "		          to restore one, reconnect to its full endpoint — it is kept under previous_bindings in this team's config.json, and is not printed here because it can embed the access token"
+  fi
 }
 
 # _remote_status_json_one <team> — prints one JSONL object for <team>'s

tests/fixtures/endpoint-verdicts.jsonl

@@ -69,3 +69,8 @@
 {"endpoint": "https://example.com", "verdict": "allow", "why": "an ordinary name over https"}
 {"endpoint": "https://ex..ample.com", "verdict": "allow", "why": "doubled dot: odd, but both parsers accept it — not tightened beyond Node"}
 {"endpoint": "https://xn--wgv71a.example", "verdict": "allow", "why": "punycode name over https"}
+{"endpoint": "https://example.com/t/agsy_token", "verdict": "allow", "why": "a hosted capability path: the path IS the endpoint, and archiving/restoring it must carry it verbatim (#849)"}
+{"endpoint": "https://example.com/a\tb", "verdict": "deny", "why": "raw TAB in the path: the WHATWG parser deletes it, so the stored raw string and the parsed URL disagree — and line-wise readers of the stored value split on it (#849)"}
+{"endpoint": "https://example.com/a\nb", "verdict": "deny", "why": "raw LF in the path: same class — one stored value becomes two lines for any line-framed reader (#849)"}
+{"endpoint": "https://exam\rple.com/x", "verdict": "deny", "why": "raw CR: deleted by the parser before parsing, present in the stored raw string (#849)"}
+{"endpoint": "\thttps://example.com", "verdict": "deny", "why": "leading TAB: trimmed by the parser, kept in the raw string (#849)"}