-
Notifications
You must be signed in to change notification settings - Fork 0
Prepare codex live readiness updates #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ensure codex in-flight ends within 2.5s of completion, pick active JSONL per process, and make opencode message completion authoritative. Remove wheel passive listener warnings and align docs/tests.
Start the server on the next available port during dev and ignore stale OpenCode status events when determining in-flight state.
There was a problem hiding this comment.
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: a968a74f69
ℹ️ 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".
| if (typeof status === "string" && status.trim()) { | ||
| state.lastStatus = status; | ||
| state.lastStatusAt = now; | ||
| if (IDLE_STATUS_RE.test(status)) { | ||
| state.inFlight = false; | ||
| state.lastInFlightSignalAt = undefined; | ||
| } else { | ||
| state.inFlight = true; | ||
| state.lastInFlightSignalAt = now; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid marking every non-idle status as in-flight
The new status handling forces inFlight = true for any status string that is not matched by IDLE_STATUS_RE. This overrides the earlier summarizeEvent logic that only treats BUSY statuses as in-flight, and it will incorrectly keep sessions active if OpenCode emits non-busy statuses such as "connected", "ready", or other lifecycle states. In those cases the UI will show a session as in-flight indefinitely until an explicit idle status arrives, which is a regression from the previous behavior. Consider checking BUSY_STATUS_RE here instead of treating all non-idle statuses as busy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function packCell(cx: number, cy: number): number { | ||
| const ux = (cx + CELL_OFFSET) & 0xffff; | ||
| const uy = (cy + CELL_OFFSET) & 0xffff; | ||
| return ((ux << 16) | uy) >>> 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Spatial-hash cell packing wraps after ~32k grid units, causing collisions/overlaps for large layouts
packCell() stores cx/cy in 16-bit lanes after adding a fixed offset and masking with 0xffff. This means coordinates outside the range [-32768, 32767] wrap around and collide with unrelated cells.
Actual vs expected:
- Expected: cell keys remain unique as the layout grows (even for many agents or far-from-origin anchors).
- Actual: once the spiral frontier (or group anchors) move beyond the representable range, different cells produce the same packed key, so the spatial index can think two distant tiles are in the same bucket and either (a) falsely detect collisions (preventing placement) or (b) miss collisions (allowing overlaps), depending on the wrapped mapping.
Click to expand
Code:
const ux = (cx + CELL_OFFSET) & 0xffff;
const uy = (cy + CELL_OFFSET) & 0xffff;
return ((ux << 16) | uy) >>> 0;public/src/lib/layout.ts:98-101
With CELL_OFFSET = 32768, any cx such that cx + 32768 exceeds 65535 (or is negative) will wrap due to & 0xffff.
Impact:
- Very large numbers of agents, or any pathologically large anchor/spiral traversal, can degrade placement stability and violate the “never overlap / never move existing tiles” guarantees.
Recommendation: Avoid 16-bit packing for coordinates that can grow unbounded. Options: (1) use a string key ${cx},${cy}; (2) use a 64-bit bigint key (BigInt(cx) << 32n) ^ BigInt(cy); or (3) keep separate 32-bit hashes with low collision probability and handle collisions via per-cell maps keyed by full coords.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Superseded by fix/activity-state; live readiness changes consolidated. |
Summary
Testing
Ensure you read the contribution guide before submitting a commit and follow the exact guidelines.