Skip to content

fix: comprehensive bugfixes — 53 issues resolved#2

Open
NeutronZero wants to merge 1 commit into
nicepkg:mainfrom
NeutronZero:fix/comprehensive-bugfixes
Open

fix: comprehensive bugfixes — 53 issues resolved#2
NeutronZero wants to merge 1 commit into
nicepkg:mainfrom
NeutronZero:fix/comprehensive-bugfixes

Conversation

@NeutronZero

Copy link
Copy Markdown

Summary

Comprehensive bugfix addressing 53 identified issues across the AI Peer codebase. All changes are backwards-compatible and use only Python stdlib.

Critical Fixes (5)

# File Fix
1 ws_client.py Absolute import → relative import
2 server.py PID file encoding + write after bind (race condition)
3 client.py Port-check loop after daemon restart (race condition)
4 spawn.py Temp file cleanup via atexit (resource leak)
5 server.py Localhost-only check for /api/shutdown (DoS prevention)

High Fixes (9)

# File Fix
6 helpers.py MD5 → SHA-256 for content dedup
7 client.py Log rotation (1MB limit)
8 ops_ai.py Validate human_secret before HMAC
9 ops_chat.py Log WebSocket failures instead of silent pass
10 crypto.py Catch InvalidToken specifically
11 db.py Document WAL read safety
12 ops_room.py int() validation for --port
13 server.py 100KB per-message content limit
14 ops_peer.py Consistent peer ID generation

Medium Fixes (12)

# File Fix
15 init.py Return exit code from output() (testability)
16 ops_chat.py int() validation for -n flag
17 ops_ai.py int() validation for --timeout/--rounds
18 spawn.py atexit for temp file cleanup
19
elay_client.py Always send peer_name/peer_tool
20 db.py Move SELECT inside lock
21 ops_room.py Relay warning → info level
22 constants.py Fallback chain for get_machine_id()
23 ops_chat.py Join poller thread on exit
24 helpers.py getpass.getuser() fallback for identity
25 client.py Consistent dict return from _req()

Low Fixes (16)

  • Socket timeout + ping/pong keepalive
  • Copy message before mutating
  • Dynamic @mention regex from TOOL_COMMANDS
  • --name flag for discuss command
  • Pass context in quick invites
  • Try HTTP shutdown before taskkill /F
  • Close DB before temp dir cleanup

Documentation (3)

  • PRD.md: Fix argparse reference → manual dispatch
  • PRD.md: Update test count 40 → 69
  • SKILL.md: Clarify 16 sub-commands vs 12 top-level commands

Test Fixes

  • conftest.py: Try HTTP shutdown before taskkill /F
  • est_db.py: Close DB before temp dir cleanup (Windows PermissionError)

Test Results

All 66 tests passing on Windows with Python 3.10+.

Changes

18 files changed, 247 insertions(+), 68 deletions(-)

Critical fixes:
- ws_client.py: absolute import → relative import
- server.py: PID file encoding + write after bind (race condition)
- client.py: port-check loop after daemon restart (race condition)
- spawn.py: temp file cleanup via atexit (resource leak)
- server.py: localhost-only check for /api/shutdown (DoS prevention)

High fixes:
- helpers.py: MD5 → SHA-256 for content dedup
- client.py: log rotation (1MB limit)
- ops_ai.py: validate human_secret before HMAC
- ops_chat.py: log WebSocket failures instead of silent pass
- crypto.py: catch InvalidToken specifically
- db.py: document WAL read safety
- ops_room.py: int() validation for --port
- server.py: 100KB per-message content limit
- ops_peer.py: consistent peer ID generation

Medium fixes:
- __init__.py: return exit code from output() (testability)
- ops_chat.py: int() validation for -n flag
- ops_ai.py: int() validation for --timeout/--rounds
- spawn.py: atexit for temp file cleanup
- relay_client.py: always send peer_name/peer_tool
- db.py: move SELECT inside lock
- ops_room.py: relay warning → info level
- constants.py: fallback chain for get_machine_id()
- ops_chat.py: join poller thread on exit
- helpers.py: getpass.getuser() fallback for identity
- client.py: consistent dict return from _req()

Low fixes:
- ws_client.py: socket timeout + ping/pong keepalive
- ops_chat.py: copy message before mutating
- ops_ai.py: dynamic @mention regex from TOOL_COMMANDS
- ops_room.py: --name flag for discuss command
- ops_ai.py: pass context in quick invites
- conftest.py: try HTTP shutdown before taskkill /F
- test_db.py: close DB before temp dir cleanup

Documentation:
- PRD.md: fix argparse reference → manual dispatch
- PRD.md: update test count 40 → 69
- SKILL.md: clarify 16 sub-commands vs 12 top-level commands

@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: 21eb9bf75f

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

cmd = []
for part in config["cmd"]:
if part == "{prompt}":
cmd.extend(["--file", _tmp_prompt_file])

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 Avoid passing Codex prompts via --file

On Windows, any non-stdin tool with a prompt over 32 KB takes this branch, so codex is invoked as codex exec --full-auto --file <tmp>. The official Codex CLI reference documents PROMPT as string | - (read stdin) for codex exec, not a --file flag (https://developers.openai.com/codex/cli/reference#codex-exec). In Windows rooms with enough history/context, inviting Codex will fail before it can respond; restrict this fallback to tools that actually support file attachments or use stdin (-) for Codex.

Useful? React with 👍 / 👎.

Comment on lines +49 to +50
pid = make_ai_peer_id(tool)
return client.register_peer(pid, name, ptype, tool, get_machine_id(), host, port)

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 Preserve the remote host in registered peer IDs

For the documented manual remote registration flow (peer register ... --host 192.168.1.50), this now generates the ID from the local machine and stores the local machine in the machine field. Registering a remote Codex peer therefore collides with/overwrites the local Codex peer or any other remote Codex peer instead of creating a host-specific peer, so peer list loses the remote endpoint metadata needed to distinguish participants.

Useful? React with 👍 / 👎.

Comment on lines +169 to +170
if len(b["content"]) > 100 * 1024: # 100KB per-message limit
return self._json({"error": "message content too large (max 100KB)"}, 413)

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 Surface oversized local message rejects

When a public-room message hits this new 413 path, _post_message treats the returned {'error': ...} as a truthy local message and still writes the content to the relay without a local message id. For messages or AI responses over 100KB, the CLI can report success while SQLite never stores the message (and local-only sends also return ok: true with an embedded error), so local history/export diverges from the relay. Raise on the local rejection before the relay write or enforce this limit before returning success.

Useful? React with 👍 / 👎.

Comment on lines +101 to +104
except socket.timeout:
# Idle timeout — send ping to keep connection alive
self._send_ping(b"")
continue

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 Mark dead WebSockets before skipping polling

On an idle timeout this loop sends a ping and immediately continues, while _send_ping swallows send failures and no pong is required before keeping ws_connected true. In interactive relay chats where the network path becomes half-open or the relay stops responding without closing the TCP socket, the poller keeps skipping HTTP polling indefinitely, so new messages are not displayed until the process is restarted. Treat a missed ping/pong or send failure as a disconnect so the existing polling fallback can run.

Useful? React with 👍 / 👎.

Comment on lines +191 to +193
try:
import getpass
username = getpass.getuser()

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 Keep fallback identities stable when env vars are absent

When neither USER nor USERNAME is set, previous versions generated the human id with the stable fallback name user; this new fallback commonly returns the OS account such as root. Upgrading in that environment rewrites an existing *-human-user identity to *-human-root, so prior room membership and relay identity no longer match the user's existing peer. Preserve the old fallback for existing identities or only use getpass when creating a brand-new identity.

Useful? React with 👍 / 👎.

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