Skip to content

fix(#110 follow-up): align CLI to PR-2 server schema — read prep.code, widen regex#244

Open
s2agi wants to merge 1 commit into
mainfrom
fix/110-node-local-only-schema
Open

fix(#110 follow-up): align CLI to PR-2 server schema — read prep.code, widen regex#244
s2agi wants to merge 1 commit into
mainfrom
fix/110-node-local-only-schema

Conversation

@s2agi

@s2agi s2agi commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Author

Agent: 通信工程马

Summary

测试马 PR-5 Case 2 caught a schema mismatch between PR-2 (server, landed) and PR-3 (my CLI in #225 / commit f28ffd9). anet node rename node-c node-d --force on a purely-created node — exactly the case #110 was meant to fix — still hard-rolled-back with rc=1 instead of doing the local-only rename.

Root cause

Server PR-2 (server/src/rename.ts:117-124) ships:

{
  "ok": false,
  "code": "node_local_only",
  "error": "node 'X' has no server session in this network",
  "suggested": "rename locally"
}

My PR-3 (this file, pre-fix) checked:

prep.error === "node_local_only"                  // wrong field — error is the message
|| /not found in this network/i.test(errStr)       // wrong wording
|| /node .* not found/i.test(errStr)               // generic

isLocalOnly = falsethrow → PHASE 1 rollback → rc=1.

Fix

Switch to prep.code as the primary signal (matches PR-2 contract), keep prep.error checks as belt-and-braces, widen the regex to include the server's actual wording:

isLocalOnly =
    prep.code === "node_local_only"            // PR-2 contract (primary)
 || prep.error === "node_local_only"           // legacy / older builds
 || /node_local_only/i.test(errStr)            // belt-and-braces
 || /has no server session/i.test(errStr)      // server's actual phrase
 || /not found in this network/i.test(errStr)  // pre-PR-2 wording
 || /node .* not found/i.test(errStr)          // generic fallback

Smoke

Bundle marker verification (post npm run build w/ javascript-obfuscator):

String Count Source
node_local_only 2 new prep.code check + regex literal
has no server session 1 new regex literal (matches server's actual phrase)
not found in this network 1 legacy regex retained

Full integration verify deferred to 测试马 PR-5 Case 2 rerun (their harness has the live commhub-server + the create→rename sequence).

Refs

Test plan

  • bunx tsc --noEmit clean
  • npm run build (bun + obfuscator x3) 14s clean
  • Bundle marker grep — all 4 schema strings present
  • 测试马 PR-5 Case 2 rerun: anet node create node-c (no start) → anet node rename node-c node-d --force → expect rc=0, .anet/nodes/node-d exists, .anet/nodes/node-c gone

🤖 Generated with Claude Code

…, widen regex

测试马 PR-5 Case 2 caught: `anet node rename node-c node-d --force` on a
purely-created node (never started, no commhub sessions row) still hard-
rolled-back with "rolling back / rollback complete — node-c unchanged"
rc=1 on main HEAD even though PR-1/2/3/4 were all merged. The intended
spec — server returns `node_local_only` → CLI does dir-rename + config-
update only, rc=0 — never fired.

Root cause: schema mismatch between PR-2 (server, landed correctly) and
PR-3 (my CLI, in #225 / commit f28ffd9).

  Server PR-2 (server/src/rename.ts:117-124) ships:
    { ok: false, code: "node_local_only",
      error: "node 'X' has no server session in this network",
      suggested: "rename locally" }

  CLI PR-3 (this file, pre-fix) checked:
    prep.error === "node_local_only"                   // wrong field
    || /not found in this network/i.test(errStr)        // wrong wording
    || /node .* not found/i.test(errStr)                // wrong wording

So:
  - prep.code === "node_local_only"   was never inspected
  - prep.error was the human message, not the type string
  - the regex looked for "not found", server says "has no server session"

→ isLocalOnly = false → throw → PHASE 1 rollback → rc=1.

Fix: switch to `prep.code` as the primary signal (matches the server
contract), keep prep.error checks as belt-and-braces, widen the regex
to match the server's actual wording.

  isLocalOnly =
       prep.code === "node_local_only"           // PR-2 contract (primary)
    || prep.error === "node_local_only"          // legacy / older builds
    || /node_local_only/i.test(errStr)           // belt-and-braces
    || /has no server session/i.test(errStr)     // server's actual phrase
    || /not found in this network/i.test(errStr) // pre-PR-2 wording
    || /node .* not found/i.test(errStr)         // generic fallback

Bundle marker verification (post obfuscator):
  node_local_only ×2 ✓ (the new prep.code check + regex literal)
  "has no server session" ×1 ✓ (new regex literal)
  "not found in this network" ×1 ✓ (legacy retained)

Refs: #110 (RFC-010 §4.1 "created is recommended rename path"), #146 PR-3
(#225 / f28ffd9 — root cause), 测试马 PR-5 Case 2

Author-Agent: 通信工程马

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4bd660bee

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread agent-network/bin/cli.ts
// fallback to include the server's actual "has no server session" phrase.
const errStr = String(prep.error || "");
const isLocalOnly = prep.error === "node_local_only"
const isLocalOnly = prep.code === "node_local_only"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Check remote alias conflicts before local-only rename

When the old node has no sessions row, accepting prep.code === "node_local_only" skips the only server-side check that new_alias is free. In the current server path, prepareRename returns node_local_only before testing newAlias against sessions, so a never-started local node can now be renamed to an alias that is already registered by another machine in the same network; when this renamed node is later started, report_status deletes other same-alias sessions in that network, effectively evicting the real node. Please either have the local-only path verify the target alias against the hub or require the server response to guarantee that the target alias is unused.

Useful? React with 👍 / 👎.

s2agi pushed a commit that referenced this pull request Jun 15, 2026
Full 14-case Docker matrix verifies the rename/identity family fixes
(#146 + #180 + #110 + #203 + #213 + #174 + RFC-010 §6.4).

Stack:
- agent-network: fix/110-node-local-only-schema branch (pre-merge validate
  of #244 PR-3 schema-key fix)
- agent-node + commhub-server: main HEAD (PR-1+2+4 all merged)

Results (14/14 PASS):
- #146 send_message canonical redirect: case 6 response carries
  `renamed_from:old-name, renamed_to:new-name`; inbox row at canonical
- #110 purely-created local-only rename: case 2 rc=0, dir flipped,
  node_id preserved; #244 schema fix lands cleanly
- #180 stale-pid no-misskill: case 10 unrelated bystander alive post-force
- #203 alias-identity isolation: case 5 N3 sessions.node_id ≠ N2's
- #213 from_node_id outbound resume: case 13 tasks.from_node_id populated,
  list_tasks finds old-alias outbound by node_id
- #174 projectRestart verifyNodeUp: case 14 reports 1/1 up post-rename

Cases 6/7 use a SEPARATE sender node (not the renamed target) to satisfy
PR-4 #203 from_session_identity_mismatch enforcement (bf564fb). Sender's
ntok proves the from_session field; target alias is what canonical resolve
operates on.

Case 13 schema migration + populate wire both proven (column exists +
populated by send_task path).

Validation pattern: this run is pre-merge branch-build of agent-network
from fix/110-node-local-only-schema. All 14 PASS → safe to merge into main.

Harness in tests/test-rename-identity/ (Dockerfile + lib/helpers.sh + 14
case scripts + run-all.sh + Makefile). Re-runnable via `make all` or
`make case-N`.

Evidence: docs/tests/p-146-pr5-rename/{REPORT.md, MATRIX.md, case-N/...}

红线: Docker only, no host hub, no prod hub, tokens masked.
Author-Agent: 通信测试马
Refs: #146 #180 #110 #203 #213 #174 RFC-010 §6.4 PR#244
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.

2 participants