fix(p2p): harden CLI status and operator/key-rotation flows

[trvon]

Summary

Stacked draft child of #75. Closes the robustness/quality items from the deep review of the #68 stack that were not security-blocking but should land before the stack is moved to ready-for-review. No merge or deploy.

1. p2p status text is machine-parseable again (P1) — removed ANSI severity colorization from the key=value stream; every field is now a plain value.
2. CLI connect/disconnect/forget stop throwing on malformed daemon responses (P1) — replaced .at("peer_node_id")/.at("node_id") with .value(key, default) and widened the deltas_sent/deltas_received defaults to std::uint64_t (no size_t→unsigned int truncation).
3. Handshake/delta timeout options are bounded to kP2pMaxOperationTimeout (P2) — validateConfig/validateOptions now reject a timeout above 1 minute up front instead of passing config then dying at readFrame.
4. getMemorySyncStatus degrades instead of failing (P2) — a transient p2p_peers read failure now logs and reports peerCount=0 rather than failing the whole status IPC.
5. sync_interval_ms is ceiling-checked for direct transport (P2) — a value above the 5-minute reconnect ceiling now fails config resolution instead of aborting daemon startup in P2pManager::create.
6. Key rotation is possible via an explicit forget operation (P2) — added yams p2p forget <node-id> end-to-end (proto enum FORGET=9, dispatcher, ServiceManager::forgetP2pPeer, P2pManager::forget → PeerRegistry::removePeer). Fail-closed enrollment is preserved: re-enrolling with a different pin is still rejected; rotation requires explicit forget-then-enroll.

RED evidence

• formatConnect/formatDisconnect would throw nlohmann::json::out_of_range/type_error on a parseable-but-malformed response.
• sync_interval_ms = 300001 passed config resolution and aborted daemon startup later.
• A peer enrolled with pin A could not be rotated (enroll with pin b returned Unauthorized and no removal path existed).

Validation

Focused normal matrix (green, no regressions):

• p2p_protocol 495 / 14, p2p_delta 301 / 10, p2p_manager 180 / 12, peer_registry 91 / 6
• service_manager 789 / 47 passed + 1 skip
• daemon_component_submodule 1319 / 109 (config resolver + proto serializer)
• memory_sync 838 / 60, memory_sync_service 263 / 27

New coverage: timeout ceiling, direct sync_interval_ms ceiling, and an explicit key-rotation flow (enroll → fail-closed replace → forget → re-enroll rotated pin).

Changed-file formatting, Windows-header, GPL, portability (574 reviewed / 0 new), and git diff --check pass. Audit OpenGrep reports 166 pre-existing findings (159 in generated ipc_protocol_requests.h, 5 in p2p_manager.cpp, 2 reviewed getenv reads re-allowlisted after a line shift) — none introduced by this change.

TODO_PREPUSH

AI disclosure

Extensive AI assistance was used for implementation, test generation, and adversarial review. All changes were locally inspected and validated; the umbrella #68 remains blocked by the decision-grade live-performance gate.

Stack

• Base: fix/p2p-wire-hardening / fix(p2p): harden direct-P2P framing and ingress admission #75 at 24923e95835bf31633ff9c54b46fe2a5c0b1e55c
• Head: fix/p2p-robustness at cd956acdbee34e6c560af1a47fdfc1057ad7fa50
• Umbrella: feat(p2p): add authenticated direct replication #68

Do not merge or deploy this child independently.

[Copilot]

🟡 Changes recommended

The new forget() flow currently only removes the peer from the registry and does not revoke the running listener’s allowlist, so “forget” may not take effect immediately without additional lifecycle handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens the direct-P2P CLI/daemon lifecycle by making status output parseable again, bounding potentially dangerous timeouts/intervals at config-validation time, and adding an explicit “forget” operation to enable operator-driven key rotation while preserving fail-closed enrollment semantics.

Changes:

• Add p2p forget <node-id> end-to-end (IPC/proto enum + dispatcher + ServiceManager + P2pManager + registry removal) to support explicit key rotation.
• Make CLI connect/disconnect/forget formatting resilient to malformed daemon payloads and remove ANSI colorization from p2p status key/value output.
• Enforce protocol/config hard ceilings for operation timeouts and direct-transport reconnect interval during validation, with corresponding unit tests.

File summaries

File	Description
tests/unit/daemon/peer_registry_catch2_test.cpp	Adds a key-rotation test covering explicit peer removal then re-enrollment with a new pin.
tests/unit/daemon/p2p_delta_catch2_test.cpp	Adds a regression test ensuring delta exchange rejects timeouts above kP2pMaxOperationTimeout.
tests/unit/daemon/components/config_resolver_test.cpp	Adds a test that rejects overly-large sync_interval_ms for direct transport at config resolution time.
tests/scripts/portability_allowlist.txt	Updates allowlist line references after dispatcher shifts.
src/daemon/p2p/p2p_protocol.cpp	Bounds handshake timeout during config validation.
src/daemon/p2p/p2p_manager.cpp	Adds forget() to remove a peer from the registry.
src/daemon/p2p/p2p_delta.cpp	Bounds delta-exchange timeout during options validation.
src/daemon/ipc/proto_serializer.cpp	Updates enum coverage assert to include the new Forget operation.
src/daemon/components/ServiceManager.cpp	Degrades status when peer-registry reads fail; adds forgetP2pPeer.
src/daemon/components/RequestDispatcher.cpp	Wires new MemorySyncOperation::Forget request handling.
src/daemon/components/ConfigResolver.cpp	Rejects direct-transport reconnect interval (sync_interval_ms) above the 5-minute ceiling during config resolution.
src/cli/commands/p2p_command.cpp	Removes ANSI severity colorization from status output; adds forget subcommand and formatter; hardens connect/disconnect formatting.
include/yams/daemon/p2p/p2p_manager.h	Exposes P2pManager::forget.
include/yams/daemon/ipc/proto/ipc_envelope.proto	Adds FORGET = 9 to the MemorySyncRequest operation enum.
include/yams/daemon/ipc/ipc_protocol_requests.h	Updates generated MemorySyncOperation enum to include Forget.
include/yams/daemon/components/ServiceManager.h	Declares forgetP2pPeer.

Review details

Suppressed comments (2)

src/cli/commands/p2p_command.cpp:137

• If the daemon response is malformed and node_id is missing, this prints disconnected with a blank id. Prefer a visible placeholder to avoid confusing output.

    return "disconnected " + payload.value().value("node_id", std::string{});

src/cli/commands/p2p_command.cpp:148

• If the daemon response is malformed and node_id is missing, this prints forgot with a blank id. Prefer a visible placeholder to avoid confusing output.

    return "forgot " + payload.value().value("node_id", std::string{});

• Files reviewed: 16/16 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.