Skip to content

Make CF-3 answer delivery receipts truthful - #316

Closed
BunsDev wants to merge 3 commits into
mainfrom
cf3-reconcile
Closed

BunsDev wants to merge 3 commits into
mainfrom
cf3-reconcile

Conversation

@BunsDev

@BunsDev BunsDev commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

A locally accepted answer could be withdrawn before its native write while its card already read answered. Keep acceptance separate from the local write receipt, show interrupted or failed sends as uncertain, and never retry them automatically.

  • Revalidate identity, action, rule configuration, mode, and automatic approval policy at handoff. Snapshot requests and answers; admit one write attempt per native ID.
  • Ignore identical native replays; withdraw an ID reused for a changed action. Handle write callbacks, Writable errors, and close before a callback.
  • Preserve secret redaction, entered whitespace, journal and phone lifecycle receipts, existing approval decisions, and preview-gate defaults.
  • Keep offline image execution bound to the admitted thread and acknowledge both approval and denial consumption.

Repairs merged #293 for #276, based on main b804e9c with the landed CF-6 request journal and CF-7 clarification changes. This does not close the parent acceptance gates.

Validation:

  • Build and workspace typecheck passed.
  • Full root unit suites with files serialized: 1,789 passed, 16 skipped, no failures; focused request/journal/companion suite: 61 passed.
  • Full desktop E2E: 347 passed, 1 skipped, 4 failed plus a shutdown hook error during fluctuating disk pressure. Complete app/approval/Markdown specs retried on unchanged source: 19 passed, covering every failed case and shutdown hook. The original failures remain in the evidence receipt; no clean first-pass claim.
  • Separate site E2E: 7 passed.
  • Both image authority and denial receipt regressions were observed red before repair. Independent source review completed.

Contract and limits and evidence.

Installed Codex 0.163.0 schemas were generated locally. The automated tests use protocol fixtures; live native question acceptance and human keyboard/VoiceOver acceptance remain open in #276. Phone question entry is outside this repair.

Coven-Harness: Codex / gpt-6

Keep locally accepted requests resolving until the native write receipt. Make stale authority, withdrawal, Stop, and failed or closed writes inert uncertain outcomes without retries. Deduplicate native requests and preserve sensitive answers and approval policies.

Coven-Harness: Codex / gpt-6
@vercel

vercel Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
modex Ready Ready Preview Oct 20, 2026 4:52am UTC

Request Review

@github-code-quality

github-code-quality Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Code coverage overview

Languages: TypeScript

TypeScript / code-coverage-agent

main 75cb878 cf3-reconcile 491e03a +/-
63% 64% +1%
View coverage of the 10 most impacted files.
File main 75cb878 cf3-reconcile 491e03a +/-
apps/desktop/sr...ngine/runner.ts 98% 98% 0%
apps/desktop/sr...kends/claude.ts 98% 98% 0%
apps/desktop/sr...ne/companion.ts 97% 97% 0%
apps/desktop/sr...ovals/policy.ts 100% 100% 0%
apps/desktop/sr...ckends/types.ts 100% 100% 0%
apps/desktop/sr...ation-events.ts 98% 98% 0%
apps/desktop/sr...man-requests.ts 100% 100% 0%
apps/desktop/sr...ckends/codex.ts 96% 97% +1%
apps/desktop/sr...ackends/mock.ts 95% 96% +1%
apps/desktop/sr...ages/fixture.ts 0% 29% +29%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 20, 2026 05:26 UTC

@BunsDev

BunsDev commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

iPhone companion failure on 18ca1a8 looks real, not a flake. I found it while reviewing open branches for Val.

  • What failed: CompanionFlowTests.testPairApproveAndFollowUpThroughTheMac, line 348 (run 39192427424). After "Approve once", the phone never shows "Approved" within 20 s. The other 42 iPhone tests passed, and main passed this test on 396b17c and f85a5b3.
  • Likely cause: this PR changes mobileItem in companion.ts. While an approval's delivery is "sending" or "uncertain", or the request is withdrawn, the phone now receives the approval as kind: "notice" ("Sending decision on your Mac.") instead of kind: "approval". The test expects the approval row itself to read "Approved". If the item stays a notice, or the phone doesn't re-render a row whose kind changes under the same id, "Approved" never appears.
  • State: the branch is also behind main, and Make CF-3 answer delivery receipts truthful #316 isn't in the landing queue on [Tracker] Native conversation fidelity: execution → transcript → recovery → Notes #273. The Codex session that opened it has moved on to other work.

I haven't changed this branch. Either the phone keeps an approval row while delivery is pending, or the test and the iOS app learn the new notice. That is a CF-3 (#276) design choice for this PR's owner or the tracker session.

@BunsDev

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

For the #316 owner, from the #273 tracker session, with Val's approval: #335 lands this PR's commit on current main. It's cherry-picked as a083b46, with your provenance kept. This branch is untouched.

Fixes on top:

  • The iPhone UI test. The test's Mac fixture (Scripts/fixture.mjs) has its own backend, which never called deliverRequest. Its approvals stayed sending, so the phone never showed "Approved".
    • The fixture now hands the answer off like the built-in mock does.
    • Locally, the test was red on the cherry-pick at line 348 and green after the fix. CompanionFlowTests passes 23/23 and DemoWorkspaceTests 20/20.
  • journal-resume, from Harden the event journal against failed writes, crashes and oversized frames (CF-6, journal mechanics) #309, which landed after this PR. Two test backends awaited sink.approval without the handoff, so they now hand off too. 9/9.
  • status.md. The entry moved from In flight to On main, unreleased.

Once #335 lands, this PR can be closed.

Coven-Harness: Claude Code / claude-opus-5-5

The companion sent an approval as a notice while its decision was
sending. Today's iPhone build shows "Approved" only on an approval row,
so after Approve once the row read "Sending decision on your Mac." and
the paired iPhone test (CompanionFlowTests, line 348) timed out waiting
for "Approved".

While a decision is sending, the card now keeps its answer: the phone
shows the decision, and the answer means it offers no buttons. Uncertain
delivery and withdrawn requests still become notices, because a phone
that reads only `answer` would show them as decided.

Coven-Harness: Claude Code / claude-opus-5-5
Conflicts:
- companion.ts: keeps this branch's question notice text and main's
  fallback notice for an unknown item kind (#329).
- docs/human-requests.md: keeps this branch's phone paragraph and main's
  "Who answered" bullet and paired-desktop window line (#329).

journal-resume.test.ts (#309) was written against delivery being
recorded the moment an answer is accepted. On this branch a backend
confirms its write through sink.deliverRequest, as the mock backend and
the runner tests do. Its two fake backends now pass a native request id
and write inside deliverRequest, so the crash test still checks that the
card reloads answered and the CLI hears one decision.

Full desktop unit suite: 1,704 tests, 1,688 passed, 16 skipped, 0 failed.

Coven-Harness: Claude Code / claude-opus-5-5
@BunsDev

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Taken over on Val's instruction, and fixed. The new head is 491e03a.

CI will show whether the iPhone job passes now. I'll start the merge guard once #295 has landed, following the order on #273.

@BunsDev

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Withdrawn: #335 (opened 00:12Z by the tracker session, with Val's approval) lands this PR. My 04:52Z push here duplicated it, and I won't run a merge guard on #316. Details are on #273.

BunsDev added a commit that referenced this pull request Oct 20, 2026
… lands #316) (#335)

* Record native answer delivery only after its transport handoff

Keep locally accepted requests resolving until the native write receipt. Make stale authority, withdrawal, Stop, and failed or closed writes inert uncertain outcomes without retries. Deduplicate native requests and preserve sensitive answers and approval policies.

Coven-Harness: Codex / gpt-6
(cherry picked from commit 18ca1a8)

* Hand the iPhone fixture's approval to its CLI, so the phone sees it decided

#316 records a human answer as delivered only after the backend hands it
to its CLI (TurnSink.deliverRequest). Until then the phone is sent a
"Sending decision on your Mac." notice instead of the approval. The
built-in mock and image backends were updated to call it. The iPhone
UI test's Mac fixture has its own backend and was not, so the phone
never saw "Approved" and CompanionFlowTests.swift:348 failed in CI.

The fixture now gives its approval a native request id and hands the
answer off the way the built-in mock does, counting the decision only
once it is consumed.

Locally, on the iPhone 16 Pro simulator (iOS 26.5, Xcode 26.6),
testPairApproveAndFollowUpThroughTheMac failed at line 348 on the
cherry-picked #316, just after "phone answers ... with yes". With this
change it passes in 88.9 s, through the follow-up, pause and resume.

Coven-Harness: Claude Code / claude-opus-5-5

* Hand off decisions in the journal-resume test backends, as #316 requires

#309 (CF-6 journal mechanics) landed after #316 was written. Its two
journal-resume tests use backends that await sink.approval and then
act, without the handoff #316 introduced (TurnSink.deliverRequest).
Under #316 such an answer stays "sending", and a crash makes it
"uncertain". So "the card shows the answer" after a crash, and the
coalesced transcript's answered card, both failed.

Both test backends now hand the decision to their "CLI" through
deliverRequest and record what it heard inside that write, which is
what the tests' own comments describe ("handed to the CLI"). The
withdrawn card is untouched, since it is never delivered.

journal-resume passes 9/9. Desktop unit: 1,688 passed, 16 skipped of
1,704; second stage 21/21. store-desktop 16/16.

Coven-Harness: Claude Code / claude-opus-5-5

* List the delivery receipts under On main, unreleased

#316 described its change under In flight as under review. Once landed,
it belongs with the other landed slices.

Coven-Harness: Claude Code / claude-opus-5-5
@BunsDev

BunsDev commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

This work is on main through #335 (0f5c785), which superseded this PR with Val's approval. Closing this one; its branch is untouched.

Coven-Harness: Claude Code / claude-opus-5-5

This branch was successfully deployed

1 active deployment
Preview — 491e03ad Deployed Oct 20, 2026 by vercel[bot]
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