Repository navigation
Promote expbkmain to bkmain - #119
Merged
Merged
Conversation
…nt at Anchored plan feedback reached the agent as raw `<review_comment>` XML. Three links, each verified: `locateQuotedLineRange` compared the reviewer's quote — captured from rendered Plate output, so already stripped of markdown — against raw source with `String.includes`, so any bold or code-spanned line missed; `formatPlanReviewComment` then omitted the range; and the transcript parser required that range, rejected the block, and fell back to printing it verbatim. Comments on formatted lines are the common case, not an edge case. Fix all three. `planReviewAnchorText` projects both sides onto the text a reader would select before comparing, the parser treats the range as optional in web and mobile alike, and a plan comment now renders as its own card — plan title and a blockquote, not a workspace-relative path and a monospace code block. Separately, `CommentLeaf` existed but nothing ever applied a mark, so a comment was real on the server and invisible in the document. Selecting now highlights immediately, saving turns that into a persistent amber anchor, and reopening the panel restores highlights by re-locating the stored quote. Comment marks are proven not to reach the serialized markdown, so commenting never dirties the draft. Rounds out the review surface with the UX worth taking from Plannotator: an inline comment popover beside the selection, one-click 👍/remove labels, two-way rail-to-document linking, and a width-gated Contents outline carrying per-section comment counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…data
Two defects found by running this for real.
A failed publish left a draft holding the version's tag, and that wedged
the staging channel permanently. The build job resolves the next version
with `contents: read`, and GitHub hides draft releases from callers
without push access — so the resolver kept handing out the same version
while the publisher, which has write access and can see drafts, kept
rejecting it as already taken. Three consecutive staging builds failed
this way. The publisher now recognises a draft for the version it is
publishing as the wreckage of an earlier attempt, deletes it and
continues; a real published release still hard-fails as before.
Release notes published the raw designated requirement, which for an
Apple-issued certificate embeds the developer's email and Team ID — on a
public repository, on every release. They now carry a 16-hex fingerprint
of that requirement instead. It answers the operational question ("did
the signing identity change?") without the personal data, and normalises
away the `Executable=` suffix codesign appends, which is the build
machine's temp path and leaked a runner directory into every release.
Also corrects the notes calling these builds "self-signed" — the current
trial certificate is Apple Development, which the docs already record.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(planreview): anchor comments reliably and highlight what they point at
…on saving Three follow-ups from reviewing the first release on expbkt3. Commenting re-sent the entire plan as a reviewer edit. Plate serializes a commented leaf as an MDX JSX element, and `mdast-util-to-markdown` throws on it inside a table cell — so on any plan containing a table, serialization failed, the editor's change handler read "cannot serialize" as a reviewer edit, and the panel shipped the whole Plate-normalized document under `<plan_edit mode="full">`. Nobody had edited anything. `stripPlanReviewCommentMarks` now removes comment properties and re-merges the leaves `split: true` fragmented, so the plan is always serialized as if it carried no comments. A test pins the underlying serializer failure too, so the strip cannot be quietly dropped later. Deciding on a plan with unsaved hand edits was ambiguous — neither side could say afterwards which text was approved. Unsaved edits now replace the decision row with a single **Save the plan**. Approving with open comments told the agent to implement "exactly as written" and then handed it a list of changes. It now says to start implementing and apply the comments as amendments, without returning to planning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On a shared dev machine an agent had no reliable way to tell who it was working for, so it fell back to whatever the machine said — git config, a checked-in dotfile — and attributed one contributor's session to another. Provider sessions now spawn with three additive variables read from the durable environment-user directory: BK_IDENTITY_RUNTIME (always t3-code), BK_SESSION_OWNER_EMAIL for the thread owner, and BK_MESSAGE_SENDER_EMAIL for the user who actually sent the message being answered — never the owner by fallback, because an inferred sender is the misattribution this replaces. A missing or unreadable user record leaves the runtime marker alone and the turn proceeds, so "unknown" is expressible and nothing blocks on identity. Because a process reads its environment once at spawn, ProviderCommandReactor fingerprints the identity each live session was started with and restarts on an owner transfer or a new sender, beside the existing credential-actor restart. The markers compose with source-control profiles instead of replacing them: mergeSourceControlEnvironment now scrubs inherited Git and GitHub credentials only when the overlay carries a source-control identity of its own, so machine-identity mode keeps its own GH_TOKEN and still carries the markers. TEC-964 Model: Claude Opus 5 in T3 Code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The OpenCode adapter refuses to start against an external server whenever execution options carry an environment, because it cannot inject one there. Now that every session carries identity markers in that same field, the guard would have failed every start against an external server. Test the guard on what it actually protects: a source-control identity in the overlay. The markers claim none, so they are simply not delivered to an external server instead of failing the turn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ctor Proves the wiring, not just the resolver: owner and sender reach the provider execution options, an anonymous turn reports the runtime marker alone, the markers compose with a thread source-control profile, and an owner transfer restarts the session while the sender stays the same. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tity feat(server): agents can name the session owner and message sender
fix(planreview): stop comments dirtying the plan, and gate decisions on saving
…oyed Merging a green PR to expbkmain no longer needs an ask. The rule that matters is the second half: a merge is not a delivery. Nothing is verifiable until the timer has installed the artifact and expbkt3 is serving it, and a build or install can still fail after a green PR — so follow it through and report once, when the running service is on the merge SHA. bkmain is unchanged and still needs a human, because deploying it kills the team's live sessions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs(agents): pre-authorise expbkmain merges, and own them until deployed
…aling scroll Writing a comment from the bottom of a long plan threw the reviewer back to the top. `useSelectionAnchor` returned before computing anything when `frozen` was set, so the composer's anchor was always null and it fell back to absolute positioning *inside* the scroll container — then autofocused itself, scrolling the plan to reach it. The frozen branch was mine and never worked; it is gone, and the option with it. The composer now docks below the document as a sibling of the scroll container, so taking focus cannot move the plan, with `preventScroll` as a second guard. It is also the size a review comment deserves: five lines to start, growing with what is typed up to 30vh so it can never swallow the plan it is about. Separately, "Approve with comments" was being cut in half — two buttons sharing a row in a 288px rail. They stack now, so no label truncates at any panel width. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(planreview): dock the comment box under the plan, and stop it stealing scroll
github-actions
Bot
added
size:XXL
vouch:trusted
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
A merge to bkmain built the production app and then waited for a human to approve the publish, so the team's download lagged the deploy by however long that took. The required-reviewer rule is removed and both channels now publish the moment their build is green. That rule lived in the `bk-desktop-production` environment's settings, not in this workflow — protection rules are repository settings and no workflow file can express their absence. The comment says so, so the next reader looking for a gate that is not in the YAML knows where it went and how to put it back. Release notes now open with the commits since the previous build of the same channel: subject lines only, merge commits dropped so each change is listed once rather than twice, duplicates collapsed, and a tail summary past fifteen. Resolved through the compare API because the publish job checks out at depth 1 and has no history to walk, and never fatal — a release without its change list is cosmetic, a release that does not ship is an outage of the update channel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat(desktop): publish the production app on merge, and say what changed
…nnel The change list shipped empty. The previous-build lookup matched on the brand's `updateChannel`, which is already `staging-nightly`, so it searched tags for `-staging-nightly-nightly.`, found nothing, and returned no commits — and because the change list is deliberately never fatal, the release published quietly without it. Confirmed on v0.0.34-staging-nightly.20260819.1, built from the commit that introduced it. Key on the channel instead, the same predicate the workflow's already-published check uses, and move the lookup into a tested function. The first version tested the summariser and left the lookup inline and uncovered, which is exactly where the bug was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restarting the server for a deploy killed every running agent session for good. Provider processes are children of the server, so they die with it, but nothing re-derived their state at boot: the projection kept `status: running` and a non-null `activeTurnId`, so the thread showed "Working" forever. Recovery was lazy — it only ran when the next operation was routed to the thread — and the state was self-reinforcing: the reaper skips threads with an active turn, settle is rejected while running, and Stop respawned the agent only to silently no-op on a session with no turn. Even a clean shutdown left this behind, because the reactors are torn down before the adapters emit their exit events. A boot-time sweep now marks those sessions interrupted, clears the stuck turn, releases approval and user-input requests whose callbacks died with the process, and restarts the interrupted turn with a prompt that restates the original request and tells the agent to check for partially completed work before redoing it. The agent resumes from its persisted resume cursor, so a deploy no longer costs the user a session. Also: Stop settles the thread instead of respawning a dead session, session start is bounded so one wedged child cannot hang the shared command worker, bindings are reconciled at boot to survive SIGKILL, and shutdown stops adapters concurrently so it fits the launcher's 5s kill deadline. The sweep lives in its own file and is wired in with three lines, to keep upstream merges cheap. Auto-restart is one exported constant away from off. Model: Claude Opus 5 (1M context). Harness: Claude Code in T3 Code.
A deploy can leave the client holding a socket that never delivers a close event — common with a proxy or tunnel in the path. The supervisor only leaves the connected phase on a close or an explicit signal and has no timer while connected, so it believed it was connected indefinitely and the only way back was the Reconnect button. A durable subscription that lost its transport made this worse: it logged, drained, and waited for a session that was never coming. The connected phase now probes an idle connection every 20 seconds, reusing the same guarded probe a foreground wake already runs, so a silently dead transport is detected in about 30 seconds and reconnects on the normal backoff ladder. A healthy connection is untouched. Subscriptions that lose their transport report the session instead of waiting on it; the report is gated on being the live session, which collapses duplicates and drops stale ones. Heartbeat failures deliberately do not skip the first backoff rung — nobody is waiting on the app, and that is what keeps a flapping server from becoming a reconnect storm. Model: Claude Opus 5 (1M context). Harness: Claude Code in T3 Code. chore(client): mark the fork's reconnect edits with T3-CUSTOM markers
fix: recover agent sessions and reconnect clients after a server restart
fix(desktop): key the change list on the channel, not the updater channel
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Routine promotion of the staging line to production. 13 commits, all already running on expbkt3.
What this carries
Native plan review (mine, this session's work — verified on expbkt3):
<review_comment>XML. The anchor locator compared the reviewer's quote against raw markdown, so any bold or code-spanned line failed to locate and the transcript parser rejected the block.mdast-util-to-markdownthrows on inside a table cell; the editor read that failure as an edit.Not mine, already on expbkmain:
4c63e043/e077acab/004fc274— session identity through the provider command reactor (TEC-964)16103573— desktop publish fixVerification
Every commit here has been through the
expbkmainvalidate workflow and has been running on expbkt3.dev.beknown.live. The most recent,d1690c5e, has been live since 15:45 UTC yesterday.Merging this restarts
t3-bkmain.serviceand interrupts in-flight sessions on bkt3.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.