Skip to content

Stop showing a request for a secret the run that asked has outlived - #457

Open
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/stale-secret-request
Open

Stop showing a request for a secret the run that asked has outlived#457
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/stale-secret-request

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

An unanswered request to take the wheel stops being shown after ten minutes, because control belongs
to a Bot's computer rather than to a conversation. The other prompt held on that same state machine —
the masked box a Bot opens when it needs one value it must not be told — was never given the same
treatment. requestSecret records no time at all, so nothing expires it: it is cleared only when the
value lands, or when somebody takes the wheel and hands it back.

The surface does not tell the two apart. app/src/components/computer/needs-you.ts lights the same
"needs you" on either:

setNeeded(Boolean(state && (state.requested || state.secretWanted !== undefined)));

So the fix for the ask left the half that asks for a password behaving exactly as the ask used to: the
run that made it has ended, and every later conversation with that Bot is flagged and shows a request
captioned with a label written for whoever asked half a day earlier.

It is also the prompt where being stale matters more than being untidy. Answering it types a value
into a field named by a ref from a snapshot the browser has long since moved past — deliberately with
no generation check, on the reasoning that a ref resolves only against the most recent snapshot —
which is a sound argument inside the run that asked and not one about a request from an hour ago.

The change

A private secretRequestedAt in the closure, and dropStaleSecret() on read. Kept off ControlState
because nothing outside needs it, and because the existing test that pins the exact set of secret*
keys in the published state is right to pin them.

Expired from pendingSecret() as well as from get(), not only from the path the surface polls:
pendingSecret is what /human/secret asks before it types, so a prompt that has stopped being shown
has to stop being answerable at the same moment.

SECRET_REQUEST_TTL_MS is the same ten minutes, named separately because they are two prompts and
shortening one should not silently shorten the other.

Fail before, pass after

The tests were written first, then agent-computer/src/control.ts was reverted to main with
git checkout HEAD -- agent-computer/src/control.ts:

$ bun test agent-computer/tests/control.test.ts
(fail) an unanswered request for a secret > stops being shown once it is stale, and takes the field it named with it
        Received: "the six-digit code"
(fail) an unanswered request for a secret > stops being answerable at the same moment it stops being shown
        Received: { ref: "e12", snapshotId: undefined }
(fail) an unanswered request for a secret > a fresh request after a stale one is shown, not swallowed by it
        Received: { ref: "e12", snapshotId: undefined }
 25 pass
 3 fail
Ran 28 tests across 1 file.

With the fix restored:

$ bun test agent-computer/tests/control.test.ts
 28 pass
 0 fail
Ran 28 tests across 1 file.

Guard against over-correcting. The fourth new test, "is still shown, and still answerable, inside
the window", passes before and after: five minutes in, the label is still there and pendingSecret
still returns the field, so this shortens nothing that was working. "A fresh request after a stale
one" is the other direction — the expiry has to clear its own bookkeeping, or a Bot that asked twice
would be answerable neither time. Every existing test in the file passes unchanged, including
"a FAILED attempt leaves the request open" and "handing the wheel over or back closes any pending
secret".

Verification

$ bun test agent-computer/tests supervisor/tests
 241 pass / 20 skip / 20 fail        # 237 pass / 20 skip / 20 fail before, the same 20 failures
$ cd agent-computer && bunx tsc --noEmit    # clean
$ bunx biome check agent-computer/src/control.ts agent-computer/tests/control.test.ts   # clean

The 20 failures are identical on main and on this branch: shell.test.ts spawns /bin/bash and
workspace.test.ts creates symlinks, and this Windows machine can do neither. Nothing here touches
either file.

Note

The changelog entry sits at the top of ## Unreleased and shares that anchor with other open PRs, so
it may need a one-line rebase.

An unanswered ask to take the wheel stops being shown after ten minutes, because
control belongs to the computer rather than to a conversation. The secret request
on the same state machine had no expiry at all, and the surface lights the same
"needs you" on both, so a Bot stayed flagged and every later conversation showed
a masked box wanting a password for a run that had ended.

It expires on the same window, on read, and pendingSecret checks it too so a
prompt that has stopped being shown stops being answerable at the same moment.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

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