Skip to content

Rename Call History to Phone and add a phone_call tool - #213

Merged
mattt merged 8 commits into
mainfrom
mattt/phone
Sep 8, 2026
Merged

mattt merged 8 commits into
mainfrom
mattt/phone

Conversation

@mattt

@mattt mattt commented Sep 8, 2026 •

Copy link
Copy Markdown
Owner

Follow up to #157

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical activation, argument-validation, and dialing-safety issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Renames Call History to Phone and adds outbound calling support.

Changes:

  • Adds phone_calls_fetch and phone_call.
  • Refactors call-history access.
  • Updates service registration and settings.
File summaries
File Review
App/Services/Phone.swift Critical: activation unnecessarily requires call-history access; fetch arguments lack validation; phone-number sanitization can dial unintended recipients.
App/Services/CallHistory.swift Removes the superseded standalone service.
App/Controllers/ServerController.swift Registers and configures the Phone service.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread App/Services/Phone.swift Outdated
Comment thread App/Services/Phone.swift Outdated
Comment thread App/Services/Phone.swift Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Four moderate issues in Phone.swift must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

App/Services/Phone.swift:185

  • storeBookmark(for:) catches bookmark creation failures, so this method logs that access was granted and continues even though no usable bookmark exists; the immediately following fetch then fails misleadingly with noBookmarkFound (or can reuse an older bookmark). Make bookmark creation throwing and propagate the failure here before reporting success.
    App/Services/Phone.swift:402
  • Any sqlite3_step result other than SQLITE_ROW is currently treated as successful completion. If stepping fails with SQLITE_BUSY, SQLITE_IOERR, or another error, this returns an empty or partial call list as a successful tool result. Check explicitly for SQLITE_DONE and throw SQLiteError for every other result.
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread App/Services/Phone.swift Outdated
Comment thread App/Services/Phone.swift

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Outbound calling currently accepts unsupported characters that may fail to initiate the requested call.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread App/Services/Phone.swift
@mattt
mattt merged commit ecef7a7 into main Sep 8, 2026
1 check passed
@mattt
mattt deleted the mattt/phone branch September 8, 2026 17:48
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.

2 participants