Finish AgentCTL, Gateway, desktop, and Polylogue runtime integration - #12
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: e4697adea6
ℹ️ 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".
| self._operation_identity_key( | ||
| project, operation, parameter_digest, principal, environment, tree, checkout | ||
| ) |
There was a problem hiding this comment.
When otherwise identical verification requests carry different bead_binding contracts—or a bound request follows an unbound cached check—this identity key is unchanged because it hashes only the operation, parameters, principal, tree, and environment. The later request is therefore coalesced with or reuses the earlier job, whose persisted contract belongs to another bead or has no binding; packet delivery then cannot obtain the authoritative matching verifier and rejects an otherwise valid completion. Include the normalized contract in the reuse identity or disable coalescing/cache reuse for contract-bound jobs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 794fb598ff
ℹ️ 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".
| coalesce_key = ( | ||
| self._operation_identity_key( | ||
| project, operation, parameter_digest, principal, environment, tree, checkout | ||
| ) | ||
| if operation.service is None or operation.cache == "tree+environment" | ||
| else None |
There was a problem hiding this comment.
When two clean workspaces have the same tree and start the same non-service cache = "none" operation concurrently, this newly enabled coalescing computes the same key because _operation_identity_key includes checkout identity only for service operations. The second request consequently receives the first workspace's job record while the command executes in the first workspace; this makes its verification unusable for delivery and can run mutating operations against the wrong checkout. Include the checkout identity in active-operation keys for checkout-backed jobs.
Useful? React with 👍 / 👎.
| packet_job_id = arguments.get("packet_job_id") | ||
| return self.delivery.publish( | ||
| *publish_arguments, | ||
| **({"packet_job_id": packet_job_id} if isinstance(packet_job_id, str) else {}), |
There was a problem hiding this comment.
When an API client includes packet_job_id with a non-string value, this branch silently drops the field and calls ordinary publication instead of rejecting the request. The same pattern exists in workspace.land, so a serialization or client bug can bypass the requested Beads binding and write-scope checks and still publish or merge using only the ordinary verifier. Validate a present packet ID as a non-empty string and fail the request otherwise.
Useful? React with 👍 / 👎.
| publication_snapshot = ( | ||
| self.workspaces.delivery_snapshot( | ||
| workspace_id, project.workspace.default_base, scope=scope, merge_base=True | ||
| ) | ||
| if packet is not None |
There was a problem hiding this comment.
When the branch already has a PR targeting a base other than workspace.default_base, publication reuses that PR and landing accepts it because baseRefName is never checked, while this scope snapshot is always computed against the configured default base. A commit present on the default base but absent from the PR's real base is therefore omitted from publication_snapshot, allowing the PR to merge out-of-scope changes despite the packet check. Require the review base to match the configured base or compute the scope from the authoritative review base.
Useful? React with 👍 / 👎.
Use the installed polylogue-hook for Claude and Codex without baked sidecar paths. Make parity tests exercise the generated archive root and remove the wrapper-only Home Manager surface.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4425cbd372
ℹ️ 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".
| binding = value.get("bead_binding") | ||
| if isinstance(binding, Mapping) and "write_scope" in binding: | ||
| _seal_packet_result(value, checkout, result_path) |
There was a problem hiding this comment.
When a bead carries write_scope, this branch always calls _seal_packet_result, which JSON-decodes the backend's entire last message and later requires the exact anti_vacuity, unresolved_work, delegation, deletion_evidence, and evidence_only fields. However, Runtime.v2_run_for_bead only asks the worker to report evidence and residuals and never supplies this schema or requires raw JSON. A normal prose or fenced-JSON final response therefore raises RunnerError after otherwise successful work and marks the packet job failed, preventing it from being used for publication or landing; include the exact raw-JSON result contract in the agent prompt or use a structured result channel.
Useful? React with 👍 / 👎.
| metadata = bead.get("metadata") | ||
| encoded_scope = ( | ||
| metadata.get("write_scope") if isinstance(metadata, Mapping) else None | ||
| ) |
There was a problem hiding this comment.
When this path reads a real task through BeadsService.get, _normalize places native properties—including metadata—under bead["fields"]; it never emits top-level metadata. Consequently a production bead's write_scope is never copied into the binding (the added test masks this by mocking a non-normalized top-level shape), so the runner does not seal a packet result and subsequent workspace.publish or workspace.land with that packet is rejected for lacking an authoritative scoped binding. Read fields.metadata from the normalized bead contract.
Useful? React with 👍 / 👎.
| final_head = observed.stdout.strip() | ||
| if ( | ||
| observed.returncode != 0 | ||
| or len(final_head) != 40 | ||
| or any(value not in "0123456789abcdef" for value in final_head) |
There was a problem hiding this comment.
For a checkout initialized with Git's SHA-256 object format, git rev-parse HEAD returns a valid 64-character object ID, but this validation rejects every such packet before sealing. GitHubDelivery._packet repeats the same 40-character restriction, even though the surrounding runtime does not constrain registered checkout heads to SHA-1 and task merge evidence explicitly accepts both 40- and 64-character IDs. Accept the repository's actual object-ID format in both validations, or validate the observed value directly against the recorded checkout head.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
sinnix/pkgs/sinnixd/sinnixd/workspaces.py
Line 1013 in 8d3e12d
Bound retained workspace checkpoints
Each checkpoint call creates a fresh UUID-backed directory and can persist up to 64 MiB, but there is no per-workspace count/aggregate limit, deduplication, expiry, or pruning until the entire workspace record is removed. A retry loop or long-lived workspace can therefore grow daemon-owned state without bound and eventually exhaust the filesystem, disrupting checkpoints and other sinnixd state; enforce an aggregate retention policy before persisting another checkpoint.
AGENTS.md reference: AGENTS.md:L75-L80
ℹ️ 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".
| return { | ||
| **result, | ||
| "ref": REGISTRY.reference("job", {"job_id": values["job_id"]}), | ||
| "target": wait_target.value, | ||
| } |
There was a problem hiding this comment.
When a job remains nonterminal through the requested deadline, sinnixd returns wait_timed_out: true, but this MCP-facing async branch returns that response verbatim without the normalized outcome: "timeout", evidence, source revision, and continuation used by other waits. Clients waiting through the public async tool can therefore interpret an expired wait as an ordinary successful observation; the synchronous sibling does not compensate because it checks the nonexistent timed_out field instead.
Useful? React with 👍 / 👎.
1 participant
Consolidates the outstanding Sinnix runtime work: durable scratch, active-operation coalescing, typed AgentCTL task and packet contracts, bounded Gateway effector discovery, Polylogue tmpfs sizing, and the Noctalia notification-effect regression fix. Exact-head lint is green; the complete Sinnix check is queued behind the active Lynchpin promotion and remains required before landing.