fix: code quality, security hardening, and test coverage#9
Conversation
- Reject migration when WASM hash length != 32 instead of silently skipping verification (security: prevented hash bypass) - Use errors.Is for sentinel error comparison in checkpoint loading - Guard manifest path slicing with strings.HasSuffix to prevent panic on non-.wasm paths - Close replay engine compilation cache to prevent memory leak - Return TickSnapshot copy from LatestSnapshot to prevent pointer invalidation after replay window eviction - Add 10s read deadline on p2p ping handler and 30s per-peer timeout on bootstrap to prevent connection stalls - Protect nodeCapabilities with RWMutex to prevent data race between SetNodeCapabilities and handleIncomingMigration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Sort Names() output to match docstring contract - Return slice copy from History() to prevent internal state mutation - Wrap error in hostcall registry instantiation - Warn on oversized log_emit instead of silent drop - Remove unused AgentStarted.StartTime field (code + docs) - Add tests for internal/p2p/ (node lifecycle, ping, bootstrap) - Add tests for cmd/igord/ (manifest loading, escalation policy) - Add WASM agent build step to CI workflow - Update IMPLEMENTATION_STATUS.md with optimizations and hardening tables - Update CLAUDE.md to reflect Phase 3 completion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00e01d0959
ℹ️ 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".
| - name: Build WASM agent | ||
| run: make agent |
There was a problem hiding this comment.
This new step will fail on runners that do not already have TinyGo installed (including typical ubuntu-latest images), because make agent exits with an error when tinygo is missing (Makefile target agent checks which tinygo and scripts/bootstrap.sh only treats TinyGo as optional). As written, the workflow now adds a hard dependency without provisioning it, so CI can fail even after all prior checks pass.
Useful? React with 👍 / 👎.
1 participant
Summary
Comprehensive code review fixes addressing security, resource safety, and test coverage across the runtime.
Changes
Security & Resource Safety
strings.HasSuffix/TrimSuffix(no panic)SetNodeCapabilitieswrite access in migration serviceerrors.Is()forErrCheckpointNotFoundcomparisonResource Cleanup & Correctness
defer replayEngine.Close()in main tick loopCode Organization
loadManifestData()helper in CLI entry pointTests & CI
cmd/igord/main_test.gocovering manifest loading and escalation policyinternal/p2p/node_test.gocovering bootstrap, ping round-trip, error casesDocumentation
StartTimefield fromAgentStartedStartTimefrompkg/protocol/messages.go