Skip to content

Commit 5370c79

Browse files
committed
improvement(ci): run the live desktop suite unless every change is clearly unrelated, and fail open
- The change check skips only docs, the other apps and published content; any change elsewhere, and any failure to fetch or diff the base, runs the suite - settled() discounts only requests a hold is keeping from Sim right now - The warm-up's Stop turn waits for its stream to close instead of on a promise that never settles
1 parent aa313ff commit 5370c79

4 files changed

Lines changed: 50 additions & 19 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#!/usr/bin/env bash
2+
# Prints `changed=false` only when every file a pull request changes is clearly unrelated to the
3+
# live desktop suite, and `changed=true` otherwise, including when the diff cannot be worked out.
4+
#
5+
# Usage: desktop-live-changes.sh <base-sha>
6+
set -u
7+
8+
unrelated='^(apps/docs/|apps/pii/|apps/sim/content/|packages/(python-sdk|ts-sdk)/)|\.mdx?$|(^|/)LICENSE$'
9+
10+
base=${1:-}
11+
run() {
12+
echo "changed=true"
13+
echo "Running the live desktop suite: $1" >&2
14+
exit 0
15+
}
16+
[ -n "$base" ] || run 'no base commit'
17+
git fetch --quiet --depth=1 origin "$base" || run "could not fetch $base"
18+
names=$(git diff --name-only "$base" HEAD) || run "could not diff against $base"
19+
[ -n "$names" ] || run 'no changed files listed'
20+
if printf '%s\n' "$names" | grep -qvE "$unrelated"; then
21+
run 'a change may affect it'
22+
fi
23+
echo "changed=false"
24+
echo 'Skipping the live desktop suite: every change is unrelated to it' >&2

‎.github/workflows/test-build.yml‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -442,35 +442,26 @@ jobs:
442442
if-no-files-found: ignore
443443
retention-days: 7
444444

445-
# Pull requests run the live desktop suite only when they touch what it exercises. The diff
446-
# comes from git against the pull request's base, so it needs no API access.
445+
# Pull requests skip the live desktop suite only when every change is clearly unrelated to the
446+
# app it drives (docs, other apps, published content). Anything else, and any failure to work
447+
# out the diff, runs it: a pull request that skipped it wrongly would first fail on staging.
447448
desktop-live-changes:
448449
name: Detect desktop tool changes
449450
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
450451
timeout-minutes: 5
451452
outputs:
452-
changed: ${{ github.event_name != 'pull_request' || steps.diff.outputs.changed == 'true' }}
453+
changed: ${{ github.event_name != 'pull_request' || steps.diff.outputs.changed != 'false' }}
453454
steps:
454455
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
455456
if: github.event_name == 'pull_request'
456457
with:
457458
fetch-depth: 2
458-
- name: Diff desktop tool paths
459+
- name: Diff against the pull request's base
459460
id: diff
460461
if: github.event_name == 'pull_request'
461462
env:
462463
BASE: ${{ github.event.pull_request.base.sha }}
463-
run: |
464-
git fetch --quiet --depth=1 origin "$BASE"
465-
paths='^(\.github/workflows/test-build\.yml|\.github/scripts/stop-session\.sh|bun\.lock'
466-
paths="$paths|apps/desktop/|apps/realtime/|packages/(browser-protocol|db|desktop-bridge|terminal-protocol)/"
467-
paths="$paths|apps/sim/app/api/(auth|copilot|desktop|mothership)/|apps/sim/app/workspace/[^/]+/(home|chat)/"
468-
paths="$paths|apps/sim/lib/(auth|desktop|mothership)/|apps/sim/stores/)"
469-
if git diff --name-only "$BASE" HEAD | grep -qE "$paths"; then
470-
echo "changed=true" >> "$GITHUB_OUTPUT"
471-
else
472-
echo "changed=false" >> "$GITHUB_OUTPUT"
473-
fi
464+
run: bash .github/scripts/desktop-live-changes.sh "$BASE" >> "$GITHUB_OUTPUT"
474465

475466
# Desktop tools in the real Electron app against a local app, on its own runner: the
476467
# Electron app, the dev app and its realtime server together outgrow the http-e2e runner.

‎apps/desktop/e2e/desktop-tools-live-sim.spec.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ test.describe('desktop tools against a live Sim', () => {
182182
)
183183
agent.script('[warm-stop]', async (turn) => {
184184
turn.text('Stopping soon.')
185-
await new Promise<void>(() => {})
185+
// The leg stays open until Stop ends it.
186+
await turn.closed
186187
})
187188
const page = await openApp(user, 'Warm chat', COMPILE_MS)
188189
await send(page, '[warm-up] read and import', COMPILE_MS)

‎apps/desktop/e2e/fixtures/live-sim.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ export class SimProxy {
170170
rewrittenChatBodies = 0
171171
private readonly server: Server
172172
private holds: HeldRequest[] = []
173+
/** Requests a hold is keeping from Sim right now. */
174+
private readonly heldEntries = new Set<ProxiedRequest>()
173175
private chatBodyRewrite: ((body: Record<string, unknown>) => void) | undefined
174176
private readonly sockets = new Set<Duplex>()
175177

@@ -248,9 +250,12 @@ export class SimProxy {
248250
let quietSince = Date.now()
249251
for (;;) {
250252
const waiting = this.requests.filter(
251-
(entry) => entry.status === undefined && entry.clientClosedAt === undefined
253+
(entry) =>
254+
entry.status === undefined &&
255+
entry.clientClosedAt === undefined &&
256+
!this.heldEntries.has(entry)
252257
)
253-
if (waiting.length > this.holds.length) quietSince = Date.now()
258+
if (waiting.length > 0) quietSince = Date.now()
254259
else if (Date.now() - quietSince >= 3_000) return
255260
if (Date.now() > deadline)
256261
throw new Error(
@@ -287,7 +292,10 @@ export class SimProxy {
287292
const held = this.holds.find((candidate) => candidate.matches(method, url.pathname))
288293
if (held) {
289294
this.holds = this.holds.filter((candidate) => candidate !== held)
290-
if (!(await held.hold(entry, response)) && !held.deliverIfAbandoned) return
295+
this.heldEntries.add(entry)
296+
const deliver = await held.hold(entry, response)
297+
this.heldEntries.delete(entry)
298+
if (!deliver && !held.deliverIfAbandoned) return
291299
}
292300
if (this.chatBodyRewrite && method === 'POST' && url.pathname === '/api/mothership/chat') {
293301
const parsed: Record<string, unknown> = JSON.parse(body.toString('utf8'))
@@ -346,6 +354,9 @@ interface Resume {
346354
/** A chat turn Sim opened against the agent, written to as the scripted model acts. */
347355
class AgentTurn {
348356
readonly toolCallIds: string[] = []
357+
/** Settles once this leg's stream has ended, from either side. */
358+
readonly closed: Promise<void>
359+
private markClosed!: () => void
349360
private seq = 0
350361
private readonly keepAlive: ReturnType<typeof setInterval>
351362
private ended = false
@@ -355,6 +366,9 @@ class AgentTurn {
355366
readonly streamId: string,
356367
private readonly response: ServerResponse
357368
) {
369+
this.closed = new Promise((resolve) => {
370+
this.markClosed = resolve
371+
})
358372
response.writeHead(200, {
359373
'Content-Type': 'text/event-stream',
360374
'Cache-Control': 'no-cache',
@@ -432,6 +446,7 @@ class AgentTurn {
432446
this.ended = true
433447
clearInterval(this.keepAlive)
434448
this.response.end()
449+
this.markClosed()
435450
}
436451
}
437452

0 commit comments

Comments
 (0)