feat: harden live macOS runtime and governance - #1317
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds separate local chat and embedding routing, semantic embedding chunking, stricter authentication and governance validation, safer email import locking, canonical read-state migrations, updated macOS Compose detection, and new policy documentation. ChangesLocal runtime and embedding integration
Input, identity, and live-smoke safety
Governance, schema, and extractor defaults
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
PR governance metadata gate is not ready for
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headcf8c21e4b0843aef49f8c15d4f0f93816178dc09. -
Head SHA:
cf8c21e4b0843aef49f8c15d4f0f93816178dc09 -
Workflow run: 31542616436
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Workflow: pr-governance.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: pr-governance.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Backend (30 files)"]
S3 --> I3["API and service runtime"]
I3 --> R3["Review risk: Backend (30 files)"]
R3 --> V3["backend tests"]
Evidence --> S4["Docs (7 files)"]
S4 --> I4["operator or user guidance"]
I4 --> R4["Review risk: Docs (7 files)"]
R4 --> V4["docs review"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Workflow: pr-governance.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: pr-governance.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Backend (32 files)"]
S3 --> I3["API and service runtime"]
I3 --> R3["Review risk: Backend (32 files)"]
R3 --> V3["backend tests"]
Evidence --> S4["Docs (7 files)"]
S4 --> I4["operator or user guidance"]
I4 --> R4["Review risk: Docs (7 files)"]
R4 --> V4["docs review"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headcf8c21e4b0843aef49f8c15d4f0f93816178dc09. -
Head SHA:
cf8c21e4b0843aef49f8c15d4f0f93816178dc09 -
Workflow run: 31542616436
-
Workflow attempt: 2
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Workflow: pr-governance.yml"]
S2 --> I2["GitHub Actions review job"]
I2 --> R2["Review risk: Workflow: pr-governance.yml"]
R2 --> V2["actionlint plus required checks"]
Evidence --> S3["Backend (30 files)"]
S3 --> I3["API and service runtime"]
I3 --> R3["Review risk: Backend (30 files)"]
R3 --> V3["backend tests"]
Evidence --> S4["Docs (7 files)"]
S4 --> I4["operator or user guidance"]
I4 --> R4["Review risk: Docs (7 files)"]
R4 --> V4["docs review"]
|
Actual iCloud mail directory validation remains scoped to the running Colima stack; no private message content or credentials are included. The import embedding path has now been corrected to use content-graph semantic segments (heading/paragraph/structured fields) as primary units, preserving heading context. Only oversized segments are split to the physical provider ceiling, then pooled back to the semantic segment and finally to the existing email/attachment centroid. The contextual-orchestrator batch path receives the same bounded physical inputs. Validation on the local semantic update: backend 1739 passed, 33 skipped; focused embedding/import/batch suites 79 passed; Ruff and git diff --check passed. Local commit 9c1250c; remote PR head a57f8a7. Current-head GitHub Checks and independent review must rerun for this head before normal protected Merge. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Current-head repair record (2026-08-12 UTC):
This addresses the three current actionable review findings. The central trusted-uv coverage repair remains the prerequisite for a fresh exact-head OpenCode review; no predecessor review or check evidence is being transferred. |
|
Follow-up current-head repair (2026-08-12 UTC):
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Current-head CI repair (head
The branch is being rechecked from this exact head. No approval or merge decision is inferred from this repair. |
|
TDD red phase for the next review repairs (head |
|
TDD green implementation at exact head |
|
TDD red phase for the remaining local-tokenizer review (head |
|
Current-head repair at |
Stale review: all review-thread comments on this PR are resolved and the reviewer's cited commit predates the current head, which passes all non-metadata-gate required checks (verified via gh pr checks and the reviewThreads GraphQL query — 0 unresolved threads). Dismissing as superseded per AGENTS.md stale-review guidance.
opencode-agent
Bot
added
area: security
|
Root-repair handoff at Acquisition lines285–298 catches Exception but not asyncio.CancelledError, so cancellation while acquiring bypasses this helper's explicit connection cleanup. Release lines326–338 does not inspect the unlock response and closes normally after uncertainty. Test actual acquisition cancellation, require confirmed unlock, and invalidate uncertain ownership before cleanup can wait or pool the connection. Budget the separate lease connection alongside the already-used work session, including supported one-slot configurations. Add migrated-PostgreSQL tests around real item commits/rollbacks, same-pool readers, independent-replica reacquisition, cancelled acquisition, failed unlock, and retained imported bytes/identities. Keep tests/test_email_import_service.py and tests/test_emails_api.py coverage; mocked query ordering alone is insufficient. #1469 exact worker investigation reproduced the original pool-return defect and records cleanup-order pitfalls. Its305-pass/100%-worker-coverage receipt is not this import path's GREEN. No source takeover, copied implementation, approval, closure, or protected-readiness claim. |
Normally integrate migration owner #1503 at 19d5860 without discarding #1317 history. Preserve both migration branches with a no-DDL merge revision and retain owner-scoped mail and graph identities. Record actual bounded-pool, cancellation, lost-connection and signed API evidence in doctoring; keep ADR Proposed and hosted CI prerequisite #1562 explicit. Candidate verification: 242 focused tests passed against fresh and repeated PostgreSQL migrations; no protected merge or release claimed.
|
Supplemental read-only agent review of |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| spec.loader.exec_module(revision) | ||
| calls = [] | ||
| operations = SimpleNamespace( | ||
| get_bind=lambda: object(), |
| finish_projection.set() | ||
| expected_error = RuntimeError | ||
| with pytest.raises(expected_error): | ||
| await import_task |
| assert await _replica_can_import(observer_engine, owner_key) is False | ||
| acquire_task.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await acquire_task |
| else RuntimeError | ||
| ) | ||
| with pytest.raises(expected_error): | ||
| await release_task |
seonghobae
removed
the
status: needs-review
seonghobae
added
status: draft
ChatGPT Codex Connector
1 participant
Current repair status — Draft / prerequisite-first
Exact head:
af362d58190c0bf2ed122d718473fe3c2bd503c4(tree027eb9d28c2a3677dd11d2ffd878fd4ef1c3fe29).Normal merge parents: original owner
1b422f15e6e5f56be679f691c8ff925c9a420fb1and migration owner #150319d5860bc27e860acba940390f5792721cd99e5e. This retargets onto #1503'sfix/workspace-document-registry-migrationbranch without force push, dropped predecessor delta, or deleted migration IDs.Current repair:
0020_merge_import_registry, use Alembic's native graph inspection, and mark the open-PR local-runtime ADR Proposed.Committed-head verification: 242 passed in 11.41s, exit 0, after fresh and repeated actual PostgreSQL migrations; task-only DB container/network cleanup confirmed. This includes real concurrent same-owner/other-user/other-organization imports and signed backend ASGI import/unsigned rejection. The old API suite also contains mocks; its test names alone do not prove signed DB behavior. Focused Ruff and
git diff --checkpassed; the worktree remained clean.Command from
backend/, after task-isolated DB provisioning:JUnit
import_af362d5.xml: SHA-256238da7ad1c6e772cd087a3576f7c81d4d81780a6f0f0b4b3986a229910da965c(local receipt, not uploaded hosted evidence).Decision, RED receipts, replay source and limits.
Remaining gates: #1503 predecessor integration; #1562 migrated-PostgreSQL Application CI prerequisite (owner request); fresh required checks and qualifying independent review on the exact head. Local success is not hosted approval, protected merge, deployed provider/browser behavior, or p95/100% coverage acceptance. Existing default-branch dependency findings also remain required security work, not waived by these tests.
Caller precondition remains settled/read-only work before import: the pending-ORM guard cannot detect already-flushed or raw-SQL uncommitted writes. The actual provider-lookup caller is verified; do not generalize to arbitrary external transactions.
Preserved earlier PR record
The following is historical proposal/validation material retained for provenance. Earlier counts and live-mail/model/browser observations do not transfer to the current head and have not been rerun in this repair.
Summary
mlx-lm->llama.cpp-> Ollama.Historical validation
1739 passed, 33 skippedafter semantic-segment embedding update (a57f8a75); current-head CI reruns after each branch update427 passed; lint and typecheck passedactionlint, shell syntax checks, andtest_pr_governance_gate.sh: passedSecurity boundary
Privileged workflow logic is materialized only from a trusted full SHA. PR head content is handled as data; archive traversal, links, devices, and workspace escapes are rejected. No admin merge or branch-protection bypass is used.
Summary by CodeRabbit
Live mail validation update
The actual iCloud mail directory supplied by the operator was exercised through the running Colima stack without copying private mail into the repository. Real-mail import completed with zero failed items, same-owner sequential imports released all PostgreSQL advisory locks, and inbox/search visibility was verified through both the backend API and the same-origin frontend cookie proxy. Private message contents and credentials were not included.
Semantic embedding update
Import embeddings now use the content-graph parser's heading/paragraph/structured-field segments first. Only an oversized semantic segment is physically split for provider safety; its vectors are pooled back to the segment, then to the existing email/attachment source centroid. The persisted content segments remain the Ontology and Project Graph citation units.