Skip to content

docs(ship): propose record 0074, a side effect crosses one typed seam - #2194

Merged
justinhelmer merged 3 commits into
mainfrom
plan/write-decision-record-00-60beb0/u1
Sep 22, 2026
Merged

justinhelmer merged 3 commits into
mainfrom
plan/write-decision-record-00-60beb0/u1

Conversation

@justinhelmer

Copy link
Copy Markdown
Contributor

Record 0074, status proposed: every effect a run performs beyond edits in its checkout — a push, a rebase and its conflict resolution, a pull-request open or update, a comment, and later connector writes such as a Linear issue update (#1625) — crosses one typed seam owned by the runner, never the child's shell. The child asks through typed tools; the runner validates current facts, performs the effect with the gates that belong to it, and returns a typed receipt or refusal.

Why. PR #2164 spent ten review rounds and thirteen force-pushes on 2026-09-21 because a judge over the text of git push can always be bypassed one more way (#2173); the pre-push rebase is a prompt paragraph while the sweep holds the real resolver (#2153); tonight's Worker/container skew (#2187) is the same class of untyped side effect on the deploy side. Direction of records 0071 and 0073.

Where to look. 1. docs/decisions/0074-a-side-effect-crosses-one-typed-seam.md — the whole record (TL;DR, today at a sha, the shape, the hard-case trace, the difficulty map, the hard parts, plan sketch U1–U4). 2. docs/explanation/design-decisions.md — the index row.

Feedback wanted. Whether the seam's first unit (push performed by the runner after the changed-set gates on the exact tree) is the right first cut, and whether connector writes belong to the same seam or a sibling record.

Risk. Docs only; acceptance is a person's later step.

Verified. docs:check, hygiene:check, specs:check in the coding run; a cold-reader gate is posted as a PR comment before review.

Tracking: #2173, #2174, #2182, #2185, #2187, #1625. Opened by the operator from the pushed head after the ship runner aborted the unit under the skew (#2187).

🤖 Generated with Claude Code

@justinhelmer

Copy link
Copy Markdown
Contributor Author

Cold-reader gate (record 0074 at 3cc8a06) — PASS. A fresh reader given only the record text, 2026-09-22 07:0xZ:

  1. The bet, restated. Every externally visible write a run makes — push, rebase, pull-request open/update, comment, later connector writes — becomes a narrow typed command the child sends to the runner, and the child's shell is stripped of any credential or route to perform those writes itself. The runner owns the whole operation from fresh facts to outcome: resolve, authorize and fence on an expected sha/version, run the operation's gates on the exact object, perform one write, return a typed receipt or a closed-reason refusal — so safety stops depending on a text judge recognizing every shell spelling.
  2. The hardest part. Actually removing write authority from the child's shell across resident, cold and salvage execution while the child still needs read-only fetches (a fetch-capable git credential is normally also push-capable) and while the runner must reach the child's exact checkout to run gates on the tree it publishes. Convinced by difficulty-map item 1 and the opening of "The hard parts" ("The strongest type does nothing if bash can still perform the same write").
  3. First objection and whether it is answered. "This is a convention with better types unless the shell physically lacks the token; where does the child's read-only credential come from, and how does the runner's adapter spend a different one in the same sandbox?" Answered in direction and placement — "The credential boundary is the architecture … The child's executor needs ordinary filesystem and read-only repository access; the effect implementation needs short-lived, resource-scoped write authority. The runner is where those domains meet. This extends record 0009's credential separation" — and assigned to unit one ("Remove publish authority from the child shell"); the concrete read-only-fetch mechanism is named as work, not designed.

Verdict: PASS — the bet, the hardest part and the credential objection are all locatable in the document alone, the objection answered in principle and explicitly placed in unit one.

@coreplane-switchboard coreplane-switchboard Bot 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 requested: The decision record leaves retry identity undefined and contradicts itself about when its cold-reader gate runs.

Warning

Changes requested · head 3cc8a06 · 2 findings: 2 minor

Severity Finding Where
minor F1 Effect commands lack the stable ID required for safe retries docs/decisions/0074-a-side-effect-crosses-one-typed-seam.md:44
minor F2 Cold-reader gate timing is internally contradictory docs/decisions/0074-a-side-effect-crosses-one-typed-seam.md:185
Full review

F1 — Minor, high confidence. The command union does not carry the effectId that line 80 relies on for idempotency. If a comment succeeds remotely but its response is lost, a resumed caller cannot identify the original attempt and may post twice. Define a stable effect ID in the shared command envelope and persist it before dispatch.

F2 — Minor, high confidence. Line 185 requires the cold-reader gate “Before review,” while line 187 says it becomes runnable only after this proposal merges. Those rules cannot both hold for this review. Change the first requirement to “Before acceptance,” or run and record the gate before review.

justinhelmer and others added 2 commits September 22, 2026 07:12
Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>
Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>

@coreplane-switchboard coreplane-switchboard Bot 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 requested: The typed effect ID still cannot make comment retries safely recoverable after an ambiguous remote success.

Warning

Changes requested · head e754d82 · 1 finding: 1 minor

Severity Finding Where
minor F1 Comment effects cannot reconcile an ambiguous successful post docs/decisions/0074-a-side-effect-crosses-one-typed-seam.md:116
Full review

F1 — Minor, high confidence. If GitHub accepts a comment but the response is lost, the runner has neither the immutable comment ID nor any remote marker correlating the comment with effectId. Reading the subject cannot distinguish that comment from an identical pre-existing one, so the runner must either risk posting twice or leave a successful effect without its promised receipt. Define a remote idempotency marker or another deterministic reconciliation key for comments.

Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>
@justinhelmer

Copy link
Copy Markdown
Contributor Author

Re-review requested at c9fd19d — F1 (a comment effect could not reconcile an ambiguous success) addressed in the record: every comment and open_pr body carries an invisible <!-- switchboard:effect <effectId> --> marker, an ambiguous success lists the subject's comments (or the branch's pull requests) and matches the marker before posting, and the invariant is stated: published text carries its own id; a published git object is reconciled by ref and sha. Difficulty map gained row 5 (reconciliation keys for published text). Status stays proposed.

@coreplane-switchboard coreplane-switchboard Bot 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.

LGTM: The effect-marker reconciliation closes the prior ambiguous-success gap, with no remaining review findings.

Note

Approved · head c9fd19d · no findings

Full review

The effect marker and bounded reconciliation flow resolve the earlier ambiguous-success duplication risk.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

@justinhelmer
justinhelmer merged commit 57a0e07 into main Sep 22, 2026
30 checks passed
@justinhelmer
justinhelmer deleted the plan/write-decision-record-00-60beb0/u1 branch September 22, 2026 07:59
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