ReviewOS
Reload

also looking at this

stacks/uplink

fix(app): a fresh install now either works or says why

#8
Merged glennmichael123 wants to merge fix/setup-blockers into main
16 files +512 -75

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

app/Commands/Uplink.tsmodified+26-11
@@ -30,7 +30,7 @@ async function openMessages(config: ReturnType<typeof loadConfig>, appDir: strin
3030 catch (error) {
3131 const message = error instanceof MessagesAccessError ? error.message : String(error)
3232 console.error(`[uplink] ${message} Retrying in ${RETRY_OPEN_MS / 1000}s.`)
33 await writeHeartbeat(appDir, { pid: process.pid, startedAt, lastPollAt: null, lastError: message, allowed: config.allowed, own: [], active: [] })
33 await writeHeartbeat(appDir, { pid: process.pid, startedAt, lastPollAt: null, lastError: message, automation: null, lastSendError: null, allowed: config.allowed, own: [], active: [] })
3434 await Bun.sleep(RETRY_OPEN_MS)
3535 }
3636 }
@@ -73,24 +73,39 @@ export default defineCommand((cli) => {
7373 keepMessagesOpen()
7474
7575 const messages = await openMessages(config, appDir, startedAt)
76 const sender = new AppleScriptSender()
7677 const uplink = new Uplink({
7778 config,
7879 messages,
79 sender: new AppleScriptSender(),
80 sender,
8081 engine: selectedEngine(config),
8182 store: new ModelStore(`${appDir}/storage/uplink/cursor`),
8283 })
8384 await uplink.start()
8485
85 const beat = (): Promise<number> => writeHeartbeat(appDir, {
86 pid: process.pid,
87 startedAt,
88 lastPollAt: uplink.lastPollAt,
89 lastError: uplink.lastError,
90 allowed: uplink.allowedHandles,
91 own: uplink.ownHandles,
92 active: uplink.activeRuns,
93 })
86 // The Automation grant is per app, so only this process can prove that
87 // Uplink.app may drive Messages. Ask once at start, and again whenever a
88 // send has failed, so the doctor and the dashboard report the subject
89 // that actually sends rather than whichever terminal ran them.
90 let automation = await sender.canSend()
91 if (!automation.ok)
92 console.error(`[uplink] cannot send through Messages: ${automation.detail}`)
93
94 const beat = async (): Promise<number> => {
95 if (uplink.lastSendError && automation.ok)
96 automation = await sender.canSend()
97 return writeHeartbeat(appDir, {
98 pid: process.pid,
99 startedAt,
100 lastPollAt: uplink.lastPollAt,
101 lastError: uplink.lastError,
102 automation: { ok: automation.ok, detail: automation.detail },
103 lastSendError: uplink.lastSendError,
104 allowed: uplink.allowedHandles,
105 own: uplink.ownHandles,
106 active: uplink.activeRuns,
107 })
108 }
94109 // .env is read once, at start. Editing it (a new token, say) used to do
95110 // nothing until someone remembered uplink:restart, and every text in
96111 // between failed. Under launchd, which restarts us, pick it up on our
app/Uplink/desktop-agent.tsmodified+39-1
@@ -2,6 +2,7 @@
22import type { Server } from 'bun'
33import type { UplinkConfig } from './config'
44import type { EngineId, EngineInstall, EngineProbe, ProbeReason } from './engine'
5import type { AutomationState } from './sender'
56import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
67import { homedir } from 'node:os'
78import { join } from 'node:path'
@@ -161,6 +162,7 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
161162 if (settings.openAtLogin && (!existsSync(LAUNCH_AGENT) || !readFileSync(LAUNCH_AGENT, 'utf8').includes(`<string>${process.execPath}</string>`)))
162163 setOpenAtLogin(true)
163164
165 const sender = new AppleScriptSender()
164166 const store = new SqliteStore(DATABASE_PATH)
165167 const startedAt = Date.now()
166168 let messages: MessagesDb | null = null
@@ -168,6 +170,17 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
168170 let uplink: Uplink | null = null
169171 const engines = new Map<string, EngineState>()
170172 let checkingEngines = false
173 let automation: AutomationState | null = null
174
175 /**
176 * Whether macOS will let this app drive Messages. This process is Uplink.app,
177 * so unlike `buddy uplink:doctor` under Terminal it is the right subject to
178 * ask. The first call raises the prompt, which is why setup calls it rather
179 * than leaving it to the first real reply.
180 */
181 const checkAutomation = async (): Promise<void> => {
182 automation = await sender.canSend()
183 }
171184
172185 /**
173186 * Probe every engine, not only the selected one: the popover shows the other
@@ -198,7 +211,7 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
198211 messagesError = error instanceof MessagesAccessError ? 'Uplink needs Full Disk Access to read Messages.' : String(error)
199212 return
200213 }
201 uplink = new Uplink({ config, messages, sender: new AppleScriptSender(), engine: selectedEngine(config), store })
214 uplink = new Uplink({ config, messages, sender, engine: selectedEngine(config), store })
202215 await uplink.start()
203216 }
204217
@@ -218,10 +231,16 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
218231 const keepMessagesOpen = setInterval(() => {
219232 if (uplink && Bun.spawnSync(['pgrep', '-x', 'Messages']).exitCode !== 0)
220233 Bun.spawnSync(['open', '-g', '-a', 'Messages'])
234 // Re-ask while the answer is anything but yes. A refusal only changes in
235 // System Settings, and a timeout from a Messages that was still starting
236 // should not leave the app stuck in setup for ever.
237 if (!automation?.ok)
238 void checkAutomation()
221239 }, 15_000)
222240
223241 await startWatching()
224242 void checkEngines()
243 void checkAutomation()
225244
226245 const status = () => {
227246 const recent = store.recentRuns(8).map(run => ({
@@ -273,6 +292,16 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
273292 detail: allowed.length > 0 ? allowed.join(', ') : 'No handles yet. Sign in to Messages, or add your number.',
274293 informational: false,
275294 },
295 {
296 // The outbound half of the chain. Without this the app reported itself
297 // ready while every reply was refused, so a first text produced a run
298 // marked done and a phone that heard nothing.
299 id: 'automation',
300 name: 'Send replies',
301 ok: automation?.ok ?? false,
302 detail: uplink?.lastSendError ?? automation?.detail ?? 'Not checked yet',
303 informational: false,
304 },
276305 ]
277306 // An engine the person has not selected must not hold the app in setup.
278307 const ready = checks.every(check => check.ok || check.informational)
@@ -325,6 +354,15 @@ export async function startDesktopAgent(options: { version: string }): Promise<D
325354 Bun.spawnSync(['open', 'x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles'])
326355 return json({ ok: true })
327356
357 case '/api/automation/test':
358 await checkAutomation()
359 return json(automation)
360
361 case '/api/open/automation': {
362 Bun.spawnSync(['open', 'x-apple.systempreferences:com.apple.preference.security?Privacy_Automation'])
363 return json({ ok: true })
364 }
365
328366 case '/api/open/install-docs': {
329367 // The selected engine's own documentation, so the link cannot drift
330368 // from the command shown beside it.
app/Uplink/doctor.tsmodified+26-1
@@ -35,6 +35,10 @@ export interface Heartbeat {
3535 startedAt: number
3636 lastPollAt: number | null
3737 lastError: string | null
38 /** Whether the watcher itself can drive Messages. Null before it has asked. */
39 automation: { ok: boolean, detail: string } | null
40 /** Why the last reply could not be delivered, if one could not. */
41 lastSendError: string | null
3842 allowed: string[]
3943 own: string[]
4044 active: Array<{ prompt: string, startedAt: number, lastActivity: string | null, queued: number }>
@@ -150,7 +154,28 @@ export async function runChecks(config: UplinkConfig, appDir: string): Promise<C
150154 ? { name: 'Messages app', ok: true, detail: 'running' }
151155 : { name: 'Messages app', ok: false, detail: 'not running', fix: 'Open Messages and sign in with your Apple ID' })
152156
153 checks.push({ name: 'Sending replies', ok: true, detail: 'macOS asks once, on the first reply, to let Uplink control Messages - click Allow' })
157 // The Automation grant is per app: the watcher inside Uplink.app and this
158 // command under Terminal are different subjects. Only the watcher's answer is
159 // about the thing that will actually send, so ask the heartbeat rather than
160 // measuring this process and reporting it as Uplink's.
161 if (fresh && heartbeat.automation) {
162 checks.push(heartbeat.automation.ok
163 ? { name: 'Sending replies', ok: true, detail: heartbeat.automation.detail }
164 : {
165 name: 'Sending replies',
166 ok: false,
167 detail: heartbeat.lastSendError ?? heartbeat.automation.detail,
168 fix: `Allow ${paths.bundle} to control Messages in System Settings > Privacy & Security > Automation, then ./buddy uplink:restart. macOS asks once; if it was declined, the switch is in that pane.`,
169 })
170 }
171 else {
172 checks.push({
173 name: 'Sending replies',
174 ok: false,
175 detail: 'not checked: the watcher is not running, and only it can prove Uplink.app may control Messages',
176 fix: 'Start it with ./buddy uplink:install, then run this again',
177 })
178 }
154179
155180 return checks
156181}
app/Uplink/menubar.htmlmodified+19-0
@@ -134,6 +134,20 @@
134134 <div class="actions"><button data-action="save-allowed">Save</button></div>
135135 </div>
136136 </div>
137 <div class="card step" id="step-automation">
138 <span class="mark" data-mark>4</span>
139 <div>
140 <strong>Let Uplink send replies</strong>
141 <p data-detail></p>
142 <div data-when-todo>
143 <p class="help">macOS asks once for permission to control Messages. Do it now, at the Mac: nobody can answer that prompt from a phone, and until it is answered a reply has nowhere to go.</p>
144 <div class="actions">
145 <button class="primary" data-action="test-automation">Ask now</button>
146 <button data-action="open-automation">Open Automation settings</button>
147 </div>
148 </div>
149 </div>
150 </div>
137151 </section>
138152
139153 <section id="now" hidden>
@@ -236,6 +250,7 @@
236250 renderStep('#step-fda', byId.fda)
237251 renderStep('#step-engine', byId.engine)
238252 renderStep('#step-handles', byId.handles)
253 renderStep('#step-automation', byId.automation)
239254 renderEngine(status, byId.engine)
240255 if (!allowedEdited)
241256 $('#allowed').value = status.settings.allowed.join('\n')
@@ -343,6 +358,10 @@
343358 catch {}
344359 },
345360 'open-install-docs': () => api('/api/open/install-docs', {}),
361 // Raises the macOS prompt if it has never been answered, and re-reads the
362 // grant if it has.
363 'test-automation': () => api('/api/automation/test', {}).then(refresh),
364 'open-automation': () => api('/api/open/automation', {}),
346365 'save-token': async () => {
347366 const error = $('#token-error')
348367 error.hidden = true
app/Uplink/sender.tsmodified+63-0
@@ -15,8 +15,27 @@ export interface ReplyTarget {
1515 service: string
1616}
1717
18/**
19 * Whether macOS will let this process drive Messages.
20 *
21 * The grant is per app and per process identity: the watcher inside Uplink.app
22 * and `buddy uplink:doctor` under Terminal are two different subjects, so a
23 * result is only ever about the process that asked.
24 */
25export interface AutomationState {
26 ok: boolean
27 reason: 'ok' | 'denied' | 'failed'
28 detail: string
29}
30
1831export interface Sender {
1932 send: (target: ReplyTarget, text: string) => Promise<void>
33 /**
34 * Can this process send at all. The first call raises the macOS Automation
35 * prompt, which is the point of having it: it has to happen while someone is
36 * at the Mac, not on the first real reply to a phone that then hears nothing.
37 */
38 canSend: () => Promise<AutomationState>
2039}
2140
2241const SCRIPT = `
@@ -40,7 +59,51 @@ on run argv
4059end run
4160`
4261
62/**
63 * Reads from Messages and sends nothing, so it can be run during setup and on
64 * demand. Counting accounts needs exactly the Automation permission a real
65 * reply needs.
66 */
67const PROBE_SCRIPT = 'tell application "Messages" to count of accounts'
68
69/** macOS error -1743 is "not authorized to send Apple events". */
70const NOT_AUTHORIZED = /-1743|not authori[sz]ed|not allowed to send|doesn't have permission/i
71
72/** -1712 is an AppleEvent timeout, which a busy or starting Messages produces. */
73const TIMED_OUT = /-1712|timed out/i
74
75/** Long enough for someone to read the macOS prompt and click Allow. */
76const PROBE_TIMEOUT_MS = 60_000
77
4378export class AppleScriptSender implements Sender {
79 async canSend(): Promise<AutomationState> {
80 try {
81 const proc = Bun.spawn(['osascript', '-e', PROBE_SCRIPT], { stdin: 'ignore', stdout: 'pipe', stderr: 'pipe' })
82 // The prompt itself blocks the script until someone answers it, and a
83 // busy or starting Messages can time out on its own, so this waits
84 // generously rather than reporting a refusal that has not happened.
85 const timer = setTimeout(() => proc.kill(), PROBE_TIMEOUT_MS)
86 const [code, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()])
87 clearTimeout(timer)
88
89 if (code === 0)
90 return { ok: true, reason: 'ok', detail: 'Uplink can send through Messages' }
91
92 const said = stderr.trim()
93 if (NOT_AUTHORIZED.test(said))
94 return { ok: false, reason: 'denied', detail: 'macOS has not been allowed to let Uplink control Messages' }
95
96 // A timeout is not a refusal. Say so, because the two want different
97 // things from the person reading it.
98 return TIMED_OUT.test(said)
99 ? { ok: false, reason: 'failed', detail: 'Messages did not answer in time. It may still be starting up.' }
100 : { ok: false, reason: 'failed', detail: said || `osascript exited ${code}` }
101 }
102 catch (error) {
103 return { ok: false, reason: 'failed', detail: error instanceof Error ? error.message : String(error) }
104 }
105 }
106
44107 async send(target: ReplyTarget, text: string): Promise<void> {
45108 const proc = Bun.spawn(
46109 ['osascript', '-e', SCRIPT, text, target.chatGuid, target.handle, target.service === 'SMS' ? 'SMS' : 'iMessage'],
app/Uplink/uplink.tsmodified+18-4
@@ -89,6 +89,8 @@ export class Uplink {
8989 private timer: ReturnType<typeof setTimeout> | null = null
9090 lastPollAt: number | null = null
9191 lastError: string | null = null
92 /** Why the last reply could not be delivered, for the heartbeat and the popover. */
93 lastSendError: string | null = null
9294 private ticking = false
9395 private stopped = false
9496
@@ -373,7 +375,16 @@ export class Uplink {
373375 const text = result.authFailure
374376 ? `I cannot reach ${engine.label}: it is not signed in on this Mac. Someone there needs to ${engine.authFailureHint}.`
375377 : result.ok ? result.text : `That failed: ${result.text}`
376 await this.reply(active.target, text, { keepRest: true })
378
379 // A run whose answer never left the Mac is not done, whatever the agent
380 // did. Recording it as done is how a first install reports itself healthy
381 // while the phone hears nothing.
382 if (!await this.reply(active.target, text, { keepRest: true })) {
383 await this.deps.store.updateRun(active.runId, {
384 status: 'failed',
385 error: `The answer could not be delivered through Messages: ${this.lastSendError ?? 'unknown error'}`,
386 })
387 }
377388 }
378389
379390 const next = state.queue.shift()
@@ -437,7 +448,7 @@ export class Uplink {
437448 * and easy to lose track of. Short status replies never touch the stored
438449 * conversation, so one sent mid-run cannot race the run's own save.
439450 */
440 async reply(target: ReplyTarget, text: string, options: { keepRest?: boolean } = {}): Promise<void> {
451 async reply(target: ReplyTarget, text: string, options: { keepRest?: boolean } = {}): Promise<boolean> {
441452 const parts = chunk(toPlainText(text), this.config.maxChars - this.config.replyPrefix.length)
442453 const now = parts.slice(0, this.config.maxParts)
443454 const later = parts.slice(this.config.maxParts)
@@ -456,10 +467,13 @@ export class Uplink {
456467 await this.deps.sender.send(target, body)
457468 }
458469 catch (error) {
459 this.log.error(`Reply to ${target.handle} failed: ${error instanceof Error ? error.message : String(error)}`)
460 return
470 this.lastSendError = error instanceof Error ? error.message : String(error)
471 this.log.error(`Reply to ${target.handle} failed: ${this.lastSendError}`)
472 return false
461473 }
462474 }
475 this.lastSendError = null
476 return true
463477 }
464478}
465479
tests/unit/uplink/fixtures.tsmodified+11-1
@@ -1,5 +1,5 @@
11import type { Engine, EngineEvent, EngineProbe, EngineRequest, EngineResult, EngineRun } from '../../../app/Uplink/engine'
2import type { ReplyTarget, Sender } from '../../../app/Uplink/sender'
2import type { AutomationState, ReplyTarget, Sender } from '../../../app/Uplink/sender'
33import { Database } from 'bun:sqlite'
44import { mkdtempSync } from 'node:fs'
55import { tmpdir } from 'node:os'
@@ -119,10 +119,20 @@ export class FakeChatDb {
119119
120120export class FakeSender implements Sender {
121121 sent: Array<{ target: ReplyTarget, text: string }> = []
122 /** Set to make every send fail, the way a refused Automation grant does. */
123 refuse: string | null = null
124 automation: AutomationState = { ok: true, reason: 'ok', detail: 'Uplink can send through Messages' }
125
122126 /** Mirror each send into chat.db, the way Messages records it. */
123127 constructor(private readonly db?: FakeChatDb) {}
124128
129 async canSend(): Promise<AutomationState> {
130 return this.automation
131 }
132
125133 async send(target: ReplyTarget, text: string): Promise<void> {
134 if (this.refuse)
135 throw new Error(this.refuse)
126136 this.sent.push({ target, text })
127137 this.db?.add({ chat: target.handle, text, fromMe: true })
128138 }
tests/unit/uplink/uplink.test.tsmodified+17-0
@@ -190,6 +190,23 @@ describe('Uplink', () => {
190190 expect(h.engine.runs[1].request.sessionId).toBeNull()
191191 })
192192
193 it('does not record a run as done when the reply could not be delivered', async () => {
194 const h = await harness()
195 // What a refused Messages Automation grant looks like from here. Without
196 // this the run was recorded done, the dashboard showed an answer, and the
197 // phone heard nothing at all.
198 h.sender.refuse = 'Messages refused to send (osascript exit 1): not authorized to send Apple events'
199 h.fake.add({ chat: ME, text: 'hello', fromMe: true })
200 await h.uplink.tick()
201 h.engine.finish(0, 'Here are the scores')
202 await settle()
203
204 expect(h.sender.texts()).toEqual([])
205 expect(h.store.runs.get(1)?.status).toBe('failed')
206 expect(h.store.runs.get(1)?.error).toContain('could not be delivered')
207 expect(h.uplink.lastSendError).toContain('not authorized')
208 })
209
193210 it('tells the user which CLI is not logged in, and how to fix it', async () => {
194211 const h = await harness()
195212 h.fake.add({ chat: ME, text: 'hello', fromMe: true })