Apply the visibility answer the Agents screen already recorded - #680
Open
George-Payne wants to merge 6 commits into
Open
Apply the visibility answer the Agents screen already recorded#680George-Payne wants to merge 6 commits into
George-Payne wants to merge 6 commits into
Conversation
The field was absent from the wire models, so the flow asked who may read future sessions and step 3 overwrote the answer seconds later. It rides the Agents decision, so it is read off the same answer and validated against the CLI closed set rather than forwarded: the value lands in profile config and is stamped on every session after.
Falling through to the prompt does not leave the profile alone: its cursor starts on org_public, so a Return on a re-run widens an existing private on a question already answered. Whether the step settled separates the two nulls, and DecideVisibility is extracted because step 3 interactive branches had no coverage at all.
PR Summary by QodoApply browser visibility choice during CLI setup
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Its claim is that the attempt gave up rather than waiting the 30s hold, but its bound was the hook 5s budget itself, so a loaded runner overshot it by 240ms with the spool correct. The two sibling tests for the same claim use 15s and 10s; this brings the outlier into line, still well under the hold.
George-Payne
requested review from
alexeyzimarev,
nortonandreev,
realtonyyoung and
rsergeenko
August 27, 2026 08:11
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.
AI-2282
What & why
The Agents step asks who may read the sessions this machine records from now on, records it on
FirstRunAgentsDecidedEvent, and serves it on the poll asdefault_visibility. No CLI read it: the field was absent fromCapacitor.Cli.Core/FirstRun's wire models, so it was dropped at deserialisation, andkcap setup's step 3 then prompted unconditionally and wrote its own answer over it — the one place in the flow that asked a question and discarded the answer.It now rides the Agents answer, since it comes from the same decision and the same timestamp, and step 3 spends it instead of asking again.
Where to look
org_public, so a Return on a re-run would widen an existingprivateon a question the user had already answered. An answered-but-unset screen therefore re-writes what the profile already holds — the lane's contract for a null answer, and a no-op downstream.SetupCommand.DecideVisibilityis the single place that decides, extracted becauseHandleSetupAsync's interactive branches have no coverage at all (everyHandleAsync_*test drives--no-prompt, which never reaches the browser leg).AppConfig.ValidVisibilities, never forwarded. This is the one field on the lane that persists locally: it lands in profile config and is stamped on every session afterwards, so a stop a newer server invented would be written to a file this build owns and read back by something whose meaning for it cannot be checked here.--default-visibility. That flag is read only under--no-prompt, and the browser leg is skipped there entirely, so the two can never both be present. Interactively the flag is ignored today, before and after this change; making it live is a separate behaviour change.VisibilityLabelso the prompt's converter and the browser-answer line cannot describe the same stop differently.Verification
Core 2425, Cli 3718, App 1161, Integration 233 — all pass, bar two unrelated timing tests that pass in isolation (
Callback_fault_losing_to_dispose_is_logged_once_and_run_settles_detached, local IPC; andCodex_StdoutHandshake_UnaffectedByALargeUnreachableSpoolBacklog).dotnet publish -c Releaseclean, no IL2026/IL3050.Five mutations, each killed. On the boundary: ignoring the validation, and never reading the field — both needed reshaping to compile, since dropping the reference trips IDE0005/IDE0051 as errors, which is analyzer protection rather than coverage. On the rule: an answered-but-unset screen falling through to the prompt, an unsettled screen treated as answered, and the kept branch inventing a fallback instead of carrying the profile's value.
The field is round-tripped through the source-generated JSON context against WireMock, present and absent — nothing else covered that, since every other test builds
FirstRunFlowResponsedirectly, so a naming or AOT-binding slip would have left the profile untouched for ever with the suite green.