Skip to content

Comments

Sync desktop/ from omi-desktop (474e1e1f)#4960

Closed
m13v wants to merge 1 commit intomainfrom
sync/desktop-20260222-172315
Closed

Sync desktop/ from omi-desktop (474e1e1f)#4960
m13v wants to merge 1 commit intomainfrom
sync/desktop-20260222-172315

Conversation

@m13v
Copy link
Collaborator

@m13v m13v commented Feb 22, 2026

Automated sync from omi-desktop at 474e1e1f.

This PR was created by the sync pipeline.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request represents a major sync, introducing a significant set of features and architectural improvements. Key highlights include the unification of the AI chat bridge to a single, more capable ACPBridge with a robust OAuth flow, and a massive overhaul of the Tasks feature with local-first operations, full drag-and-drop reordering, and recurring tasks. The changes also bring numerous critical reliability fixes, such as non-blocking audio setup, improved memory management with watchdogs for external processes, and an auto-restart mechanism for extreme memory pressure. While the overall quality is excellent, I've identified one critical compilation issue in AgentSyncService where a return statement is missing.

Comment on lines 358 to 359
let body = String(data: data, encoding: .utf8) ?? ""
log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This code path is missing a return statement. The function pushData has a return type of PushResult, but this else block does not return a value, which will cause a compilation error. You should likely return .otherError here to handle non-5xx errors.

Suggested change
let body = String(data: data, encoding: .utf8) ?? ""
log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)")
let body = String(data: data, encoding: .utf8) ?? ""
log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)")
return .otherError

@m13v m13v closed this Feb 23, 2026
@m13v m13v deleted the sync/desktop-20260222-172315 branch February 23, 2026 03:32
@github-actions
Copy link
Contributor

Hey @m13v 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

  • Project standards — Ensuring consistency across the codebase
  • User needs — Making sure changes align with what our users need
  • Code best practices — Maintaining code quality and maintainability
  • Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant