Multi-Session & Codegen Support #36
Conversation
Co-authored-by: kalil0321 <kalil.bouzigues@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| document.addEventListener('keydown', handleKeyDown, true) | ||
|
|
||
| // Check for navigation periodically | ||
| setInterval(checkNavigation, 100) |
There was a problem hiding this comment.
Fuite de mémoire: l'intervalle n'est jamais nettoyé lorsque stopRecording() est appelé
| setInterval(checkNavigation, 100) | |
| // Check for navigation periodically | |
| const navigationInterval = setInterval(checkNavigation, 100) |
Puis dans stopRecording(), ajouter clearInterval(navigationInterval)
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/content/codegen-recorder.ts
Line: 208:208
Comment:
Fuite de mémoire: l'intervalle n'est jamais nettoyé lorsque `stopRecording()` est appelé
```suggestion
// Check for navigation periodically
const navigationInterval = setInterval(checkNavigation, 100)
```
Puis dans `stopRecording()`, ajouter `clearInterval(navigationInterval)`
How can I resolve this? If you propose a fix, please make it concise.| // Create a new session if none exists | ||
| if (!activeSessionId) { | ||
| const { session } = await createSession() | ||
| activeSessionId = session.id | ||
| currentRunId = session.runId | ||
| } | ||
|
|
||
| currentRunId = generateRunId() |
There was a problem hiding this comment.
Confusion logique: génération d'un nouveau runId juste après avoir créé une session avec son propre runId
La ligne 368 définit currentRunId = session.runId, puis la ligne 371 génère immédiatement un nouveau runId qui écrase le précédent. Cela rend le runId de la session nouvellement créée inutile.
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/background/service-worker.ts
Line: 364:371
Comment:
Confusion logique: génération d'un nouveau `runId` juste après avoir créé une session avec son propre `runId`
La ligne 368 définit `currentRunId = session.runId`, puis la ligne 371 génère immédiatement un nouveau `runId` qui écrase le précédent. Cela rend le `runId` de la session nouvellement créée inutile.
How can I resolve this? If you propose a fix, please make it concise.| const parentSelector = getSelector(parent) | ||
| if (parentSelector !== 'body') { |
There was a problem hiding this comment.
Récursion infinie potentielle: getSelector() s'appelle lui-même sans condition de sortie claire
Si parentSelector est constamment différent de 'body', la fonction continuera à remonter l'arbre DOM récursivement sans garantie d'arrêt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/content/codegen-recorder.ts
Line: 69:70
Comment:
Récursion infinie potentielle: `getSelector()` s'appelle lui-même sans condition de sortie claire
Si `parentSelector` est constamment différent de `'body'`, la fonction continuera à remonter l'arbre DOM récursivement sans garantie d'arrêt.
How can I resolve this? If you propose a fix, please make it concise.| chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { | ||
| if (message.type === 'codegenAction' && codegenActive && sender.tab?.id === codegenTabId) { | ||
| const { action, selector, value, url } = message | ||
| let code = '' | ||
|
|
||
| switch (action) { | ||
| case 'click': | ||
| code = ` page.click("${selector}")\n` | ||
| break | ||
| case 'fill': | ||
| code = ` page.fill("${selector}", "${value}")\n` | ||
| break | ||
| case 'navigate': | ||
| code = ` page.goto("${url}")\n` | ||
| break | ||
| case 'select': | ||
| code = ` page.select_option("${selector}", "${value}")\n` | ||
| break | ||
| } | ||
|
|
||
| if (code) { | ||
| codegenScript += code | ||
| broadcastMessage({ type: 'codegenUpdate', script: codegenScript, newCode: code }) | ||
| } | ||
|
|
||
| sendResponse({ success: true }) | ||
| return true | ||
| } | ||
| return false |
There was a problem hiding this comment.
Code Python généré vulnérable aux injections: les valeurs selector, value et url sont directement interpolées sans échappement
Les chaînes de caractères provenant du content script sont insérées directement dans le code Python sans validation ni échappement. Un utilisateur malveillant pourrait injecter du code Python arbitraire via les interactions de la page.
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/background/service-worker.ts
Line: 322:350
Comment:
Code Python généré vulnérable aux injections: les valeurs `selector`, `value` et `url` sont directement interpolées sans échappement
Les chaînes de caractères provenant du content script sont insérées directement dans le code Python sans validation ni échappement. Un utilisateur malveillant pourrait injecter du code Python arbitraire via les interactions de la page.
How can I resolve this? If you propose a fix, please make it concise.| await chrome.runtime.sendMessage({ type: 'switchSession', sessionId }) | ||
| const res = await chrome.runtime.sendMessage({ type: 'getState' }) | ||
| if (res) setState(prev => ({ ...prev, ...res })) | ||
| // Load messages for the switched session (TODO: implement message persistence per session) |
There was a problem hiding this comment.
Les messages de session ne sont pas restaurés lors du changement de session
Le commentaire indique que la persistance des messages par session doit être implémentée, mais actuellement les messages sont simplement effacés.
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/sidepanel/side-panel.tsx
Line: 272:272
Comment:
Les messages de session ne sont pas restaurés lors du changement de session
Le commentaire indique que la persistance des messages par session doit être implémentée, mais actuellement les messages sont simplement effacés.
How can I resolve this? If you propose a fix, please make it concise.| chrome.runtime.sendMessage({ type: 'getCodegenState' }).then(response => { | ||
| if (response?.codegenActive) { | ||
| startRecording() | ||
| } | ||
| }).catch(() => { | ||
| // Extension not ready yet | ||
| }) |
There was a problem hiding this comment.
Vérifier que le content script est correctement injecté avant la navigation initiale
Le script vérifie l'état au chargement, mais si l'extension n'est pas prête, il échoue silencieusement sans réessayer.
Prompt To Fix With AI
This is a comment left during a code review.
Path: chrome-extension/src/content/codegen-recorder.ts
Line: 241:247
Comment:
Vérifier que le content script est correctement injecté avant la navigation initiale
Le script vérifie l'état au chargement, mais si l'extension n'est pas prête, il échoue silencieusement sans réessayer.
How can I resolve this? If you propose a fix, please make it concise.![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: 4d67493724
ℹ️ 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".
| async function switchSession(sessionId: string): Promise<{ success: boolean; session: Session | null }> { | ||
| // Don't allow switching if capturing on current session | ||
| if (captureManager.isCapturing()) { | ||
| throw new Error('Cannot switch sessions while capturing. Stop capture first.') | ||
| } |
There was a problem hiding this comment.
Session switching only guards against captureManager.isCapturing() and ignores codegenActive, so users can switch sessions mid‑recording. Because stopCodegen later persists the script to whatever activeSessionId is current, any mid‑recording session switch will save the generated script into the wrong session and orphan the original session’s recording. This is a data integrity issue for multi‑session codegen; consider preventing switching when codegen is active or tracking the session id that started codegen and persisting to that session regardless of UI switches.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="chrome-extension/src/sidepanel/side-panel.tsx">
<violation number="1" location="chrome-extension/src/sidepanel/side-panel.tsx:260">
P2: Creating a session while a capture is running clears the current session’s chat even though the UI remains on the same session, causing the user to lose their visible conversation.</violation>
</file>
<file name="chrome-extension/src/background/service-worker.ts">
<violation number="1" location="chrome-extension/src/background/service-worker.ts:188">
P1: Session switching should also be blocked when codegen recording is active. Currently only capture mode is checked, but switching sessions during codegen will cause the recorded script to be saved to the wrong session when `stopCodegen` persists to `activeSessionId`.</violation>
<violation number="2" location="chrome-extension/src/background/service-worker.ts:329">
P2: Escape selectors/values before embedding them into the generated Playwright script so that recorded interactions containing quotes or backslashes don’t produce invalid Python code.</violation>
</file>
<file name="chrome-extension/src/content/codegen-recorder.ts">
<violation number="1" location="chrome-extension/src/content/codegen-recorder.ts:47">
P2: Attribute values are inserted into selectors without escaping, so placeholders/aria-labels containing quotes produce invalid selectors and break recorded Playwright actions. Use `CSS.escape()` (or equivalent) before embedding attribute values.</violation>
<violation number="2" location="chrome-extension/src/content/codegen-recorder.ts:208">
P2: `setInterval(checkNavigation, 100)` is invoked on every recording start but never cleared on stop, leaking timers and causing redundant navigation checks/events across sessions. Store the interval handle and `clearInterval` it when recording stops before creating a new one.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (captureManager.isCapturing()) { | ||
| throw new Error('Cannot switch sessions while capturing. Stop capture first.') |
There was a problem hiding this comment.
P1: Session switching should also be blocked when codegen recording is active. Currently only capture mode is checked, but switching sessions during codegen will cause the recorded script to be saved to the wrong session when stopCodegen persists to activeSessionId.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chrome-extension/src/background/service-worker.ts, line 188:
<comment>Session switching should also be blocked when codegen recording is active. Currently only capture mode is checked, but switching sessions during codegen will cause the recorded script to be saved to the wrong session when `stopCodegen` persists to `activeSessionId`.</comment>
<file context>
@@ -83,26 +105,251 @@ async function handleMessage(message: { type: string; [key: string]: unknown }):
+
+async function switchSession(sessionId: string): Promise<{ success: boolean; session: Session | null }> {
+ // Don't allow switching if capturing on current session
+ if (captureManager.isCapturing()) {
+ throw new Error('Cannot switch sessions while capturing. Stop capture first.')
+ }
</file context>
| if (captureManager.isCapturing()) { | |
| throw new Error('Cannot switch sessions while capturing. Stop capture first.') | |
| if (captureManager.isCapturing() || codegenActive) { | |
| throw new Error('Cannot switch sessions while capturing or recording. Stop first.') |
| const res = await chrome.runtime.sendMessage({ type: 'getState' }) | ||
| if (res) setState(prev => ({ ...prev, ...res })) | ||
| // Clear messages for new session | ||
| setMessages([]) |
There was a problem hiding this comment.
P2: Creating a session while a capture is running clears the current session’s chat even though the UI remains on the same session, causing the user to lose their visible conversation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chrome-extension/src/sidepanel/side-panel.tsx, line 260:
<comment>Creating a session while a capture is running clears the current session’s chat even though the UI remains on the same session, causing the user to lose their visible conversation.</comment>
<file context>
@@ -213,6 +250,81 @@ export function SidePanel() {
+ const res = await chrome.runtime.sendMessage({ type: 'getState' })
+ if (res) setState(prev => ({ ...prev, ...res }))
+ // Clear messages for new session
+ setMessages([])
+ } catch (err) {
+ console.error('Create session error:', err)
</file context>
|
|
||
| switch (action) { | ||
| case 'click': | ||
| code = ` page.click("${selector}")\n` |
There was a problem hiding this comment.
P2: Escape selectors/values before embedding them into the generated Playwright script so that recorded interactions containing quotes or backslashes don’t produce invalid Python code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chrome-extension/src/background/service-worker.ts, line 329:
<comment>Escape selectors/values before embedding them into the generated Playwright script so that recorded interactions containing quotes or backslashes don’t produce invalid Python code.</comment>
<file context>
@@ -83,26 +105,251 @@ async function handleMessage(message: { type: string; [key: string]: unknown }):
+
+ switch (action) {
+ case 'click':
+ code = ` page.click("${selector}")\n`
+ break
+ case 'fill':
</file context>
| document.addEventListener('keydown', handleKeyDown, true) | ||
|
|
||
| // Check for navigation periodically | ||
| setInterval(checkNavigation, 100) |
There was a problem hiding this comment.
P2: setInterval(checkNavigation, 100) is invoked on every recording start but never cleared on stop, leaking timers and causing redundant navigation checks/events across sessions. Store the interval handle and clearInterval it when recording stops before creating a new one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chrome-extension/src/content/codegen-recorder.ts, line 208:
<comment>`setInterval(checkNavigation, 100)` is invoked on every recording start but never cleared on stop, leaking timers and causing redundant navigation checks/events across sessions. Store the interval handle and `clearInterval` it when recording stops before creating a new one.</comment>
<file context>
@@ -0,0 +1,247 @@
+ document.addEventListener('keydown', handleKeyDown, true)
+
+ // Check for navigation periodically
+ setInterval(checkNavigation, 100)
+
+ console.log('[Codegen] Recording started')
</file context>
| // Try placeholder for inputs | ||
| if (element.hasAttribute('placeholder')) { | ||
| const placeholder = element.getAttribute('placeholder') | ||
| return `[placeholder="${placeholder}"]` |
There was a problem hiding this comment.
P2: Attribute values are inserted into selectors without escaping, so placeholders/aria-labels containing quotes produce invalid selectors and break recorded Playwright actions. Use CSS.escape() (or equivalent) before embedding attribute values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chrome-extension/src/content/codegen-recorder.ts, line 47:
<comment>Attribute values are inserted into selectors without escaping, so placeholders/aria-labels containing quotes produce invalid selectors and break recorded Playwright actions. Use `CSS.escape()` (or equivalent) before embedding attribute values.</comment>
<file context>
@@ -0,0 +1,247 @@
+ // Try placeholder for inputs
+ if (element.hasAttribute('placeholder')) {
+ const placeholder = element.getAttribute('placeholder')
+ return `[placeholder="${placeholder}"]`
+ }
+
</file context>
| return `[placeholder="${placeholder}"]` | |
| return `[placeholder="${CSS.escape(placeholder ?? '')}"]` |
Summary
Enhance Chrome extension with multi-session support and codegen mode:
Key Changes
Technical Improvements
Summary by cubic
Adds multi-session capture and a new Codegen mode that records live Playwright Python scripts. Sessions now isolate traffic, chat, and generated code, with an updated UI to switch modes and manage sessions.
New Features
Migration
Written for commit 4d67493. Summary will update on new commits.
Greptile Overview
Greptile Summary
Cette PR ajoute la gestion multi-session et un mode codegen pour enregistrer des scripts Playwright en direct. L'implémentation est généralement solide, mais contient plusieurs problèmes critiques qui doivent être résolus :
Problèmes critiques identifiés :
codegen-recorder.ts: l'intervalle de navigation n'est jamais nettoyéservice-worker.ts)startCapture()qui génère un nouveaurunIdimmédiatement après la création d'une sessiongetSelector()du content scriptAméliorations recommandées :
Points positifs :