Add plugin_data mapping by persona_name - #12489
Conversation
Implemented `map_plugin_data_by_persona_name()` in `backend/scripts/web.py` to fetch plugin_data from Firestore, map it by `persona_name`, and write to `plugin_data_by_persona_name.json`. Called in the main execution block. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 2036f3dd63
ℹ️ 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".
| for plugin in plugins: | ||
| raw: object = plugin.to_dict() | ||
| data: Dict[str, Any] = cast(Dict[str, Any], raw) if isinstance(raw, dict) else {} | ||
| persona_name = data.get("persona_name") |
There was a problem hiding this comment.
name field for the mapping key
For current plugins_data documents, personas store their display name in name, as defined by AppBaseModel and assigned by the persona creation paths in backend/routers/apps.py; there is no production writer for persona_name. Consequently, running this script skips normal persona documents and produces an empty or severely incomplete mapping instead of the requested export.
Useful? React with 👍 / 👎.
| with open("plugin_data_by_persona_name.json", "w") as f: | ||
| json.dump(plugin_data_by_persona_name, f, default=str) |
There was a problem hiding this comment.
When plugins_data contains an MCP-backed app, dumping each complete document also writes external_integration.mcp_oauth_tokens, which production populates with client secrets, access tokens, refresh tokens, and PKCE verifiers in backend/routers/apps.py. The generated JSON is not gitignored, so merely running this analysis script creates a plaintext, untracked credential export that can be accidentally committed or collected by backups; export only the metadata required for the persona analysis or explicitly redact these nested fields.
Useful? React with 👍 / 👎.
| data: Dict[str, Any] = cast(Dict[str, Any], raw) if isinstance(raw, dict) else {} | ||
| persona_name = data.get("persona_name") | ||
| if persona_name: | ||
| plugin_data_by_persona_name[persona_name] = data |
There was a problem hiding this comment.
When two personas have the same display name, the later streamed document silently replaces the earlier one in this dictionary. The creation path only makes username unique and permits duplicate name values, so the export loses records and associates any name-based message analysis with an arbitrary persona depending on Firestore stream order; store a list per name or key records by a unique identifier.
Useful? React with 👍 / 👎.
Git-on-my-level
added
security-review
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for picking up the TODO in backend/scripts/web.py! The intent (map plugin data by persona for the planned questions analysis) is clear, but a couple of things should be addressed before this merges.
backend/scripts/web.py
-
(Blocking)
persona_nameisn't a field production writes. Persona documents inplugins_datastore their display name innameand their unique handle inusername— see the persona creation paths inbackend/routers/apps.py, whereusernameis uniquified viaincrement_usernamebutnameis not. Nothing in the backend or the Flutter app ever writespersona_name, sodata.get("persona_name")is alwaysNonehere and the export comes out empty. Keying bynamewould match the TODO's wording; keying byusernamewould additionally be collision-safe. -
(Blocking, security-sensitive) The script dumps each full
plugins_datadocument (json.dump(plugin_data_by_persona_name, f)), and for MCP-backed apps those documents containexternal_integration.mcp_oauth_tokens— client secrets, access/refresh tokens, and PKCE verifiers, as written bybackend/routers/apps.py. Running this against production therefore writes a plaintext credentials file, andplugin_data_by_persona_name.jsonisn't covered by.gitignore, so it can easily be committed or scooped up by backups. Please export only the fields the analysis actually needs (persona name/handle plus the metadata you're measuring), or explicitly stripexternal_integration/mcp_oauth_tokensbefore writing. -
(Minor) When two personas share the same display
name, the later streamed document silently replaces the earlier one in the dict; keying byusernamewould fix this too. -
(Minor)
map_plugin_data_by_persona_name()runs afterget_user_messages_with_bot_name(), i.e. only after the long first pass. If the mapping is the point of this change, consider calling it first or behind a small arg/flag.
fix_web.patch / fix_web2.patch
These two contradict each other — fix_web.patch rewrites the collection to plugin_data, fix_web2.patch changes it back to plugins_data — and look like scratch artifacts committed at the repo root by accident. Neither should land; please drop both. (For the record, the current plugins_data collection name in web.py is correct — it matches apps_collection in backend/database/apps.py.)
Holding merge for a maintainer decision on the export semantics — which key the mapping should use, and which fields may leave the database — since that's a small product/data-handling call rather than just a code fix.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
kodjima33
left a comment
kodjima33
left a comment
There was a problem hiding this comment.
Approve only: feature (plugin_data by persona_name script), not a bug fix — stays approve-only per policy.
|
Duplicate of #12481 (and of the older conflicting #11348). Git-on-my-level: |
3 participants
Adds script functionality to dump plugin metadata to a JSON file mapped by persona name.
PR created automatically by Jules for task 1952785066089303131 started by @undivisible