Repository navigation
feat: harden live macOS runtime and governance - #1317
seonghobae wants to merge 31 commits into
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 |
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
2026-10-03 actual PostgreSQL/browser verificationProtected I tested the complete existing #1317 source Real execution:
Receipt directory (operator-local, not a hosted artifact): #1381's MERGED object |
October 3, 2026 — real DB prerequisite reconciliation, not merged acceptanceThe exact private #1317 af362d5 lineage with #1828 published64eff330 dependency bytes passes241 existing owner regression cases plus1 signed import/list/detail/search cross-user/cross-org isolation case on disposable PostgreSQL17, zero skips. Parent verified1885 unchanged source files, emitted JUnit and final command-log hashes. Repeated diagnostic log labels are explicitly marked superseded; no full historical-log preservation claim. Fresh native bootstrap exits0 but emits unclosed asyncpg connection/transport/socket ResourceWarning, so strict acceptance FAILS. A diagnostic disposal wrapper is not a source repair. Migration and historical-shape upgrade/repeat preserve existing body/read-state values, but raw SQL omitting email_records.is_read still fails because no DB server_default exists. #1782's existing default repair remains needed; explicit is_read test seeds do not clear it. Native Alembic inspection of immutable #1782 migration addition gives heads0018_email_record_read_state +0020_merge_import_registry. Adding current#1503 organization/personal successors gives0020_merge_import_registry +0021_workspace_personal_owner_binding. Current#1503head9151c755 is not ancestor of#1317; original19d5860b is. No existing IDs/edges were deleted/reparented/stamped and no upgrade-heads workaround used. Current registry semantic deltas cannot be claimed adopted from two migration files alone. New owned scratch proposals are addressing only bootstrap executable cleanup and narrow existing-owner default/graph composition, preserving canonical source/dependency/security/CI owners. No duplicate PR, production edit, commit/push or protected merge. Complete source review and actual fresh/existing DB acceptance are required before any owner adoption. Existing CI isolatedcapacity/HOLD remains independent. |
October 3, 2026 — whole composed-source review FAILED; owner escalationFrozen private tree P1 — TENANT-KEY-CROSSES-GLOBAL-EMBEDDING-OVERRIDELocations: P2 — NEW-RUNTIME-ENV-CREDENTIAL-SOURCELocations: P2 — IMPORT-EMBEDDING-WINDOW-BOUND-REMOVEDLocations: P2 — LOCAL-NATIVE-TOKENIZER-404-SILENT-FALLBACKLocations: P2 — CURRENT-CANDIDATE-DOC-CONTRACT-DRIFTLocations: The source-repair/re-review cycle limit has been reached. No further autonomous product edits or publication from this failed candidate are authorized by the review process. Existing canonical source owners must decide the bounded provider/embedding repair direction. All old findings/attempts and immutable source are preserved; no duplicate PR, source write, commit/push, formal approval, protected merge or deployment. Same-origin browser rehearsal also remains failed/unverified: initial build emitted symbol warning and Podman fixture transport failed before DB/browser start. A separate harness-only recovery is running without product edits. It cannot clear this source verdict. Personal orgless signed positive and full Linux frontend acceptance remain unresolved. October 3, 2026 — synthetic final-transport reproduction completedAll five findings are confirmed within a bounded synthetic diagnostic, not customer/provider execution. Preserved run2 completed20 cases/exit0/harness-errors0 and matched1898 frozen source hashes. Actual resolver plus unchanged search/import consumer bodies and installed AsyncOpenAI serialization send the synthetic tenant Authorization to127.0.0.1 when tenant-selected URL isNone and global embedding override islocal. Default-tenant external, explicit external-provider and local-placeholder positives distinguish the destination/key pairing. No real API keys, external connect/DNS or provider calls; one import-time socket.bind was blocked by the audit hook. Exit0 means defect observation completed, NOT safe product acceptance. Synthetic Settings environment credential reached local_environment with registry lookup0. Normal/direct physical requests33 exceed independent ceiling32; partial remainder32+1 is separately bounded. Unknown-local tokenizer404 continued embedding; actual model tokenizer mismatch remains unmeasured. Current documentation sayshttpx2 2.5.0/256characters versus actual2.13.1/256tokens; executed400characters/201tokens/one-chunk control confirms the distinction. Earlier run1 placeholder rejection and later instrumentation run3/4 timeouts remain failed; current diagnostic.py replay reliability is unresolved. Preserved run2 is the completed evidence; historical window-removal attribution was not reverified by this diagnostic's failed protected-blob lookup. Browser harness recovery also STOPPED after its second failure. Podman CLI transport works and genuine build cache removes the no-cache message, but warm pnpm build emitted13WARN rows and unexpectedly recreated/downloaded dependencies despite the requested no-repeat-network boundary. That violation is disclosed, logs retained, and no third rerun. No DB/app/Chromium/UI journey or screenshot executed in either recovery; no browser PASS. No product source edits or unrelated resource cleanup. The complete failed111-path review/patch/binding plus run2 diagnostic, final DB result/JUnit and browser stop log are preserved outside scratch in a hash-verified owner-escalation packet. Source repair cycles remain exhausted. Existing source owner/expert must take a bounded next repair under normal review/security/protection gates; no further autonomous product edits, owner replacement, credential changes, commit/push/merge/deploy. GitHub auth-status returned1 but normal existing gh PR read/write is working; no credentials changed or route bypassed. |
October 4, 2026 KST — verified read-only repair contract ready for existing ownerThe expert handoff is complete and parent-verified, not implemented or approved. Contract SHA256 The contract distinguishes unconfigured/default/explicit endpoint states; binds credentials to the selected endpoint without ambient global override; reuses existing encrypted LLMProvider/TenantConfig rather than inventing a registry API; uses a nonsecret local placeholder until authenticated local ownership is explicitly established; fails closed for unknown-local native tokenizer absence; limits final physical requests to32 and preserves token-weighted physical→semantic plus equal-semantic→source pooling. Completed-prefix batch work must not be submitted again on partial fallback. Exact search/import/grounded-answer/local-engine/orchestrator consumer RED→GREEN matrix and documentation corrections are specified, not executed. Crucially, narrow leakage containment is not approval of Naruon-owned provider/model routing. Existing#1540/contextual-orchestrator#1023 released-client/schema requirement remains, last successful release query returns0. Preserve#1480/#1525/#1384 authority and existing branch writers; no invented CO API, copied wire schema or default routing workaround. Owner decisions required: precise source adopter, endpoint/default compatibility, authenticated-local credential binding and tokenizer/batch authority. Source repair budget remains exhausted; this handoff does not reset it. The existing owner must settle that design/ownership boundary before a new implementation phase. Actual prior DB1966pass/3conditional-skips does not clear the five failed source findings. Latest browser attempts remain unexecuted/failed. No candidate or canonical source edits, product execution, provider/DB access, adoption, commit/push, formal approval, merge or deployment were performed by this contract task. Current canonical1828head64eff330 remains clean; owner coordination reply is still unreceived. |
This branch has not been deployed
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.