Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a Telegram-based remote-control feature: main-process presenter and services (poller, client, parser, router, runner, binding/auth), renderer Remote settings and sidebar integration, shared types, tests, and i18n; plus small prompt text edits. Changes
Sequence Diagram(s)sequenceDiagram
participant TG as Telegram (user)
participant Client as TelegramClient
participant Poller as TelegramPoller
participant Parser as TelegramParser
participant Router as RemoteCommandRouter
participant Runner as RemoteConversationRunner
participant Presenter as RemoteControlPresenter
participant DeepChat as DeepChat (agent/runtime)
TG->>Client: user sends message / callback
Client->>Poller: getUpdates → update
Poller->>Parser: parseUpdate(update)
Parser-->>Poller: TelegramInboundEvent
Poller->>Router: handleMessage(event)
Router->>Runner: route command / send text / manage session
Runner->>DeepChat: createDetachedSession / sendMessage / setModel
DeepChat-->>Runner: SessionWithState / assistant response
Runner-->>Router: conversation result / snapshot
Router-->>Poller: replies / outboundActions / callbackAnswer
Poller->>Client: sendMessage / editMessageText / answerCallbackQuery / setMessageReaction
Client-->>TG: Bot API responses displayed to user
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 1627e44a99
ℹ️ 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".
Comment on lines
+422
to
+423
| return typeof error.code === 'number' && error.code >= 400 && error.code < 500 | ||
| ? error.code !== 429 |
There was a problem hiding this comment.
runLoop catches errors from both getUpdates and handleRawUpdate, but isFatalPollError classifies any Telegram 4xx (except 429) as fatal. That means a single per-update send/edit failure (for example, a bad callback edit or other chat-level 4xx) will trigger onFatalError and disable the whole remote runtime for all users, even though polling/auth may still be healthy. Fatal classification should be restricted to true runtime-invalid conditions (e.g., invalid token/conflict) rather than every 4xx raised during update handling.
Useful? React with 👍 / 👎.
Comment on lines
+103
to
+104
| await this.handleRawUpdate(update) | ||
| this.deps.bindingStore.setPollOffset(update.update_id + 1) |
There was a problem hiding this comment.
The poll offset is persisted only after handleRawUpdate completes, so if command processing mutates state and a later outbound call fails transiently (network/backoff path), the same Telegram update is replayed on the next poll. Because handlers include non-idempotent actions like session creation and message submission, retries can duplicate side effects (extra sessions/messages) from a single inbound update.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/settings/components/prompt/SystemPromptSettingsSection.vue (1)
241-273:⚠️ Potential issue | 🟠 MajorEnsure synchronization with the main process default prompt.
The
originalContentmust stay synchronized withDEFAULT_SYSTEM_PROMPTinsrc/main/presenter/configPresenter/systemPromptHelper.ts. Currently both have been updated identically, but this manual duplication creates maintenance risk.The same concerns raised for the main process file apply here:
- The language instruction removal appears unrelated to Telegram remote control
- If this behavioral change is unintentional, both files need to be reverted
As noted in the review of
systemPromptHelper.ts, consider extracting this constant to a shared location to maintain a single source of truth and prevent future sync issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/settings/components/prompt/SystemPromptSettingsSection.vue` around lines 241 - 273, The resetDefaultSystemPrompt handler hardcodes originalContent which duplicates DEFAULT_SYSTEM_PROMPT from the main process (risking drift); update resetDefaultSystemPrompt to import/use the shared DEFAULT_SYSTEM_PROMPT constant instead of the local originalContent and call systemPromptStore.updateSystemPrompt('default', { content: DEFAULT_SYSTEM_PROMPT, updatedAt: Date.now() }), update currentSystemPrompt.value and systemPrompts.value entries from DEFAULT_SYSTEM_PROMPT, and if DEFAULT_SYSTEM_PROMPT isn’t already exposed, move the constant into a shared module and reference that shared symbol from both this component and systemPromptHelper to ensure a single source of truth.
🧹 Nitpick comments (4)
src/shared/types/agent-interface.d.ts (1)
551-561: Reduce contract drift between session input types.
CreateDetachedSessionInputduplicates most ofCreateSessionInputfields. Derive it fromCreateSessionInputto reduce maintenance risk when fields evolve.Suggested refactor
-export interface CreateDetachedSessionInput { - agentId?: string - title?: string - projectDir?: string - providerId?: string - modelId?: string - permissionMode?: PermissionMode - activeSkills?: string[] - disabledAgentTools?: string[] - generationSettings?: Partial<SessionGenerationSettings> -} +export interface CreateDetachedSessionInput + extends Omit<CreateSessionInput, 'message' | 'files' | 'agentId'> { + agentId?: string + title?: string +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/types/agent-interface.d.ts` around lines 551 - 561, Replace the duplicated interface with a derived type from CreateSessionInput to avoid contract drift; specifically, change CreateDetachedSessionInput to be based on Partial<CreateSessionInput> (or Omit<CreateSessionInput, 'fieldA' | 'fieldB'> if you need to exclude specific fields) and then intersect or add any extra fields (e.g., disabledAgentTools, generationSettings) that are unique to detached sessions, ensuring you reference the CreateSessionInput type and preserve PermissionMode and SessionGenerationSettings types.src/renderer/src/i18n/he-IL/settings.json (1)
1488-1489: Consider localizing placeholder examples.The placeholder text uses English abbreviations "e.g." at lines 1488, 1489, and 1513. For consistency with a fully Hebrew-localized UI, consider using the Hebrew equivalent "למשל" (which is already used elsewhere in this file, e.g., line 1369).
- "allowedUserIdsPlaceholder": "e.g. 123456789, 987654321", + "allowedUserIdsPlaceholder": "למשל 123456789, 987654321",Also applies to: 1513-1513
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/he-IL/settings.json` around lines 1488 - 1489, Replace the English abbreviation "e.g." with the Hebrew equivalent "למשל" in the localization entries to keep the UI consistent; specifically update the value for allowedUserIdsPlaceholder (and any other occurrences such as the entry referenced at line 1513) to use "למשל" instead of "e.g." while preserving the rest of the placeholder text and punctuation.src/main/presenter/remoteControlPresenter/services/remoteCommandRouter.ts (1)
190-202: Default case falls through to send plain text - verify this is intentional.The empty
defaultcase at lines 190-192 means any unrecognized command (e.g.,/unknown) will be sent as plain text to the conversation runner. If this is intentional behavior, consider adding a comment for clarity. If unrecognized commands should return an error, the handler should be updated.💡 Optional: Add clarifying comment
default: + // Unrecognized commands and plain text messages are forwarded to the conversation runner break }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/remoteControlPresenter/services/remoteCommandRouter.ts` around lines 190 - 202, The switch's empty default silently falls through to sending the raw message via this.deps.runner.sendText(endpointKey, message.text); update the default to explicitly handle unknown commands: either add a clarifying inline comment if the passthrough is intentional, or change the default to return an error reply (e.g., return { replies: ['Unrecognized command: /...'] }) so unknown commands don't get sent as plain text; locate the empty default within the switch in remoteCommandRouter (the block that currently falls through to using this.deps.runner.sendText) and implement the chosen behavior.src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts (1)
53-55: AddgetWindowType()to theITabPresenterinterface.
getWindowType()is called ontabPresenterin multiple places (remoteConversationRunner.ts and floatingButtonPresenter.ts) but is not declared inITabPresenter. Both usages work around this type gap with local type casts, which defeats static type safety. AnITabPresenterimplementation could compile but fail at runtime ifgetWindowType()is missing. DefinegetWindowType(windowId: number): 'chat' | 'browser'directly in theITabPresenterinterface and updateRemoteConversationRunnerDepsto useITabPresenterwithout casting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts` around lines 53 - 55, Add getWindowType(windowId: number): 'chat' | 'browser' to the ITabPresenter interface declaration so implementations must provide it; then remove the local casts by updating RemoteConversationRunnerDeps to reference ITabPresenter directly (so tabPresenter is typed as ITabPresenter) and adjust usages in RemoteConversationRunner (remoteConversationRunner.ts) and FloatingButtonPresenter (floatingButtonPresenter.ts) to call tabPresenter.getWindowType without casting. Ensure any existing classes that implement ITabPresenter now implement getWindowType and update their signatures accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/index.ts`:
- Around line 418-419: The call to this.remoteControlPresenter.initialize() (and
the corresponding this.remoteControlPresenter.destroy()) is invoked without
error handling; wrap both lifecycle calls in try-catch blocks (matching the
pattern used for FloatingButtonPresenter/YoBrowserPresenter/SkillPresenter) and
log any caught errors via the same logger used in this file (e.g.,
processLogger.error or the local logger) so async rejections are not unhandled;
update the calls to await this.remoteControlPresenter.initialize() inside a try
block and similarly await this.remoteControlPresenter.destroy() inside a try
block in the shutdown path, logging errors with context like
"RemoteControlPresenter.initialize failed" / "RemoteControlPresenter.destroy
failed".
In
`@src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts`:
- Around line 168-175: The fallback lookup can still return the previous run's
assistant message because only waitForAssistantMessage's polling honors
ignoreMessageId while getActiveGeneration changes; update the fallback/finder
used by waitForAssistantMessage (specifically the callsites involving
previousActiveEventId and the function findLatestAssistantMessageAfter) to also
exclude previousActiveEventId (e.g., add an ignore/eventId parameter to the
fallback lookup or pass ignoreMessageId through to
findLatestAssistantMessageAfter) so that any fallback search filters out the old
eventId; apply the same change to the other similar callsites that use
previousActiveEventId (the blocks around the other
waitForAssistantMessage/findLatestAssistantMessageAfter usages).
In `@src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts`:
- Around line 88-118: The polling loop currently wraps getUpdates() and
per-update processing together so failures in handleRawUpdate() can prevent
advancing the Telegram offset and be treated as fatal; refactor the loop in
telegramPoller.ts to separate bootstrap/poll errors from per-update delivery
errors by: keep getUpdates() and its outer try/catch for poll-level errors (use
isFatalPollError, setStatus, deps.onFatalError), but for each update call
handleRawUpdate(update) inside its own try/catch so that any per-update
exception is caught, logged, and the offset is advanced via
deps.bindingStore.setPollOffset(update.update_id + 1) even when
delivery/outbound steps fail; additionally, introduce an
idempotency/processed-update marker (e.g., a processed flag in bindingStore or
markProcessedUpdate(update.update_id)) or persist the offset prior to
non-idempotent outbound actions in handleRawUpdate/handleMessage to avoid
replaying partially-processed updates; apply the same change to the other
similar blocks around lines 164-200 and 420-432 where
getUpdates()/handleRawUpdate are paired.
- Around line 20-22: The current sleep helper blocks teardown because it doesn't
react to stop(), so change sleep to accept an AbortSignal (e.g., sleep(ms:
number, signal?: AbortSignal)) and implement it so the returned Promise resolves
early when signal.aborted (or rejects/cleans up) by wiring
signal.addEventListener('abort') and clearing the timeout; update all calls that
do await sleep(delay) (the retry/backoff sites in TelegramPoller where
getUpdates() is polled) to pass the poller's abort signal (created by
stop()/AbortController) and ensure stop() calls controller.abort() so sleeping
retries are canceled immediately; keep getUpdates() requests still aborted by
the same signal.
In `@src/main/presenter/remoteControlPresenter/types.ts`:
- Around line 287-312: The TelegramRemoteRuntimeConfigSchema currently uses
TelegramEndpointBindingSchema for its bindings record which causes safeParse to
fail if any single binding is malformed; change the bindings field in
TelegramRemoteRuntimeConfigSchema (and the similar schema at the other location)
to accept a looser type (e.g. z.record(z.string(), z.unknown()).optional()) so
the top-level parse won't reject the whole config, then in the code that
iterates bindings (where bindings are cleaned up) use
TelegramEndpointBindingSchema.safeParse(...) for each entry and only keep
entries that validate; update any code relying on bindings being typed
accordingly after per-binding validation.
- Around line 355-360: normalizeRemoteControlConfig is currently hard-coding the
telegram.streamMode to 'draft', which discards any persisted value; change the
return for telegram.streamMode to preserve the stored value (e.g., use
telegram.streamMode if present and valid, falling back to
defaults.telegram.streamMode) and optionally normalize/validate it rather than
always setting 'draft' so persisted 'final' or other modes are retained across
reloads.
In `@src/renderer/settings/components/RemoteSettings.vue`:
- Around line 581-611: persistSettings currently swallows save failures (toasts
and resolves), causing dependent workflows to continue with stale config; change
persistSettings to surface failures by returning a boolean success flag (or
rethrowing) instead of always resolving: if buildDraftSettings or
remoteControlPresenter.saveTelegramSettings fails, ensure persistSettings
returns false (or rethrows the caught error) after showing the toast, and return
true on success; keep the existing pendingSave/isSaving flow and calls to
syncLocalFields, refreshStatus, and loadDeepChatAgents but make callers of
persistSettings check the returned flag (or await and handle the rethrown error)
so pair-code, bindings, and hook-test flows can bail on failure.
- Around line 525-527: The polling helper refreshStatus currently awaits
remoteControlPresenter.getTelegramStatus() without handling rejections, causing
unhandled promise rejections when the IPC fails; wrap the await in a try/catch
inside refreshStatus (and the equivalent refresh call around lines 818-823) so
that on error you log a warning (e.g. console.warn or processLogger.warn) and
set status.value to a safe "stale" or error state instead of letting the promise
reject; keep the interval callback as-is (dropping the promise) since
refreshStatus will now handle failures internally.
In `@src/renderer/src/components/WindowSideBar.vue`:
- Around line 356-365: The retry via window.setTimeout in openRemoteSettings is
a race and should be replaced with an explicit readiness handshake or by
providing the initial route to the creator: modify openRemoteSettings (which
calls windowPresenter.createSettingsWindow and navigateToSettings) so that
instead of scheduling a second navigateToSettings after 250ms it either (a) pass
the desired initial route ('settings-remote') into
windowPresenter.createSettingsWindow so the new window opens at the correct
route, or (b) await a settings-window-ready signal/event from the created window
(listen for a readiness IPC/event tied to the returned settingsWindowId) and
call navigateToSettings(settingsWindowId, 'settings-remote') only when that
ready event arrives; remove the fixed setTimeout retry and ensure the
NAVIGATE/send happens after the ready handshake.
In `@src/renderer/src/i18n/da-DK/settings.json`:
- Around line 1488-1513: Update the Danish placeholder text keys to use the
Danish prefix (e.g., "Fx" or "fx") instead of the English "e.g."; specifically
modify the values for allowedUserIdsPlaceholder and chatIdPlaceholder to use the
Danish prefix (maintain examples and commas as-is) so the UI is consistent with
other Danish strings like defaultAgentPlaceholder.
In `@src/renderer/src/i18n/fr-FR/settings.json`:
- Line 1488: Replace the English example prefix "e.g." with the French-localized
"ex." in the FR locale placeholders: update the value for the JSON key
allowedUserIdsPlaceholder from "e.g. 123456789, 987654321" to use "ex." and also
change the same pattern at the other occurrence referenced (around the second
placeholder at the same locale, e.g., the key present near line 1513) so all
French UI copy uses "ex." consistently.
In `@src/renderer/src/i18n/ko-KR/settings.json`:
- Around line 1488-1490: The new Korean locale strings mix English into UI text;
update the values for allowedUserIdsPlaceholder, defaultAgent, and
defaultAgentPlaceholder so they are fully localized (replace "e.g." with "예:"
and "Agent"/"Agent 선택" with the Korean term "에이전트" and natural phrasing used
elsewhere), e.g. change allowedUserIdsPlaceholder to use "예: 123456789,
987654321", defaultAgent to "기본 DeepChat 에이전트", and defaultAgentPlaceholder to
"에이전트 선택" to match the rest of ko-KR copy.
In `@src/renderer/src/i18n/pt-BR/settings.json`:
- Line 1488: Replace the English abbreviation "e.g." with the Portuguese
localized "ex.:" in the placeholder strings to match pt-BR copy; specifically
update the value for the "allowedUserIdsPlaceholder" key (and the other
identical placeholder occurrence noted in the file) to use "ex.:" followed by
the sample IDs so both entries are consistently localized.
In `@test/main/presenter/newAgentPresenter/newAgentPresenter.test.ts`:
- Around line 660-698: The test currently sets
configPresenter.resolveDeepChatAgentConfig to return disabledAgentTools:
['find'] but the assertion for sqlitePresenter.newSessionsTable.create still
expects disabledAgentTools: [], so update the expectation in the
presenter.createDetachedSession test to assert the agent-level disabled tools
are carried through; change the final expectation on
sqlitePresenter.newSessionsTable.create (referencing
presenter.createDetachedSession, configPresenter.resolveDeepChatAgentConfig,
deepChatAgent.initSession, and sqlitePresenter.newSessionsTable.create) to
expect disabledAgentTools: ['find'] instead of [].
---
Outside diff comments:
In `@src/renderer/settings/components/prompt/SystemPromptSettingsSection.vue`:
- Around line 241-273: The resetDefaultSystemPrompt handler hardcodes
originalContent which duplicates DEFAULT_SYSTEM_PROMPT from the main process
(risking drift); update resetDefaultSystemPrompt to import/use the shared
DEFAULT_SYSTEM_PROMPT constant instead of the local originalContent and call
systemPromptStore.updateSystemPrompt('default', { content:
DEFAULT_SYSTEM_PROMPT, updatedAt: Date.now() }), update
currentSystemPrompt.value and systemPrompts.value entries from
DEFAULT_SYSTEM_PROMPT, and if DEFAULT_SYSTEM_PROMPT isn’t already exposed, move
the constant into a shared module and reference that shared symbol from both
this component and systemPromptHelper to ensure a single source of truth.
---
Nitpick comments:
In `@src/main/presenter/remoteControlPresenter/services/remoteCommandRouter.ts`:
- Around line 190-202: The switch's empty default silently falls through to
sending the raw message via this.deps.runner.sendText(endpointKey,
message.text); update the default to explicitly handle unknown commands: either
add a clarifying inline comment if the passthrough is intentional, or change the
default to return an error reply (e.g., return { replies: ['Unrecognized
command: /...'] }) so unknown commands don't get sent as plain text; locate the
empty default within the switch in remoteCommandRouter (the block that currently
falls through to using this.deps.runner.sendText) and implement the chosen
behavior.
In
`@src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts`:
- Around line 53-55: Add getWindowType(windowId: number): 'chat' | 'browser' to
the ITabPresenter interface declaration so implementations must provide it; then
remove the local casts by updating RemoteConversationRunnerDeps to reference
ITabPresenter directly (so tabPresenter is typed as ITabPresenter) and adjust
usages in RemoteConversationRunner (remoteConversationRunner.ts) and
FloatingButtonPresenter (floatingButtonPresenter.ts) to call
tabPresenter.getWindowType without casting. Ensure any existing classes that
implement ITabPresenter now implement getWindowType and update their signatures
accordingly.
In `@src/renderer/src/i18n/he-IL/settings.json`:
- Around line 1488-1489: Replace the English abbreviation "e.g." with the Hebrew
equivalent "למשל" in the localization entries to keep the UI consistent;
specifically update the value for allowedUserIdsPlaceholder (and any other
occurrences such as the entry referenced at line 1513) to use "למשל" instead of
"e.g." while preserving the rest of the placeholder text and punctuation.
In `@src/shared/types/agent-interface.d.ts`:
- Around line 551-561: Replace the duplicated interface with a derived type from
CreateSessionInput to avoid contract drift; specifically, change
CreateDetachedSessionInput to be based on Partial<CreateSessionInput> (or
Omit<CreateSessionInput, 'fieldA' | 'fieldB'> if you need to exclude specific
fields) and then intersect or add any extra fields (e.g., disabledAgentTools,
generationSettings) that are unique to detached sessions, ensuring you reference
the CreateSessionInput type and preserve PermissionMode and
SessionGenerationSettings types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02140a21-82ee-4db7-8d49-024d028948b2
📒 Files selected for processing (77)
docs/specs/telegram-remote-control/plan.mddocs/specs/telegram-remote-control/spec.mddocs/specs/telegram-remote-control/tasks.mdsrc/main/presenter/configPresenter/systemPromptHelper.tssrc/main/presenter/deepchatAgentPresenter/index.tssrc/main/presenter/index.tssrc/main/presenter/newAgentPresenter/index.tssrc/main/presenter/remoteControlPresenter/index.tssrc/main/presenter/remoteControlPresenter/interface.tssrc/main/presenter/remoteControlPresenter/services/remoteAuthGuard.tssrc/main/presenter/remoteControlPresenter/services/remoteBindingStore.tssrc/main/presenter/remoteControlPresenter/services/remoteCommandRouter.tssrc/main/presenter/remoteControlPresenter/services/remoteConversationRunner.tssrc/main/presenter/remoteControlPresenter/telegram/telegramClient.tssrc/main/presenter/remoteControlPresenter/telegram/telegramOutbound.tssrc/main/presenter/remoteControlPresenter/telegram/telegramParser.tssrc/main/presenter/remoteControlPresenter/telegram/telegramPoller.tssrc/main/presenter/remoteControlPresenter/types.tssrc/renderer/settings/components/NotificationsHooksSettings.vuesrc/renderer/settings/components/RemoteSettings.vuesrc/renderer/settings/components/prompt/SystemPromptSettingsSection.vuesrc/renderer/settings/main.tssrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/da-DK/routes.jsonsrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/en-US/routes.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fa-IR/routes.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/fr-FR/routes.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/he-IL/routes.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ja-JP/routes.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ko-KR/routes.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/pt-BR/routes.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/ru-RU/routes.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-CN/routes.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-HK/routes.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/i18n/zh-TW/routes.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/index.d.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/new-agent.presenter.d.tssrc/shared/types/presenters/remote-control.presenter.d.tstest/main/presenter/deepchatAgentPresenter/deepchatAgentPresenter.test.tstest/main/presenter/newAgentPresenter/newAgentPresenter.test.tstest/main/presenter/remoteControlPresenter/remoteAuthGuard.test.tstest/main/presenter/remoteControlPresenter/remoteBindingStore.test.tstest/main/presenter/remoteControlPresenter/remoteCommandRouter.test.tstest/main/presenter/remoteControlPresenter/remoteControlPresenter.test.tstest/main/presenter/remoteControlPresenter/remoteConversationRunner.test.tstest/main/presenter/remoteControlPresenter/telegramClient.test.tstest/main/presenter/remoteControlPresenter/telegramOutbound.test.tstest/main/presenter/remoteControlPresenter/telegramParser.test.tstest/main/presenter/remoteControlPresenter/telegramPoller.test.tstest/renderer/components/RemoteSettings.test.tstest/renderer/components/WindowSideBar.test.ts
src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts
Show resolved
Hide resolved
src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts
Outdated
Show resolved
Hide resolved
src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/renderer/src/i18n/ko-KR/settings.json (1)
1513-1513:⚠️ Potential issue | 🟡 Minor
chatIdPlaceholder의e.g.를 ko-KR 표기(예:)로 통일해 주세요.Line 1513만 영어 접두사가 남아 있어 로케일 일관성이 깨집니다.
✍️ Suggested fix
- "chatIdPlaceholder": "e.g. 123456789", + "chatIdPlaceholder": "예: 123456789",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ko-KR/settings.json` at line 1513, Replace the English prefix "e.g." in the localization key chatIdPlaceholder with the Korean equivalent "예:" to maintain locale consistency; locate the chatIdPlaceholder entry in src/renderer/src/i18n/ko-KR/settings.json and update its value from "e.g. 123456789" to a Korean-prefixed string like "예: 123456789".
🧹 Nitpick comments (1)
src/renderer/src/i18n/pt-BR/settings.json (1)
1472-1530: LGTM! Translations are well-localized.The new
remotesection is properly translated to Brazilian Portuguese with correct use of the localized placeholder abbreviation (ex.:). Pluralization and grammar look correct.One minor observation: the
threadIdPlaceholderwording differs slightly from the existingnotificationsHooks.telegram.threadIdPlaceholderat line 102:
- Line 102:
"ID de thread opcional"- Line 1515:
"ID opcional da thread"Both are valid, but you may want to unify them for consistency across the UI.
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/pt-BR/settings.json` around lines 1472 - 1530, The placeholders for thread ID differ between remote.hooks.threadIdPlaceholder ("ID opcional da thread") and notificationsHooks.telegram.threadIdPlaceholder ("ID de thread opcional"); make them consistent by choosing one phrasing and updating the other key to match exactly (e.g., set remote.hooks.threadIdPlaceholder to "ID de thread opcional" or vice versa) so the UI uses the same localized string across both "threadIdPlaceholder" entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/index.ts`:
- Line 489: Presenter.destroy currently fires off destroyRemoteControl() with
void, causing an async teardown race; change Presenter.destroy to be async and
await the remote-control shutdown (await this.destroyRemoteControl()), or
implement an explicit shutdown barrier that waits for
RemoteControlPresenter.destroy() completion (which uses
enqueueRuntimeOperation()) before proceeding to close shared resources like
SQLite, FloatingButtonsPresenter, TabsPresenter, etc.; ensure
RemoteControlPresenter.destroy() returns a Promise that resolves when its queued
runtime operations are drained and then await that promise in Presenter.destroy.
- Line 119: The public field remoteControlPresenter exposes runtime methods
(e.g., initialize() and destroy()) through the generic Presenter IPC dispatcher;
make remoteControlPresenter a private field on the presenter class instead of a
public Presenter property, remove it from the Presenter surface, and implement a
narrow, explicit bridge or dedicated IPC handlers that only forward the safe
renderer-callable operations (whitelist specific methods) to the private
RemoteControlPresenterLike instance; ensure the dispatcher no longer routes to
remoteControlPresenter and that lifecycle methods (initialize/destroy) remain
inaccessible from the generic dispatcher.
---
Duplicate comments:
In `@src/renderer/src/i18n/ko-KR/settings.json`:
- Line 1513: Replace the English prefix "e.g." in the localization key
chatIdPlaceholder with the Korean equivalent "예:" to maintain locale
consistency; locate the chatIdPlaceholder entry in
src/renderer/src/i18n/ko-KR/settings.json and update its value from "e.g.
123456789" to a Korean-prefixed string like "예: 123456789".
---
Nitpick comments:
In `@src/renderer/src/i18n/pt-BR/settings.json`:
- Around line 1472-1530: The placeholders for thread ID differ between
remote.hooks.threadIdPlaceholder ("ID opcional da thread") and
notificationsHooks.telegram.threadIdPlaceholder ("ID de thread opcional"); make
them consistent by choosing one phrasing and updating the other key to match
exactly (e.g., set remote.hooks.threadIdPlaceholder to "ID de thread opcional"
or vice versa) so the UI uses the same localized string across both
"threadIdPlaceholder" entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b711eeb6-625f-4470-8b43-e7ff92a939a3
📒 Files selected for processing (11)
src/main/presenter/index.tssrc/main/presenter/remoteControlPresenter/services/remoteConversationRunner.tssrc/main/presenter/remoteControlPresenter/telegram/telegramPoller.tssrc/main/presenter/remoteControlPresenter/types.tssrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsontest/main/presenter/remoteControlPresenter/remoteBindingStore.test.tstest/main/presenter/remoteControlPresenter/remoteConversationRunner.test.tstest/main/presenter/remoteControlPresenter/telegramPoller.test.ts
✅ Files skipped from review due to trivial changes (5)
- src/renderer/src/i18n/fr-FR/settings.json
- test/main/presenter/remoteControlPresenter/remoteBindingStore.test.ts
- src/main/presenter/remoteControlPresenter/telegram/telegramPoller.ts
- test/main/presenter/remoteControlPresenter/telegramPoller.test.ts
- src/main/presenter/remoteControlPresenter/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/src/i18n/da-DK/settings.json
- test/main/presenter/remoteControlPresenter/remoteConversationRunner.test.ts
- src/main/presenter/remoteControlPresenter/services/remoteConversationRunner.ts
This was referenced Mar 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Changes
Documentation
Tests