feat: implement map plugin data by persona name - #11348
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e632ea19-eb4c-429a-90c2-27df637058e4) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 5/5
- In
backend/scripts/web.py, the new persona grouping/UID injection and missing-input-file paths currently lack hermetic regression coverage, so future edits could silently alter file transforms or error handling and ship incorrect outputs; add focused hermetic tests for those branches to lock behavior down.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/scripts/web.py">
<violation number="1" location="backend/scripts/web.py:38">
P3: Add hermetic coverage for persona grouping/UID injection and the missing-input-file branch; this new file-transforming behavior otherwise has no regression guard.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return uids | ||
|
|
||
|
|
||
| def map_plugin_data_by_persona_name() -> None: |
There was a problem hiding this comment.
P3: Add hermetic coverage for persona grouping/UID injection and the missing-input-file branch; this new file-transforming behavior otherwise has no regression guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/scripts/web.py, line 38:
<comment>Add hermetic coverage for persona grouping/UID injection and the missing-input-file branch; this new file-transforming behavior otherwise has no regression guard.</comment>
<file context>
@@ -35,9 +35,36 @@ def process_user(uid: str) -> None:
return uids
+def map_plugin_data_by_persona_name() -> None:
+ try:
+ with open("user_messages_with_bot_name.json", "r") as f:
</file context>
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
![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: 9c00a20edb
ℹ️ 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".
| message_with_uid = message.copy() | ||
| message_with_uid["uid"] = uid | ||
| plugin_data_by_persona[bot_name].append(message_with_uid) |
There was a problem hiding this comment.
Whenever this function runs, it copies records from user_messages_with_bot_name.json into the new output without ever reading the authoritative plugins_data collection (backend/database/apps.py:30). Consequently, plugin_data_by_persona_name.json contains chat-message documents plus a UID—not persona/plugin metadata—so downstream analysis expecting plugin data receives the wrong dataset despite the success message. Query the plugin documents and index their data by the persona name field instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
Git-on-my-level
added
needs-tests
|
Thanks for the focused cleanup here. I reviewed the single changed file,
I am not formally approving because this is still a backend script/data-export path, and the new JSON transform has no hermetic regression coverage. A small test or documented sample input/output for the grouping, Leaving this as a positive implementation signal, with Automated maintainer review by by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
undivisible
added
human
Git-on-my-level
removed
python
workflow-review
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e9212a3e-950e-47ac-9350-5da2ab1c565a) |
Git-on-my-level
added
positive-signal
|
Thanks for the follow-up here — this addresses the coverage gap from the earlier review. I reviewed both changed files on the current head:
Verification I ran locally:
Result: I’m removing Automated maintainer review by by AI on behalf of David — leaving final merge readiness to human maintainer review for this backend data-export script path. |
7d9d50e to
ab2b479
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_aeca79af-7654-4d07-a4ae-af10bb177eb9) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab2b479d28
ℹ️ 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".
ab2b479 to
b3218ad
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_54c6528d-cff2-4c54-9b68-436cf3efdbdb) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3218ad20d
ℹ️ 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".
| def get_user_messages_with_bot_name() -> List[str]: | ||
| user_messages_with_bot_name: Dict[str, List[Dict[str, Any]]] = {} | ||
| uids = get_users_uid()[:20] | ||
| uids = get_users_uid() |
There was a problem hiding this comment.
When this is run against the production users collection, removing the 20-user cap makes get_users_uid() materialize every UID, ThreadPoolExecutor.map() eagerly submit work for all of them, and user_messages_with_bot_name retain every matching message until the final JSON write. On a large database this can exhaust process memory or generate an unexpectedly large Firestore read burst before producing any output; iterate through bounded user batches and flush results incrementally instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ab184a93-4f04-460e-b8aa-79335f4ecd8f) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec90cbc9a8
ℹ️ 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".
| persona_name = plugin_data.get("name") | ||
| if not isinstance(persona_name, str) or persona_name not in persona_uids: |
There was a problem hiding this comment.
When two personas share a display name, or a persona is renamed after messages are saved, this join assigns every same-named plugin document to the user or drops the usage entirely. The web writer stores the stable pluginId in each message (web/personas-open-source/src/app/chat/page.tsx:165-171), while persona creation does not make name unique (backend/routers/apps.py:899-905), so use the plugin ID for the join and reserve name for the output label.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
| for uid in sorted(persona_uids[persona_name]): | ||
| # Include uid in the mapped data to keep track of who the message belongs to | ||
| plugin_data_with_uid = plugin_data.copy() | ||
| plugin_data_with_uid["uid"] = uid |
There was a problem hiding this comment.
For normal plugin documents, uid already identifies the owner and is set during persona creation; overwriting it here with the user who sent a message silently corrupts that plugin metadata and makes creator and consumer indistinguishable in the export. Keep the original uid and add the chatting user under a separate field such as user_uid.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
| persona_uids.setdefault(bot_name, set()).add(uid) | ||
|
|
||
| plugin_data_by_persona: Dict[str, List[Dict[str, Any]]] = {} | ||
| for plugin_document in db.collection("plugins_data").stream(): |
There was a problem hiding this comment.
The new mapper reads through the module-level legacy db proxy, preventing the function from using the repository's injectable Firestore seam and forcing tests to replace the imported module. Accept an optional keyword-only client and otherwise call get_firestore_client() when the mapper runs.
AGENTS.md reference: backend/AGENTS.md:L193-L193
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
| for start in range(0, len(uids), USER_BATCH_SIZE): | ||
| with ThreadPoolExecutor(max_workers=USER_BATCH_SIZE) as executor: | ||
| list(executor.map(process_user, uids[start : start + USER_BATCH_SIZE])) |
There was a problem hiding this comment.
When production contains enough users or messages, this loop only caps simultaneous Firestore reads: get_users_uid() is still fully materialized and every batch continues accumulating its matching documents in user_messages_with_bot_name until the final JSON dump. Fresh evidence in this revision is that the newly added executor batching has no per-batch flush, so memory remains proportional to the entire export and can still be exhausted; write each batch incrementally or merge bounded temporary files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Kept as-is: the executor batch already caps Firestore read concurrency (USER_BATCH_SIZE=20), and the JSON dump is the script's deliverable, so total memory is bounded by the export itself. Incremental per-batch flush would require temp-file merging with no user-facing benefit for this local analysis script.
| continue | ||
| for uid in sorted(persona_uids[persona_name]): | ||
| # Include uid in the mapped data to keep track of who the message belongs to | ||
| plugin_data_with_uid = plugin_data.copy() |
There was a problem hiding this comment.
When a matched plugins_data document belongs to an OAuth-enabled app, copying the complete document also copies external_integration.mcp_oauth_tokens, which stores client_secret, access_token, and refresh_token (backend/routers/apps.py:1895-1903,1995-2017). The subsequent JSON write creates a plaintext credential export, and .gitignore only prevents Git tracking rather than disclosure from the file itself; project only the metadata needed for analysis or explicitly remove secret-bearing fields before serialization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the updated branch. See the rebased commits (b3218ad/ec90cbc9a8/f68c0b79a8) and the hermetic tests in backend/tests/unit/test_web_persona_export.py.
Git-on-my-level
removed
the
positive-signal
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the iteration here — I reviewed the current head (ec90cbc9a84a6f3e60a1835977b0cecaf533ba77) and the implementation is close, but one merge-blocking repository contract still needs to be fixed before this should land.
File-by-file notes:
.gitignore: adding**/user_messages_with_bot_name.jsonand**/plugin_data_by_persona_name.jsonis the right direction for this local export workflow; these generated Firestore-derived JSON artifacts should not be committed.backend/scripts/web.py:get_user_messages_with_bot_name()now intentionally scans all UIDs and batches workers withUSER_BATCH_SIZE = 20, which avoids one unbounded executor over the whole user base. The newmap_plugin_data_by_persona_name()also keeps the export local/offline: it readsuser_messages_with_bot_name.json, builds persona-name to UID sets frombotName, streamsplugins_data, copies each matched plugin document, injectsuid, and writesplugin_data_by_persona_name.json. I did not find a production request/auth path change here.backend/tests/unit/test_web_persona_export.py: the added hermetic tests stubdatabase._client, cover plugin document grouping plus UID injection, cover the missing-input-file branch, verify the all-user scan writes only messages withbotName, and assert worker stream failures propagate without writing a partial export.
Validation:
- GitHub CI currently fails
PR Metadata Preflightonfailure-class-protocol: the branch includesfix(scripts): map persona exports from plugin data, and fix-prefixed commits need aFailure-Class: FC-<slug> | new | nonedeclaration. - I also ran a no-secrets local smoke of the changed export functions against stubbed Firestore collections; it passed for all-user message export, plugin-data mapping, and missing-input handling.
- The repo
backend/test.shfocused run could not execute in this local checkout because backend test dependencies are not installed here (ModuleNotFoundError: No module named 'google'fromtests/unit/conftest.py), so I am relying on the CI backend gate plus the manual hermetic smoke for execution evidence.
Please add the required failure-class declaration (or adjust the commit subject if this is not intended to be a fix) and get the failing preflight green. I’m removing the positive-signal label for now because CI is still red; the backend implementation itself looks reasonable once that metadata gate is satisfied.
Automated maintainer review by glm-5.2.
by AI on behalf of David — leaving final merge readiness to human maintainer review after the repository preflight contract is green.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b238f6f8-2694-4869-b133-681f11efecb0) |
Resolved on the current head: the PR Metadata Preflight/failure-class gate is now green.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_087ab789-27f2-4ee1-9b64-6340deea1c29) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03507336ee
ℹ️ 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 isinstance(integration, dict) and "mcp_oauth_tokens" in integration: | ||
| plugin_data_with_uid["external_integration"] = { | ||
| key: value for key, value in integration.items() if key != "mcp_oauth_tokens" | ||
| } |
There was a problem hiding this comment.
When a matched app uses query-string authentication in external_integration.webhook_url, this filter removes only mcp_oauth_tokens and copies the webhook URL unchanged into the plaintext export. Fresh evidence after the OAuth-token fix is backend/utils/app_integrations.py:718-719, which explicitly documents that configured webhook URLs can contain auth tokens; omit these URLs or export an allowlisted metadata projection so running the analysis does not leave reusable integration credentials on disk.
Useful? React with 👍 / 👎.
| with ThreadPoolExecutor(max_workers=USER_BATCH_SIZE) as executor: | ||
| list(executor.map(process_user, uids[start : start + USER_BATCH_SIZE])) |
There was a problem hiding this comment.
When the full-user export spans multiple batches, this creates and tears down a fresh 20-thread pool for every batch, bypassing the repository's shared executor sizing and observability while adding pool churn throughout the run. Submit the Firestore work to the shared db_executor instead of constructing ad-hoc executors.
AGENTS.md reference: backend/AGENTS.md:L283-L291
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b3765121b
ℹ️ 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".
|
|
||
| client = firestore_client if firestore_client is not None else get_firestore_client() | ||
| plugin_data_by_persona: Dict[str, List[Dict[str, Any]]] = {} | ||
| for plugin_document in client.collection("plugins_data").stream(): |
There was a problem hiding this comment.
When plugins_data.stream() raises during a transient Firestore failure, this function propagates the exception but leaves any plugin_data_by_persona_name.json from a previous run untouched. The missing-input path now removes stale output, but this failure can occur after the current user-message export has completed, leaving an apparently valid plugin export that represents older input and may be consumed as the current result; remove it before streaming or write to a temporary file and atomically replace the prior artifact only after success.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_77cd8c18-2599-4d40-a804-22c169baa70e) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68a8fca65b
ℹ️ 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".
| # Manual only: automatic push/schedule tagging was cutting a new macOS candidate | ||
| # tag on nearly every desktop-affecting main merge and every 15 minutes. Plan and | ||
| # publish a candidate deliberately via workflow_dispatch; qualification/promotion | ||
| # workflows remain unchanged once a tag exists. | ||
| workflow_dispatch: |
There was a problem hiding this comment.
When macOS changes merge to main, this on: block now has neither a push nor a schedule trigger, and a repo-wide workflow search finds no other job that automatically invokes the candidate planner. Fresh evidence in this exact commit is the deletion of the parent revision's hourly schedule, so candidates, Codemagic builds, qualification, and beta promotion now start only after a manual dispatch, contrary to the repository's automatic candidate pipeline.
AGENTS.md reference: AGENTS.md:L126-L129
Useful? React with 👍 / 👎.
| while True: | ||
| if await request.is_disconnected(): | ||
| break | ||
| yield f"event: ping\ndata: {{}}\n\n" | ||
| await asyncio.sleep(30) |
There was a problem hiding this comment.
When MCP clients issue GET /v1/mcp/sse, this loop sends no protocol data and holds one Cloud Run request/concurrency slot until disconnect or the one-hour request timeout. The parent revision explicitly returned 405 because these idle streams exhausted serving concurrency and caused tool-call tail latency and no available instance failures; restoring the infinite ping stream reintroduces that production-capacity incident for every connected client.
Useful? React with 👍 / 👎.
| prepare_google_credentials() | ||
| # Production safety: only override project/database when pointed at a local | ||
| # Firestore emulator. Without FIRESTORE_EMULATOR_HOST set (i.e. real Firestore), | ||
| # never let bare GOOGLE_CLOUD_PROJECT (often the GKE compute project) repoint | ||
| # the customer-data client away from SERVICE_ACCOUNT_JSON's project_id. | ||
| # defer entirely to default resolution so env vars cannot repoint prod Firestore. |
There was a problem hiding this comment.
When the development desktop backend runs under desktop_backend_auto_dev.yml, it sets GOOGLE_CLOUD_PROJECT to the development project, removes SERVICE_ACCOUNT_JSON/GOOGLE_APPLICATION_CREDENTIALS, and mounts the production Firebase credential only for authentication. Falling through to plain firestore.Client() therefore sends subscription, BYOK, quota, and usage reads/writes to development Firestore instead of the production customer project; signed Beta users can be treated as free or have usage recorded under the wrong project. Preserve the removed customer-data client and use it at those entitlement boundaries.
Useful? React with 👍 / 👎.
| MEMORY_ENABLED_USERS: | ||
| value: '' |
There was a problem hiding this comment.
When production serves /v1/knowledge-graph/canonical, an empty MEMORY_ENABLED_USERS value does not disable that route: _read_canonical_graph_page_once() unconditionally calls _canonical_graph_cursor_secret(), which raises missing_cursor_secret before reading any state. This revision removes MEMORY_V3_CURSOR_SECRET and its version from both production serving configurations, so every canonical graph request returns 503 and enrolled /v3/memories reads also fail closed; retain the HMAC secret bindings even while rollout flags are off.
Useful? React with 👍 / 👎.
| --label m1-pre-tag-readiness \ | ||
| --heartbeat-seconds 45 \ | ||
| --timeout-seconds 11400 \ | ||
| --timeout-seconds 7200 \ |
There was a problem hiding this comment.
On a cold or slow M1 readiness run, pre-tag-readiness.sh can legitimately consume sequential watchdog budgets of 1200, 3600, 1800, and 3600 seconds (10,200 seconds total), but this outer watchdog now terminates the whole script after 7,200 seconds and the Actions step after 130 minutes. Thus later in-budget stages can never finish when earlier stages approach their allowed limits, causing valid candidate cuts to fail; keep the outer watchdog and step ceiling above the sum of the inner stage ceilings.
Useful? React with 👍 / 👎.
| flutterApi?.onDeviceReady(peripheralUuid: uuid, services: bleServices) { _ in } | ||
| LimitlessFlashDrainEngine.shared.onDeviceReady(uuid) | ||
| // One sample for disconnect annotation. Do not start a 3s timer | ||
| // here — that was a fake keep-alive and a background battery cost. | ||
| peripheral.readRSSI() | ||
| if isRssiStreamingEnabled { | ||
| startRssiPolling(for: peripheral) | ||
| } | ||
| startRssiKeepAlive(for: peripheral) |
There was a problem hiding this comment.
Whenever an iOS BLE device finishes characteristic discovery, this starts a repeating timer that calls readRSSI() every three seconds for the entire connection, even when diagnostics streaming is disabled and while the app is operating in background BLE modes. RSSI reads do not keep the connection alive, so this restores continuous radio/CPU wakeups for every connected user; take one diagnostic sample and start repeating polling only when the explicit RSSI-streaming setting is enabled.
Useful? React with 👍 / 👎.
| @router.patch('/v3/memories/{memory_id}/baseline', tags=['memories'], response_model=MemoryMutationResponse) | ||
| def update_memory_baseline( |
There was a problem hiding this comment.
When a desktop user marks an insight read, dismisses it, or uses Mark All Read, InsightStorage still calls PATCH /v3/memories/{id}/read, but this router now jumps directly from the visibility route to the baseline route. Every status mutation therefore returns 404 (or 410 on the deprecated desktop backend), while the client logs and preserves its optimistic local state; the next backend sync restores the unread or undismissed insight. Restore the route or migrate every in-tree caller in the same change.
AGENTS.md reference: AGENTS.md:L85-L89
Useful? React with 👍 / 👎.
| existing_items = action_items_db.get_action_items_by_ids(uid, request.ids[i : i + 500]) | ||
| if any(item.get('is_locked', False) for item in existing_items): | ||
| raise HTTPException(status_code=402, detail="A paid plan is required to delete locked action items.") | ||
|
|
||
| deleted_ids = action_items_db.delete_action_items_batch(uid, request.ids) |
There was a problem hiding this comment.
When a free user includes a locked preview task in /v1/action-items/batch-delete—including through desktop multi-select—the database helper deletes it without inspecting fields. The single-item delete and other batch mutations explicitly reject or skip is_locked rows with 402, so removing the chunked preflight lets the batch endpoint destructively bypass the paid-content guard; restore the bounded locked-item validation before committing any deletion.
Useful? React with 👍 / 👎.
| // Local-first: soft-delete in SQLite immediately for instant UI update | ||
| do { | ||
| if isLocalOnly { | ||
| // No backend row exists; a tombstone would wait forever for an ack. | ||
| try await ActionItemStorage.shared.deleteActionItemByBackendId( | ||
| task.id, | ||
| deletedBy: "user", | ||
| authorization: Self.localMutationAuthorization( | ||
| snapshot: lease.authorizationSnapshot | ||
| ) | ||
| ) | ||
| } else { | ||
| try await ActionItemStorage.shared.markActionItemDeletedPendingBackendSync( | ||
| backendId: task.id, | ||
| authorization: Self.localMutationAuthorization( | ||
| snapshot: lease.authorizationSnapshot | ||
| ) | ||
| try await ActionItemStorage.shared.deleteActionItemByBackendId( | ||
| task.id, | ||
| deletedBy: "user", | ||
| authorization: Self.localMutationAuthorization( | ||
| snapshot: lease.authorizationSnapshot | ||
| ) | ||
| } | ||
| ) |
There was a problem hiding this comment.
When the backend delete fails because the user is offline or the request has a transient error, this immediately hard-deletes the only local row and the catch path merely logs the failure. No pending-deletion record remains for retryUnsyncedItems(), so the server copy survives and is downloaded again on refresh, reinstall, or another device, resurrecting a task the user deleted; keep a tombstone with backendSynced = false until server acknowledgement and retry it on later syncs.
Useful? React with 👍 / 👎.
| env_file='.env' | ||
| api_base_url="$BETA_API_BASE_URL" | ||
| fi | ||
|
|
||
| # Keep developer-owned settings and comments intact. These are the only keys | ||
| # owned by setup, so update them in place and replace the file atomically. | ||
| local temp_file source_file="/dev/null" | ||
| if [[ -f "$env_file" ]]; then source_file="$env_file"; fi | ||
| temp_file=$(mktemp "${env_file}.tmp.XXXXXX") | ||
| if ! awk \ | ||
| -v api_base_url="$api_base_url" \ | ||
| 'BEGIN { api_written = 0; web_auth_written = 0; custom_token_written = 0 } | ||
| /^[[:space:]]*API_BASE_URL[[:space:]]*=/ { | ||
| if (!api_written) { print "API_BASE_URL=" api_base_url; api_written = 1 } | ||
| next | ||
| } | ||
| /^[[:space:]]*USE_WEB_AUTH[[:space:]]*=/ { | ||
| if (!web_auth_written) { print "USE_WEB_AUTH=true"; web_auth_written = 1 } | ||
| next | ||
| } | ||
| /^[[:space:]]*USE_AUTH_CUSTOM_TOKEN[[:space:]]*=/ { | ||
| if (!custom_token_written) { print "USE_AUTH_CUSTOM_TOKEN=true"; custom_token_written = 1 } | ||
| next | ||
| } | ||
| { print } | ||
| END { | ||
| if (!api_written) print "API_BASE_URL=" api_base_url | ||
| if (!web_auth_written) print "USE_WEB_AUTH=true" | ||
| if (!custom_token_written) print "USE_AUTH_CUSTOM_TOKEN=true" | ||
| }' \ | ||
| "$source_file" 2>/dev/null >"$temp_file"; then | ||
| rm -f "$temp_file" | ||
| return 1 | ||
| fi | ||
| if ! mv "$temp_file" "$env_file"; then | ||
| rm -f "$temp_file" | ||
| return 1 | ||
| fi | ||
| printf 'API_BASE_URL=%s\nUSE_WEB_AUTH=true\nUSE_AUTH_CUSTOM_TOKEN=true\n' "$api_base_url" > "$env_file" |
There was a problem hiding this comment.
Whenever bash setup.sh ios or bash setup.sh android runs, this truncates .dev.env (or .env for beta) to only three setup-owned values. Existing OPENAI_API_KEY, PostHog, Maps, Google OAuth, Intercom, comments, and any future Envied fields are silently erased before code generation, so locally configured integrations stop working and developers lose credential configuration on every setup; update only the owned keys atomically instead of replacing the whole file.
Useful? React with 👍 / 👎.
68a8fca to
1a0ea33
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4c5818d6-2427-491a-bd67-56fa81901570) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a0ea33ede
ℹ️ 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".
|
Addressed the two remaining export-safety findings in af277f3: the full run clears both prior JSON artifacts before scanning, and raw UID logging is removed. Focused suite: 7 passed; normal pre-push gate passed. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_361acf22-2272-4eb9-88cc-ffef1ed80073) |
|
Rechecked the live head
Focused |
Rebuild of PR branch on current origin/main. The prior head carried an accidental revert commit reverting ~1281 files of main; this branch contains only the intended feature diff (backend/scripts/web.py persona export mapper, its hermetic unit test, and .gitignore entries). Feature: joins user messages (by stable pluginId) with plugins_data documents, redacts mcp_oauth_tokens, records the chatting user under user_uid, preserves botName-only legacy messages, and scans all users in bounded batches. Tests: 6 passed (test_web_persona_export.py).
Failure-Class: none
af277f3 to
6963ac8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6963ac8cf3
ℹ️ 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".
Write the uncapped user export incrementally via a temp file and create both artifacts with 0600 permissions so a failed scan cannot leak a world-readable customer dump.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bad2e00565
ℹ️ 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".
| plugin_id = message.get("pluginId") | ||
| if plugin_id: | ||
| persona_uids.setdefault(plugin_id, set()).add(uid) |
There was a problem hiding this comment.
When a user has historical persona messages for an app that was later deleted, the message's pluginId is added here, so it is excluded from the botName fallback; however, the app deletion path removes the corresponding plugins_data document without deleting these user messages. The subsequent join therefore emits neither plugin metadata nor an unmatched-message record, silently omitting that persona from the analysis. Preserve plugin IDs left unmatched after the collection scan, using the stored botName and messages as fallback data.
Useful? React with 👍 / 👎.
Addresses a TODO in `backend/scripts/web.py` by implementing `map_plugin_data_by_persona_name` to organize existing plugin data by persona name (`botName`) and output it to a local JSON file. Failure-Class: none Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
omi/backend/routers/desktop_proxy.py
Lines 568 to 570 in b300a7b
Preserve retry signaling for provider outages
When Gemini returns an upstream 502 or 503, this broad branch rewrites it to HTTP 502 with X-Omi-Retryable: false. The retained macOS client gates retries on that header, while retained Windows callers retry HTTP 503, so a transient provider outage immediately fails desktop generation instead of using their bounded retry loops; preserve 502/503 as a retryable 503 response with a short Retry-After.
ℹ️ 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".
| message_with_uid = message.copy() | ||
| message_with_uid["uid"] = uid | ||
| plugin_data_by_persona[bot_name].append(message_with_uid) |
There was a problem hiding this comment.
When a persona message is processed, this appends a copy of the message itself and never reads the authoritative plugins_data collection (backend/database/apps.py:30). Consequently, plugin_data_by_persona_name.json contains chat records rather than plugin metadata, so downstream analysis expecting app fields such as descriptions or integration configuration receives a structurally incorrect dataset; resolve each message's plugin ID and export the matching plugin document instead.
Useful? React with 👍 / 👎.
| with open("plugin_data_by_persona_name.json", "w") as f: | ||
| json.dump(plugin_data_by_persona, f, indent=2, default=str) |
There was a problem hiding this comment.
Whenever this script runs from the repository root or backend/, it creates an untracked JSON file containing copied customer message documents and UIDs. Fresh evidence in this exact revision is that git check-ignore matches neither possible plugin_data_by_persona_name.json path and .gitignore has no corresponding entry, so a routine git add . can commit the export; write it beneath the ignored research directory or explicitly ignore the generated path.
Useful? React with 👍 / 👎.
| ) | ||
| daily_allowed, daily_remaining, daily_retry_after = await run_blocking( | ||
| raise HTTPException(status_code=429, detail='Gemini request rate limit exceeded') | ||
| _, current, _ = await run_blocking( |
There was a problem hiding this comment.
For every non-BYOK desktop Gemini request, check_rate_limit() returns (allowed, remaining, retry_after), but this assigns the remaining count to current and discards allowed. Because remaining is clamped between zero and 1,499, the subsequent current > 1500 condition can never be true, so users can continue consuming server-paid Gemini calls indefinitely after exhausting the daily hard limit; retain and reject on the returned allowed flag.
Useful? React with 👍 / 👎.
| return UpstreamRoute( | ||
| url, | ||
| {'Authorization': f'Bearer {token}', ptr.REQUEST_TYPE_HEADER: capacity}, | ||
| {'Authorization': f'Bearer {token}'}, |
There was a problem hiding this comment.
When the purchased Vertex provisioned-throughput reservation reaches capacity, this request now sends only the bearer token and omits X-Vertex-AI-LLM-Request-Type: dedicated. Vertex therefore silently serves excess traffic as on-demand instead of returning the capacity response the proxy can route deliberately, charging pay-as-you-go while the fixed reservation is still billed; restore the capacity header for the provisioned model.
Useful? React with 👍 / 👎.
| trial_days: int | ||
|
|
||
|
|
||
| @router.get('/v1/users/me/referral', response_model=ReferralLinkResponse) |
There was a problem hiding this comment.
When an already-released desktop client opens its referral section, it still requests GET /v1/users/me/referral, while the shipped web claim flow calls the corresponding claim endpoint; deleting this router makes those installed clients receive 404s. I checked the target tree for replacement handlers and found none, so retain the deprecated routes until released callers are retired rather than deleting the complete boundary.
AGENTS.md reference: backend/AGENTS.md:L233-L233
Useful? React with 👍 / 👎.
| inject_web_search = ( | ||
| web_search_supported | ||
| and body.get('tool_choice') != 'none' | ||
| and not public_web_prohibited | ||
| and web_search_requested |
There was a problem hiding this comment.
When RealtimeHubTools builds an escalation with omi_web_search: true, it appends conversation, file, or other tool output after Tool-provided context (untrusted):; this condition now injects Anthropic's server-side web-search tool without checking whether the transcript carries that private output. The model can consequently place private Omi context into an external search query, so restore the private-tool-output gate before enabling web search.
Useful? React with 👍 / 👎.
| result.pop('tool_choice', None) | ||
| result['messages'] = _with_public_web_routing_instruction(translated) | ||
| return result | ||
| gateway_body = {key: value for key, value in body.items() if key != 'omi_web_search'} |
There was a problem hiding this comment.
When a managed pi-mono turn includes OpenAI SDK fields such as store, reasoning_effort, or parallel_tool_calls, this forwards them unchanged because it removes only omi_web_search. The gateway's validator does not accept those top-level fields and rejects the entire request with HTTP 400 before resolving a lane, which the backend returns as an in-band provider error; construct the payload from the gateway's accepted parameter set instead of forwarding the client body wholesale.
Useful? React with 👍 / 👎.
2 participants
Implement mapping for plugin data based on persona name to a local JSON file.
PR created automatically by Jules for task 10758176398782856520 started by @undivisible
Note
Medium Risk
The script is offline-only but, when run, exports all users’ persona messages and plugin metadata to local JSON; OAuth tokens are redacted and outputs are gitignored, but scope and sensitivity of the export increased materially.
Overview
Extends the
web.pyresearch export so it can build a persona-linked plugin dataset locally, not just dump bot-tagged messages.User message export now scans all users (not the first 20), processes them in batches of 20 with a bounded thread pool, and drops per-user debug logging.
clear_export_artifacts()deletes prioruser_messages_with_bot_name.jsonandplugin_data_by_persona_name.jsonbefore a full run.New
map_plugin_data_by_persona_name()reads the messages export, groups chatting users bypluginId, joins matchingplugins_datadocuments, and writesplugin_data_by_persona_name.jsonwith auser_uidfield per chatting user.mcp_oauth_tokensare stripped fromexternal_integration; legacy messages with onlybotNameland under_bot_name_onlyso they are not dropped..gitignore ignores both export JSON paths. Unit tests cover grouping, OAuth redaction, stale output cleanup, legacy messages, full-user scan behavior, and worker failures.
Reviewed by Cursor Bugbot for commit 6963ac8. Configure here.
Failure-Class: none