From 13ce3e4be1af2d821ea734694b91954ee23b022b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Val=20Alexander=20=F0=9F=91=91?= Date: Sun, 8 Nov 2026 15:29:07 -0600 Subject: [PATCH 1/2] fix(ios): preserve drafts during command picker recovery --- apps/ios/ModexCompanion/Scripts/fixture.mjs | 24 +++++- apps/ios/ModexCompanion/Scripts/test-e2e.sh | 3 + .../Sources/CompanionModel.swift | 14 +++- .../Sources/CompanionViews.swift | 22 +++++- .../Tests/CompanionModelTests.swift | 72 ++++++++++++++++++ .../UITests/CompanionFlowTests.swift | 75 ++++++++++++++++++- .../2026-11-08-companion-recovery-pr.md | 46 ++++++++++++ .../checkpoint.md | 21 ++++++ docs/status.md | 1 + 9 files changed, 273 insertions(+), 5 deletions(-) create mode 100644 docs/reviews/2026-11-08-companion-recovery-pr.md create mode 100644 docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md diff --git a/apps/ios/ModexCompanion/Scripts/fixture.mjs b/apps/ios/ModexCompanion/Scripts/fixture.mjs index 086a902c..b1b7cbc8 100644 --- a/apps/ios/ModexCompanion/Scripts/fixture.mjs +++ b/apps/ios/ModexCompanion/Scripts/fixture.mjs @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import net from "node:net"; import http from "node:http"; +import https from "node:https"; import { randomUUID } from "node:crypto"; import { Store } from "../../../desktop/dist/src/main/engine/store.js"; import { ThreadRunner } from "../../../desktop/dist/src/main/engine/runner.js"; @@ -16,6 +17,20 @@ import { EventJournal } from "../../../desktop/dist/src/main/engine/event-journa // with what the process did meanwhile: CPU time means our own work, major faults paging, neither a // starved process. const log = (message) => process.stderr.write(`[fixture ${new Date().toISOString()}] ${message}\n`); +// Test-process transport fault: consume exactly one armed command GET without answering. +// The phone's unchanged request timeout must show recovery inside the picker. All other +// traffic, including snapshot polling and mutations, keeps the real CompanionServer path. +let stallNextCommands = false; +const createHttpsServer = https.createServer; +https.createServer = (options, listener) => createHttpsServer(options, (request, response) => { + if (stallNextCommands && request.method === "GET" && request.url?.startsWith("/v1/commands?")) { + stallNextCommands = false; + log("holding one command GET until the phone times out"); + request.on("close", () => log("held command GET closed")); + return; + } + listener(request, response); +}); let lastTick = performance.now(); let lastUsage = process.resourceUsage(); setInterval(() => { @@ -134,6 +149,12 @@ const resumePath = `/resume/${randomUUID()}`; // The test closes a question on the Mac: POST /answer or /stop. const macPath = `/mac/${randomUUID()}`; const control = http.createServer(async (request, response) => { + if (request.method === "POST" && request.url === `${macPath}/timeout-commands`) { + stallNextCommands = true; + log("armed one command GET timeout"); + response.writeHead(204).end(); + return; + } if (request.method === "POST" && (request.url === `${macPath}/answer` || request.url === `${macPath}/stop`)) { const how = request.url.endsWith("/answer") ? "answer" : "stop"; const id = macThreads[how]; @@ -174,7 +195,7 @@ const control = http.createServer(async (request, response) => { await new Promise((resolve) => control.listen(0, "127.0.0.1", resolve)); const resumeURL = `http://127.0.0.1:${control.address().port}${resumePath}`; const macURL = `http://127.0.0.1:${control.address().port}${macPath}`; -fs.writeFileSync(process.argv[2], JSON.stringify({ link: server.status().pairingUri, reconnect, resumeURL, macURL })); +fs.writeFileSync(process.argv[2], JSON.stringify({ link: server.status().pairingUri, reconnect, resumeURL, macURL, defaultProjectName: store.snapshot().projects[0]?.name })); async function pauseMac() { const resumed = new Promise((resolve) => { resumePausedMac = resolve; }); @@ -204,6 +225,7 @@ async function restartMac() { async function close() { closing = true; + https.createServer = createHttpsServer; clearTimeout(restartTimer); resumePausedMac?.(); await restartTask; diff --git a/apps/ios/ModexCompanion/Scripts/test-e2e.sh b/apps/ios/ModexCompanion/Scripts/test-e2e.sh index b09ffd97..5fe57b67 100755 --- a/apps/ios/ModexCompanion/Scripts/test-e2e.sh +++ b/apps/ios/ModexCompanion/Scripts/test-e2e.sh @@ -38,5 +38,8 @@ if [[ -z "$xctestrun" ]]; then echo "Xcode did not create an iPhone test plan." # UITestBudget in UITests/XCUIElement+Ready.swift). Every UI-test wait is multiplied by this; # xcodebuild hands TEST_RUNNER_* variables to the test runner's environment. timeout_scale="${MODEX_UI_TIMEOUT_SCALE:-1}" +mkdir -p "$derived_data/Logs/Test" +result_bundle="$derived_data/Logs/Test/Paired-$(date -u +%Y%m%dT%H%M%SZ)-$$.xcresult" TEST_RUNNER_MODEX_UI_TIMEOUT_SCALE="$timeout_scale" xcodebuild -xctestrun "$xctestrun" -destination "$destination" \ + -resultBundlePath "$result_bundle" \ -parallel-testing-enabled NO -only-testing:ModexCompanionUITests/DemoWorkspaceTests -only-testing:ModexCompanionUITests/CompanionFlowTests test-without-building diff --git a/apps/ios/ModexCompanion/Sources/CompanionModel.swift b/apps/ios/ModexCompanion/Sources/CompanionModel.swift index 8f0b836d..99198608 100644 --- a/apps/ios/ModexCompanion/Sources/CompanionModel.swift +++ b/apps/ios/ModexCompanion/Sources/CompanionModel.swift @@ -47,6 +47,7 @@ import SwiftUI private var answerOperation: UUID? private var commandOperation: UUID? private var commandContext: String? + @Published private var commandFailure: (context: String, message: String)? init(store: any PairingStorage = PairingStore(), discovery: (any CompanionDiscovery)? = nil, defaults: UserDefaults = .standard, makeClient: @escaping (Pairing) -> any CompanionClient = { CompanionAPI(pairing: $0) }) { self.store = store @@ -218,6 +219,7 @@ import SwiftUI answerOperation = nil commandOperation = nil commandContext = nil + commandFailure = nil retrySavedEndpoint = false sending = false answeringId = nil @@ -372,7 +374,9 @@ import SwiftUI let operation = UUID() let context = "\(projectId)\u{0}\(backend)" commandOperation = operation - if commandContext != context { commands = [] } + commandContext = context + commands = [] + commandFailure = nil do { let result = try await api.commands(projectId: projectId, backend: backend) if connection == connectionRevision, commandOperation == operation, !Task.isCancelled { @@ -380,7 +384,9 @@ import SwiftUI commandContext = context } } catch { - if connection == connectionRevision, commandOperation == operation, !Task.isCancelled, !revokeIfNeeded(error) { self.error = error.localizedDescription } + if connection == connectionRevision, commandOperation == operation, !Task.isCancelled, !revokeIfNeeded(error) { + commandFailure = (context, error.localizedDescription) + } } } @@ -388,6 +394,10 @@ import SwiftUI commandContext == "\(projectId)\u{0}\(backend)" ? commands : [] } + func commandError(projectId: String, backend: String) -> String? { + commandFailure?.context == "\(projectId)\u{0}\(backend)" ? commandFailure?.message : nil + } + func answer(itemId: String, approve: Bool) async { guard let api, let selectedThreadId, answeringId == nil else { return } let connection = connectionRevision diff --git a/apps/ios/ModexCompanion/Sources/CompanionViews.swift b/apps/ios/ModexCompanion/Sources/CompanionViews.swift index c6623c3a..48a86854 100644 --- a/apps/ios/ModexCompanion/Sources/CompanionViews.swift +++ b/apps/ios/ModexCompanion/Sources/CompanionViews.swift @@ -666,6 +666,7 @@ private struct CommandSheet: View { let onSelect: (String) -> Void @State private var query = "" @State private var loading = true + @State private var retry = 0 private var filtered: [CompanionCommand] { let commands = model.availableCommands(projectId: projectId, backend: backend) @@ -684,6 +685,17 @@ private struct CommandSheet: View { .background(Palette.raised, in: RoundedRectangle(cornerRadius: 14)) if loading { ProgressView().tint(Palette.accent).frame(maxWidth: .infinity, maxHeight: .infinity) + } else if let error = model.commandError(projectId: projectId, backend: backend) { + VStack(spacing: 12) { + Text("Couldn’t load skills and commands").font(.headline) + Text(error).font(.subheadline).foregroundStyle(Palette.muted) + Button("Try again") { retry += 1 } + .buttonStyle(.bordered) + .frame(minHeight: 44) + .accessibilityIdentifier("retry-commands") + } + .multilineTextAlignment(.center) + .frame(maxWidth: .infinity, maxHeight: .infinity) } else if filtered.isEmpty { ContentUnavailableView("No commands found", systemImage: "command", description: Text("Install skills on your Mac or type a command directly.")) .foregroundStyle(Palette.muted) @@ -715,7 +727,7 @@ private struct CommandSheet: View { .toolbar { ToolbarItem(placement: .topBarTrailing) { Button("Done") { dismiss() }.foregroundStyle(Palette.muted) } } } .presentationDetents([.medium, .large]) - .task(id: "\(projectId)\u{0}\(backend)") { + .task(id: "\(projectId)\u{0}\(backend)\u{0}\(retry)") { loading = true await model.loadCommands(projectId: projectId, backend: backend) if !Task.isCancelled { loading = false } @@ -803,6 +815,7 @@ private struct QuestionCard: View { @EnvironmentObject private var model: CompanionModel let item: CompanionItem let busy: Bool + @FocusState private var focusedSecret: String? var body: some View { VStack(alignment: .leading, spacing: 15) { @@ -889,6 +902,9 @@ private struct QuestionCard: View { Group { if question.secret { SecureField(prompt, text: text).textContentType(.password) + .focused($focusedSecret, equals: question.id) + .submitLabel(.done) + .onSubmit { focusedSecret = nil } } else { TextField(prompt, text: text, axis: .vertical).lineLimit(1...4) } @@ -948,6 +964,7 @@ private struct PairingSheet: View { @Environment(\.dismiss) private var dismiss @State private var link = "" @State private var scanning = false + @FocusState private var linkFocused: Bool var body: some View { NavigationStack { @@ -969,6 +986,9 @@ private struct PairingSheet: View { TextField("modex://pair?data=…", text: $link) .textInputAutocapitalization(.never).autocorrectionDisabled() .keyboardType(.URL).font(.system(size: 13, design: .monospaced)) + .focused($linkFocused) + .submitLabel(.done) + .onSubmit { linkFocused = false } .padding(15).background(Palette.raised, in: RoundedRectangle(cornerRadius: 13)) .accessibilityIdentifier("pairing-link-input") Button(model.isPairing ? "Connecting…" : "Connect") { Task { if await model.open(link: link) { dismiss() } } } diff --git a/apps/ios/ModexCompanion/Tests/CompanionModelTests.swift b/apps/ios/ModexCompanion/Tests/CompanionModelTests.swift index 80339ca1..93af534a 100644 --- a/apps/ios/ModexCompanion/Tests/CompanionModelTests.swift +++ b/apps/ios/ModexCompanion/Tests/CompanionModelTests.swift @@ -422,6 +422,76 @@ import XCTest XCTAssertTrue(model.availableCommands(projectId: "p", backend: "codex").isEmpty) } + func testCommandTimeoutKeepsTheDraftAndDoesNotPresentTheRootAlert() async { + let (model, client) = fixture() + model.newThreadDraft = "Start from phone" + let pending = Pending<[CompanionCommand]>(requested: expectation(description: "commands time out")) + await client.enqueueCommands(pending) + let request = Task { await model.loadCommands(projectId: "p", backend: "codex") } + await fulfillment(of: [pending.requested], timeout: 2) + await pending.resolve(.failure(URLError(.timedOut))) + await request.value + + XCTAssertNil(model.error, "A command-list failure must stay in the picker; the root alert dismisses the new-thread sheet.") + XCTAssertNotNil(model.commandError(projectId: "p", backend: "codex")) + XCTAssertNil(model.commandError(projectId: "other", backend: "codex")) + XCTAssertNil(model.commandError(projectId: "p", backend: "claude")) + XCTAssertEqual(model.newThreadDraft, "Start from phone") + XCTAssertTrue(model.availableCommands(projectId: "p", backend: "codex").isEmpty) + XCTAssertNil(client.createdText, "Loading skills must never submit the draft.") + let beforeRetry = await client.calls + XCTAssertEqual(beforeRetry, ["commands:p:codex"], "A failed read must not retry itself.") + + let retry = Pending<[CompanionCommand]>(requested: expectation(description: "explicit command retry")) + await client.enqueueCommands(retry) + let retried = Task { await model.loadCommands(projectId: "p", backend: "codex") } + await fulfillment(of: [retry.requested], timeout: 2) + XCTAssertNil(model.commandError(projectId: "p", backend: "codex")) + let command = CompanionCommand(id: "skill:mobile-check", title: "mobile-check", detail: "Verify", insertion: "$mobile-check ", kind: "skill") + await retry.resolve(.success([command])) + await retried.value + XCTAssertEqual(model.availableCommands(projectId: "p", backend: "codex"), [command]) + XCTAssertEqual(model.newThreadDraft, "Start from phone") + XCTAssertNil(client.createdText, "Retrying a read must not submit the draft either.") + let afterRetry = await client.calls + XCTAssertEqual(afterRetry, ["commands:p:codex", "commands:p:codex"]) + } + + func testLateCommandFailureCannotReplaceAnotherProjectsResults() async { + let (model, client) = fixture() + let old = Pending<[CompanionCommand]>(requested: expectation(description: "old project commands")) + let current = Pending<[CompanionCommand]>(requested: expectation(description: "new project commands")) + await client.enqueueCommands(old) + await client.enqueueCommands(current) + let previous = Task { await model.loadCommands(projectId: "p", backend: "codex") } + await fulfillment(of: [old.requested], timeout: 2) + let next = Task { await model.loadCommands(projectId: "other", backend: "codex") } + await fulfillment(of: [current.requested], timeout: 2) + let command = CompanionCommand(id: "skill:other", title: "other", detail: "Other project", insertion: "$other ", kind: "skill") + await current.resolve(.success([command])) + await next.value + await old.resolve(.failure(URLError(.timedOut))) + await previous.value + XCTAssertEqual(model.availableCommands(projectId: "other", backend: "codex"), [command]) + XCTAssertNil(model.commandError(projectId: "other", backend: "codex")) + XCTAssertNil(model.commandError(projectId: "p", backend: "codex")) + XCTAssertNil(model.error) + } + + func testCommandRevocationStillForgetsTheMac() async { + let (model, client) = fixture() + let pending = Pending<[CompanionCommand]>(requested: expectation(description: "commands revoked")) + await client.enqueueCommands(pending) + let request = Task { await model.loadCommands(projectId: "p", backend: "codex") } + await fulfillment(of: [pending.requested], timeout: 2) + await pending.resolve(.failure(CompanionError.accessRevoked)) + await request.value + XCTAssertNil(model.pairing) + XCTAssertFalse(model.connected) + XCTAssertNotNil(model.error, "Revocation must retain the existing pairing recovery alert.") + XCTAssertNil(model.commandError(projectId: "p", backend: "codex")) + } + func testCancelledCommandPickerDoesNotPublishResultsOrErrors() async { let command = CompanionCommand(id: "skill:old", title: "old", detail: "Skill", insertion: "$old ", kind: "skill") for result: Result<[CompanionCommand], Error> in [.success([command]), .failure(CancellationError())] { @@ -435,6 +505,7 @@ import XCTest await request.value XCTAssertNil(model.error) XCTAssertTrue(model.commands.isEmpty) + XCTAssertNil(model.commandError(projectId: "p", backend: "codex")) } } @@ -905,6 +976,7 @@ private actor ControlledClient: CompanionClient { return createdThread! } func commands(projectId: String, backend: String) async throws -> [CompanionCommand] { + calls.append("commands:\(projectId):\(backend)") if !commandLists.isEmpty { return try await commandLists.removeFirst().value() } return [] } diff --git a/apps/ios/ModexCompanion/UITests/CompanionFlowTests.swift b/apps/ios/ModexCompanion/UITests/CompanionFlowTests.swift index ab36aba2..f76097ad 100644 --- a/apps/ios/ModexCompanion/UITests/CompanionFlowTests.swift +++ b/apps/ios/ModexCompanion/UITests/CompanionFlowTests.swift @@ -278,6 +278,59 @@ final class CompanionFlowTests: XCTestCase { } } + func testFreshNewThreadRequestsUseTheirOwnProjectContext() throws { + continueAfterFailure = false + guard let file = Bundle(for: Self.self).url(forResource: "LocalPairing", withExtension: "json"), + let data = try? Data(contentsOf: file), + let fixture = try? JSONDecoder().decode(Fixture.self, from: data) else { + throw XCTSkip("Run Scripts/test-e2e.sh with the paired Mac fixture.") + } + let defaultProject = try XCTUnwrap(fixture.defaultProjectName, "The fixture must name the first snapshot project, which is the global sheet's default.") + let prompts = answerSystemAlertsDuringActions() + defer { removeUIInterruptionMonitor(prompts) } + let app = XCUIApplication() + app.launch() + defer { app.terminate() } + let pair = app.buttons["pair-button"] + if !pair.waitForExistence(timeout: UITestBudget.seconds(5)) { forgetMac(app) } + pair.tapWhenReady() + let link = app.inputField("pairing-link-input") + link.typeTextWhenReady(fixture.link, in: app) + let keyboard = app.keyboards.firstMatch + if keyboard.exists { + keyboard.buttons["Done"].tapWhenReady() + XCTAssertTrue(keyboard.waitForNonExistence(timeout: UITestBudget.seconds(5))) + } + app.buttons["connect-button"].tapWhenReady() + XCTAssertTrue(app.buttons["thread-abcdef12"].appears(within: UITestBudget.seconds(20))) + + let draft = "Keep this unsent draft across sheet requests" + for attempt in 0..<3 { + let projectAction = app.buttons["New thread in empty-project"] + if !projectAction.isReadyToTap { app.swipeUp() } + projectAction.tapWhenReady() + let selection = app.buttons["new-thread-project"] + XCTAssertTrue(selection.waitForExistence(timeout: UITestBudget.seconds(10))) + XCTAssertTrue((selection.label + " " + (selection.value as? String ?? "")).contains("empty-project"), "Each project-specific request must select that project.") + let input = app.inputField("new-thread-input") + if attempt == 0 { input.typeTextWhenReady(draft, in: app) } + app.buttons["Cancel"].tapWhenReady() + XCTAssertTrue(input.waitForNonExistence(timeout: UITestBudget.seconds(10))) + + let globalAction = app.buttons["new-thread"] + if !globalAction.isReadyToTap { app.swipeDown() } + globalAction.tapWhenReady() + XCTAssertTrue(selection.waitForExistence(timeout: UITestBudget.seconds(10))) + XCTAssertTrue((selection.label + " " + (selection.value as? String ?? "")).contains(defaultProject), "A global request must use the first snapshot project, independently of the cancelled project-specific sheet.") + XCTAssertEqual(input.value as? String, draft, "A fresh context must preserve the model-owned unsent draft.") + attachScreenshot(of: app, named: "Fresh global project context \(attempt + 1)") + app.buttons["Cancel"].tapWhenReady() + XCTAssertTrue(input.waitForNonExistence(timeout: UITestBudget.seconds(10))) + } + forgetMac(app) + XCTAssertTrue(pair.waitForExistence(timeout: UITestBudget.seconds(10))) + } + func testPairApproveAndFollowUpThroughTheMac() throws { continueAfterFailure = false guard let file = Bundle(for: Self.self).url(forResource: "LocalPairing", withExtension: "json"), @@ -298,6 +351,12 @@ final class CompanionFlowTests: XCTestCase { let link = app.inputField("pairing-link-input") XCTAssertTrue(link.waitForExistence(timeout: UITestBudget.seconds(10))) link.typeTextWhenReady(fixture.link, in: app) + let keyboard = app.keyboards.firstMatch + if keyboard.exists { + keyboard.buttons["Done"].tapWhenReady() + XCTAssertTrue(keyboard.waitForNonExistence(timeout: UITestBudget.seconds(5)), "Done must dismiss the pairing-link keyboard before connecting.") + } + attachScreenshot(of: app, named: "Pairing form before Connect") app.buttons["connect-button"].tapWhenReady() let springboard = XCUIApplication(bundleIdentifier: "com.apple.springboard") @@ -321,14 +380,23 @@ final class CompanionFlowTests: XCTestCase { let newThreadInput = app.inputField("new-thread-input") XCTAssertTrue(newThreadInput.waitForExistence(timeout: UITestBudget.seconds(10))) newThreadInput.typeTextWhenReady("Start from phone", in: app) + let mac = try XCTUnwrap(fixture.macURL.flatMap(URL.init(string:))) + XCTAssertEqual(try postToLoopback(mac.appendingPathComponent("timeout-commands"), timeout: UITestBudget.seconds(10)), 204) app.buttons["new-thread-commands"].tapWhenReady() let commandSearch = app.textFields["Find a skill or command"] XCTAssertTrue(commandSearch.waitForExistence(timeout: UITestBudget.seconds(10))) commandSearch.typeTextWhenReady("mobile-check", in: app) + let retryCommands = app.buttons["retry-commands"] + XCTAssertTrue(retryCommands.waitForExistence(timeout: UITestBudget.seconds(10)), "A timed-out command GET must offer explicit recovery in the picker.") + XCTAssertEqual(commandSearch.value as? String, "mobile-check", "A failed read must preserve the command search.") + XCTAssertFalse(app.alerts["Connection issue"].exists, "The command error must not dismiss the draft's sheets.") + attachScreenshot(of: app, named: "Command timeout recovery") + retryCommands.tapWhenReady() let mobileSkill = app.staticTexts["$mobile-check"] XCTAssertTrue(mobileSkill.waitForExistence(timeout: UITestBudget.seconds(10)), "The Mac's project skill should appear for Codex.") mobileSkill.tapWhenReady() XCTAssertTrue(commandSearch.waitForNonExistence(timeout: UITestBudget.seconds(10)), "The skill sheet must finish dismissing before submitting.") + XCTAssertEqual(newThreadInput.value as? String, "Start from phone $mobile-check ", "Recovery must preserve the exact draft and insert the skill once.") attachScreenshot(of: app, named: "New thread on iPhone") app.buttons["create-thread"].tapWhenReady() XCTAssertTrue(app.staticTexts.containing(NSPredicate(format: "label CONTAINS %@", "Follow-up received: Start from phone $mobile-check")).firstMatch.appears(within: UITestBudget.seconds(20))) @@ -445,6 +513,11 @@ final class CompanionFlowTests: XCTestCase { token.typeText(secret) // A secure field reads back as one bullet per character; wait for the last keystroke. XCTAssertTrue(waitUntil(timeout: UITestBudget.seconds(10)) { (token.value as? String)?.count == secret.count }, "The secure field received every character.") + let keyboard = app.keyboards.firstMatch + if keyboard.waitForExistence(timeout: UITestBudget.seconds(2)) { + keyboard.buttons["Done"].tapWhenReady() + XCTAssertTrue(keyboard.waitForNonExistence(timeout: UITestBudget.seconds(5)), "Done must dismiss the secret-answer keyboard before sending.") + } attachScreenshot(of: app, named: "Question answered on iPhone") send.tapWhenReady() @@ -534,7 +607,7 @@ final class CompanionFlowTests: XCTestCase { } } - private struct Fixture: Decodable { let link: String; let reconnect: Bool?; let resumeURL: String?; let macURL: String? } + private struct Fixture: Decodable { let link: String; let reconnect: Bool?; let resumeURL: String?; let macURL: String?; let defaultProjectName: String? } } /// 127.0.0.1 with port 0, for `bind` to pick an ephemeral port. diff --git a/docs/reviews/2026-11-08-companion-recovery-pr.md b/docs/reviews/2026-11-08-companion-recovery-pr.md new file mode 100644 index 00000000..c3ddfdaa --- /dev/null +++ b/docs/reviews/2026-11-08-companion-recovery-pr.md @@ -0,0 +1,46 @@ +# COMP-CMD-2: Companion command recovery + +Locally qualified on 2026-11-08 from main `d7650e4c54cdd15e4ee4f14b9f4dec3455ba1e77` in the owned `companion-recovery-pr` worktree. A timed-out command-list request now keeps its error inside the skills picker, preserves search and the exact new-thread draft, and offers an explicit Try again action. Pairing-link and secret-answer fields provide Done actions to dismiss their keyboards before explicit Connect or Send answer. [Interruption checkpoint](2026-11-08-companion-recovery-pr/checkpoint.md). + +## Scope and provenance + +The repair preserves project/provider scoping, cancellation and stale-response guards, revocation recovery, secret handling and explicit submission. The fixture stalls exactly one armed command GET while other traffic uses the real CompanionServer. The paired test checks inline failure, retained search, explicit retry and the exact `Start from phone $mobile-check ` draft before creating the thread. No existing assertion, request timeout or wait deadline was removed or increased; no unconditional retry was added. + +The original qualified `companion-skill-flow` worktree and COMP-CMD-1 manifest remain intact. Six verified iOS source/test/fixture files were ported after confirming main had not modified those inputs. Current-main docs were updated separately; older status/checkpoint files and already-merged CF-3 changes were not copied. External `port-provenance.json` records the source hashes. + +## PR #496 and project selection + +PR #496 head `59e54f6f9ed5cb609b72559eb2478f7e0bad4d5f` has an eight-file merge-base diff from `49ff1e1d214b50adff54799e148669a147f96f7e`, but its current-main tree difference contains only one request-ID identity line and two UI assertion lines across two iPhone files. Its project-selection diagnosis remains separate from the recorded command GET timeout, where the correct project was already selected. + +The project-specific action supplies its project ID. A global action supplies an empty ID; NewThreadSheet chooses a valid requested project or the first snapshot project. CompanionServer preserves project order. The fixture exposes that actual first project's name so the new test verifies the contract without forcing a project to expose a skill. + +The independent regression alternates project-specific empty-project and global requests three times, cancelling between them and retaining an exact model-owned unsent draft. Baseline `context-baseline-20261108T203304Z` passed one test with zero failures/skips without `.id(request.id)`. The same test passed in all three complete paired runs. The reset is therefore not included; the independent failure was not reproduced. This result does not establish that every possible sheet-identity failure is impossible. PR #496 was not modified or closed. + +## Local qualification + +Evidence root: `/Users/buns/Documents/Codex/2026-11-08/comp-cmd-2`. Complete local run: `local-20261108T204030Z`; qualification patch SHA-256 `f3270812abc83873c7b8223697413f1acecce914a2602e5bab8f1cbb2c33e5ad`. Its nine-file manifest matched before and after every phase. Final receipt documentation changed afterward; `delivery-candidate-manifest.json` binds the final files and proves all six qualified iOS inputs stayed unchanged. + +| Gate | Result | +| --- | --- | +| `pnpm install --frozen-lockfile --offline` | Passed | +| `pnpm build` | Passed | +| `pnpm typecheck` | Passed across workspaces | +| `pnpm run test:coverage` | 2,327 passed, 26 skipped, zero failures; Cobertura retained | +| Native iPhone unit suite | 63 passed, zero failures/skips | +| Complete paired run 1 | 44 passed, zero failures/skips | +| Complete paired run 2 | 44 passed, zero failures/skips | +| Complete paired run 3 | 44 passed, zero failures/skips | +| `env CI=1 MODEX_TEST_OPEN_KNOWLEDGE=1 MODEX_SITE_TEST_PORT=55082 pnpm test:e2e` | 505 desktop passed, four live-provider tests skipped; seven site passed; zero failures | +| Exact candidate verification | Passed; runner exit zero | + +Native checks used dedicated iPhone Duo simulator `9C6CF8B1-AC2B-4C6B-9355-F89CA60DF4C5`, iOS 27.2, Xcode 27.2, `MODEX_UI_TIMEOUT_SCALE=4` and separate derived-data/result bundles. XCTest metrics and both named scenario summaries were extracted from each finalized bundle; counts were not inferred from process exit alone. All three runs were consecutive on identical source. The dedicated simulator was shut down before desktop E2E; a second EXIT cleanup logged that it was already shut down. + +First-run screenshots were inspected for inline timeout recovery, the retained skill draft, global project selection and the secret-answer screen with its keyboard dismissed and Send answer visible. Secret content was not visible. Exact trailing-space preservation and secure-field completeness are asserted by XCTest rather than inferred from pixels. `visual-inspection.json` and hashed attachments retain these observations. + +The earlier unexplained three-pixel Code Review scroll failure and unchanged focused/full confirmations remain historical COMP-CMD-1 evidence. The unchanged scroll test passed at both 1440×900 and 900×600 in this complete run. No Code Review repair or causal explanation is claimed. + +## Delivery and acceptance limits + +The user authorized signed commits, push of only the owned branch and exactly one draft PR after local qualification. GitHub authentication/push permission, registered SSH signing key and an actual git-namespace sign/verify preflight passed. The exact signed commit/tree, draft PR and hosted macOS/iPhone checks are recorded in external `delivery.json`, `hosted-latest.json` and the draft PR description; read those receipts for publication state. No merge, tag, release or TestFlight upload is part of this goal. + +For #276, the automated model/paired receipts support question, draft, secret and focus behavior; they do not complete Val's D/P rows or deferred VoiceOver rows. For #281, three complete paired simulator runs strengthen the automation receipt but do not supply human, physical-device, live-provider or release acceptance. Issue checklist counts and human rows remain unchanged. Four desktop live-provider tests and 26 coverage tests remain explicitly skipped. diff --git a/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md b/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md new file mode 100644 index 00000000..c0ba5dd2 --- /dev/null +++ b/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md @@ -0,0 +1,21 @@ +# COMP-CMD-2 checkpoint + +Local qualification is complete. Worktree `/Users/buns/Documents/GitHub/TypeSafeAI/modex/.worktrees/companion-recovery-pr`, branch `companion-recovery-pr`, owner `01a1b0d4-78f4-7a70-beb9-2edf6fecc773`, base `d7650e4c54cdd15e4ee4f14b9f4dec3455ba1e77`. Preserve the original dirty `companion-skill-flow` worktree and all other owners. Evidence root: `/Users/buns/Documents/Codex/2026-11-08/comp-cmd-2`. + +Read the objective at `/Users/buns/.codex/attachments/84ef11e5-a5ac-447d-a794-8d6a80e77d94/pasted-text-1.txt`, AGENTS.md, current status and the [review](../2026-11-08-companion-recovery-pr.md). The user authorizes signed commits, push of only this branch and one draft PR after local qualification. No changes to #496, merge, tag, release or TestFlight upload are authorized. + +Authoritative mutable handoff: external `current-state.json`, `delivery.json`, `hosted-latest.json` and `final-checkpoint.md`. Check those before acting: the signed head, PR number and hosted jobs cannot be embedded in their own commit. If a receipt is absent, that delivery gate has not been recorded. Revalidate actual branch ownership, HEAD/diff and current remote heads; never restart a live job because an observation timed out or create a duplicate PR. + +Main and PR #496 were refreshed through REST. #496 head `59e54f6f9ed5cb609b72559eb2478f7e0bad4d5f` differs from current main only in two iPhone files, despite its eight-file merge-base diff. Six verified iOS files were ported; older docs and already-merged CF-3 changes were not copied. See `port-provenance.json` and `pr496-comparison.json`. + +The separate context probe `context-baseline-20261108T203304Z` passed one test, zero failures/skips, all three requested/global context cycles and exact draft assertions, with unchanged source. Its process is terminal. `.id(request.id)` remains absent because no independent failure was reproduced. The visible-contract test passed again in all three complete paired runs. + +Local runner `local-20261108T204030Z`, PID 99095 / exec session 3779, is terminal with exit zero. Do not wait on or restart those handles. Build and typecheck passed; coverage recorded 2,327 passed / 26 skipped. Native units recorded 63 passed / zero skipped. Three consecutive complete paired bundles each recorded 44 passed / zero skipped, including the recovery and fresh-context tests. Full desktop/site E2E recorded 505 desktop passed / four live-provider skips and seven site passed. Exact candidate verification passed. Read `local-qualification.json`, commands/logs, finalized xcresult bundles, exported metrics/screenshots and `visual-inspection.json` for receipts. + +Qualification patch SHA-256 is `f3270812abc83873c7b8223697413f1acecce914a2602e5bab8f1cbb2c33e5ad`. All nine candidate files matched the qualification manifest throughout the run. Only final receipt docs changed afterward; `delivery-candidate-manifest.json` records the final files and unchanged six-file iOS source identity. A future source change invalidates affected qualification evidence. + +GitHub account BunsDev has push permission. The agent-backed key is registered with GitHub and an actual git-namespace signature verified. Global user.signingkey is unset; use per-command `git -c gpg.format=ssh -c user.signingkey=/Users/buns/.ssh/id_ed25519.pub commit -S`, then verify the actual commit locally and through GitHub. Do not change global config, expand credential scopes or commit unsigned. The private signing-key management read lacked scope; the public key read verified registration without expanding scopes. + +Dedicated simulator `9C6CF8B1-AC2B-4C6B-9355-F89CA60DF4C5` was shut down successfully before desktop E2E. Preserve the earlier COMP-CMD-1 simulator and other owners' devices. The final EXIT shutdown logged already Shutdown; this was a harmless duplicate cleanup, not a native test failure. + +Continue the smallest unmet delivery gate: verify final source identity and Markdown payloads, signed commit/push, exactly one draft PR, then hosted `unit + e2e (macOS)` and `iPhone companion` checks bound to the tested head. Preserve any hosted first failure before changes or reruns. Record final commit/tree, PR URL, check IDs/conclusions and next action externally. Keep implementation, local qualification and hosted delivery separate. #276/#281 human, physical-device and live-provider rows remain open; earlier unexplained Code Review scroll evidence is retained and no repair is claimed. diff --git a/docs/status.md b/docs/status.md index 74acfd69..6c9c67c3 100644 --- a/docs/status.md +++ b/docs/status.md @@ -127,6 +127,7 @@ before relying on a tick. ## In flight +- **Companion command recovery (COMP-CMD-2).** Current-main candidate `companion-recovery-pr` keeps timed-out skill reads inside the picker with explicit retry, retained search/drafts and keyboard Done actions. Build/typecheck and coverage passed (2,327 tests; 26 skips); native units passed 63/63, three consecutive complete paired runs passed 44/44 each, and full E2E passed 505 desktop plus seven site tests (four live-provider skips). The independent context regression passed without PR #496's reset, which is not included. Signed draft delivery and exact-head hosted receipts are recorded externally and in the PR; human and physical-device acceptance on #276/#281 remains open. [Review and checkpoint](reviews/2026-11-08-companion-recovery-pr.md). - **v0.4.2 release.** #492 (`4297235`) prepared it with Code Review hidden by default. It is not tagged or published. [Review](reviews/2026-11-03-v0.4.2-release-review.md). - **Code Review acceptance** (#319). Every slice's code is on `main` (design and history: [design page](design/code-review.md), [CR-8 matrix](reviews/2026-11-01-cr8-acceptance-matrix.md)). Built apps hide it by default (#492); Settings → General can show it. #490 added that setting, C76 accessibility coverage, performance budgets as gates and the CR-7 sign-out and CF-3 question-card regressions; it also carries #481, #484, #485 and #491. No live provider run, live GitHub write, VoiceOver pass or reference comparison has happened. Those, plus decisions 1, 5, 6 and 7 and the C58/C59 omissions, are listed in [Val's one session](reviews/2026-11-02-v0.5.0-val-session.md). - **Conversation fidelity** ([tracker #273](https://github.com/TypeSafeAI/modex/issues/273)). CF-3, CF-7 and CF-8 stay open; their code is on `main`. The #489 question-focus repair and Approve/Deny/Stop/withdrawal draft coverage are integrated by #495. The human gates are under [Open acceptance gates](#open-acceptance-gates). From d3d2eee085ef101e9e044bb7502444518a5d982d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Val=20Alexander=20=F0=9F=91=91?= Date: Sun, 8 Nov 2026 15:47:15 -0600 Subject: [PATCH 2/2] docs: distinguish expected UI self-test failure from ordinary passes --- docs/reviews/2026-11-08-companion-recovery-pr.md | 8 ++++---- .../2026-11-08-companion-recovery-pr/checkpoint.md | 2 +- docs/status.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/reviews/2026-11-08-companion-recovery-pr.md b/docs/reviews/2026-11-08-companion-recovery-pr.md index c3ddfdaa..cfe19d01 100644 --- a/docs/reviews/2026-11-08-companion-recovery-pr.md +++ b/docs/reviews/2026-11-08-companion-recovery-pr.md @@ -27,13 +27,13 @@ Evidence root: `/Users/buns/Documents/Codex/2026-11-08/comp-cmd-2`. Complete loc | `pnpm typecheck` | Passed across workspaces | | `pnpm run test:coverage` | 2,327 passed, 26 skipped, zero failures; Cobertura retained | | Native iPhone unit suite | 63 passed, zero failures/skips | -| Complete paired run 1 | 44 passed, zero failures/skips | -| Complete paired run 2 | 44 passed, zero failures/skips | -| Complete paired run 3 | 44 passed, zero failures/skips | +| Complete paired run 1 | 44 executed: 43 passed, one intentional expected-failure self-test, zero unexpected failures/skips | +| Complete paired run 2 | 44 executed: 43 passed, one intentional expected-failure self-test, zero unexpected failures/skips | +| Complete paired run 3 | 44 executed: 43 passed, one intentional expected-failure self-test, zero unexpected failures/skips | | `env CI=1 MODEX_TEST_OPEN_KNOWLEDGE=1 MODEX_SITE_TEST_PORT=55082 pnpm test:e2e` | 505 desktop passed, four live-provider tests skipped; seven site passed; zero failures | | Exact candidate verification | Passed; runner exit zero | -Native checks used dedicated iPhone Duo simulator `9C6CF8B1-AC2B-4C6B-9355-F89CA60DF4C5`, iOS 27.2, Xcode 27.2, `MODEX_UI_TIMEOUT_SCALE=4` and separate derived-data/result bundles. XCTest metrics and both named scenario summaries were extracted from each finalized bundle; counts were not inferred from process exit alone. All three runs were consecutive on identical source. The dedicated simulator was shut down before desktop E2E; a second EXIT cleanup logged that it was already shut down. +Native checks used dedicated iPhone Duo simulator `9C6CF8B1-AC2B-4C6B-9355-F89CA60DF4C5`, iOS 27.2, Xcode 27.2, `MODEX_UI_TIMEOUT_SCALE=4` and separate derived-data/result bundles. XCTest metrics and both named scenario summaries were extracted from each finalized bundle; counts were not inferred from process exit alone. All three runs were consecutive on identical source. Modern summaries and per-test statuses distinguish 43 ordinary passes from the existing intentional `DemoWorkspaceTests/testBestEffortEvidenceDoesNotStopTheTest` expected-failure self-test in each 44-execution suite. Both recovery and fresh-context tests have status Success. Hosted iPhone artifact inspection confirmed the same classification; the final documentation correction does not change iOS inputs. The dedicated simulator was shut down before desktop E2E; a second EXIT cleanup logged that it was already shut down. First-run screenshots were inspected for inline timeout recovery, the retained skill draft, global project selection and the secret-answer screen with its keyboard dismissed and Send answer visible. Secret content was not visible. Exact trailing-space preservation and secure-field completeness are asserted by XCTest rather than inferred from pixels. `visual-inspection.json` and hashed attachments retain these observations. diff --git a/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md b/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md index c0ba5dd2..8ca7c95e 100644 --- a/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md +++ b/docs/reviews/2026-11-08-companion-recovery-pr/checkpoint.md @@ -10,7 +10,7 @@ Main and PR #496 were refreshed through REST. #496 head `59e54f6f9ed5cb609b72559 The separate context probe `context-baseline-20261108T203304Z` passed one test, zero failures/skips, all three requested/global context cycles and exact draft assertions, with unchanged source. Its process is terminal. `.id(request.id)` remains absent because no independent failure was reproduced. The visible-contract test passed again in all three complete paired runs. -Local runner `local-20261108T204030Z`, PID 99095 / exec session 3779, is terminal with exit zero. Do not wait on or restart those handles. Build and typecheck passed; coverage recorded 2,327 passed / 26 skipped. Native units recorded 63 passed / zero skipped. Three consecutive complete paired bundles each recorded 44 passed / zero skipped, including the recovery and fresh-context tests. Full desktop/site E2E recorded 505 desktop passed / four live-provider skips and seven site passed. Exact candidate verification passed. Read `local-qualification.json`, commands/logs, finalized xcresult bundles, exported metrics/screenshots and `visual-inspection.json` for receipts. +Local runner `local-20261108T204030Z`, PID 99095 / exec session 3779, is terminal with exit zero. Do not wait on or restart those handles. Build and typecheck passed; coverage recorded 2,327 passed / 26 skipped. Native units recorded 63 passed / zero skipped. Three consecutive complete paired bundles each recorded 44 executed: 43 passed, one intentional expected-failure self-test, zero unexpected failures/skips, including the recovery and fresh-context tests. Full desktop/site E2E recorded 505 desktop passed / four live-provider skips and seven site passed. Exact candidate verification passed. Read `local-qualification.json`, commands/logs, finalized xcresult bundles, exported metrics/screenshots and `visual-inspection.json` for receipts. Qualification patch SHA-256 is `f3270812abc83873c7b8223697413f1acecce914a2602e5bab8f1cbb2c33e5ad`. All nine candidate files matched the qualification manifest throughout the run. Only final receipt docs changed afterward; `delivery-candidate-manifest.json` records the final files and unchanged six-file iOS source identity. A future source change invalidates affected qualification evidence. diff --git a/docs/status.md b/docs/status.md index 6c9c67c3..0b776efb 100644 --- a/docs/status.md +++ b/docs/status.md @@ -127,7 +127,7 @@ before relying on a tick. ## In flight -- **Companion command recovery (COMP-CMD-2).** Current-main candidate `companion-recovery-pr` keeps timed-out skill reads inside the picker with explicit retry, retained search/drafts and keyboard Done actions. Build/typecheck and coverage passed (2,327 tests; 26 skips); native units passed 63/63, three consecutive complete paired runs passed 44/44 each, and full E2E passed 505 desktop plus seven site tests (four live-provider skips). The independent context regression passed without PR #496's reset, which is not included. Signed draft delivery and exact-head hosted receipts are recorded externally and in the PR; human and physical-device acceptance on #276/#281 remains open. [Review and checkpoint](reviews/2026-11-08-companion-recovery-pr.md). +- **Companion command recovery (COMP-CMD-2).** Current-main candidate `companion-recovery-pr` keeps timed-out skill reads inside the picker with explicit retry, retained search/drafts and keyboard Done actions. Build/typecheck and coverage passed (2,327 tests; 26 skips); native units passed 63/63, three consecutive complete paired suites passed with 44 executions each (43 ordinary passes and one intentional expected-failure self-test; zero unexpected failures/skips), and full E2E passed 505 desktop plus seven site tests (four live-provider skips). The independent context regression passed without PR #496's reset, which is not included. Signed draft delivery and exact-head hosted receipts are recorded externally and in the PR; human and physical-device acceptance on #276/#281 remains open. [Review and checkpoint](reviews/2026-11-08-companion-recovery-pr.md). - **v0.4.2 release.** #492 (`4297235`) prepared it with Code Review hidden by default. It is not tagged or published. [Review](reviews/2026-11-03-v0.4.2-release-review.md). - **Code Review acceptance** (#319). Every slice's code is on `main` (design and history: [design page](design/code-review.md), [CR-8 matrix](reviews/2026-11-01-cr8-acceptance-matrix.md)). Built apps hide it by default (#492); Settings → General can show it. #490 added that setting, C76 accessibility coverage, performance budgets as gates and the CR-7 sign-out and CF-3 question-card regressions; it also carries #481, #484, #485 and #491. No live provider run, live GitHub write, VoiceOver pass or reference comparison has happened. Those, plus decisions 1, 5, 6 and 7 and the C58/C59 omissions, are listed in [Val's one session](reviews/2026-11-02-v0.5.0-val-session.md). - **Conversation fidelity** ([tracker #273](https://github.com/TypeSafeAI/modex/issues/273)). CF-3, CF-7 and CF-8 stay open; their code is on `main`. The #489 question-focus repair and Approve/Deny/Stop/withdrawal draft coverage are integrated by #495. The human gates are under [Open acceptance gates](#open-acceptance-gates).