Skip to content

Fix seven Activity, screen and approval bugs the computer journeys found - #1543

Open
davidmckayv wants to merge 3 commits into
mainfrom
fix/e2e-slice3-bugs
Open

davidmckayv wants to merge 3 commits into
mainfrom
fix/e2e-slice3-bugs

Conversation

@davidmckayv

Copy link
Copy Markdown
Contributor

Fixes the five bugs the computer and approvals journeys (#1529) found, and two more Activity bugs seen on 1.0.2 in the same code. Each has a journey assertion that fails on main and passes here, plus a unit test where the rule is cheap to pin.

1. Activity named a declined or handed-over step by its ref

It showed "Declined f2e13" and "Clicked on page e14" where it should say Place order and Password. A step that ran gets the computer's answer, which names the element. A step the person declined, or did themselves, gets only a sentence, so the row fell back to the ref. The snapshot the Bot took that ref from also names the element, so Activity now takes the label from there (app/src/lib/computers/activity.ts). It shows the ref only when no snapshot names it.

  • Journeys: approvals-allow-once checks that the declined row says Place order, and computer-sign-in checks that the handed-over row says Password.
  • Unit: app/tests/computer-activity-names-and-times.test.ts.

2. Steps after an approval had no time

Activity timed a step by the page watching it run. After an approval the server takes the step and carries the turn on by itself, so those steps never pass through the page. Their rows had no time, and no row had one once the conversation was opened again.

  • A step's answer now records when it finished (finishedAt, from withFinishedAt in shared/computer-control.ts). The server stamps the steps it runs (server/src/computer/headless-tools.ts), the browser stamps the steps it runs (app/src/lib/copilot/approval-tools.tsx), and Activity reads the time from the answer.

  • A step the person answered on the card ("I did it myself", Deny) is timed by when they answered. Before, its time came from when its answer reached the page, after the server's turn was over, so it read as later than the steps that followed it.

  • A step declined or handed over keeps its time across a reload too (see 7).

  • Journeys: approvals-allow-once, computer-sign-in and computer-secret check that every row has a time and that the times run newest first.

  • Unit: server/tests/headless-tools.test.ts checks that the answer carries the time. The Activity test above checks that the row shows it, that an answered step is timed by the answer, and that a step still waiting for a person gets no time.

3. A black band under the Bot's screen after a tab closed

I reproduced this against the computer on its own. Frames were 1280x800 with two tabs open and dropped to 1280x744 once the second tab closed, staying that way until the next navigation. The window bounds and the page's own size were both correct, so the problem was in the tab itself.

The cause is Chromium's warning bar for --no-sandbox. It sits in the Bot's first tab, and Chromium only makes room for it when that tab is laid out again. The window had already been fitted (fitWindowsToViewport, at the first use of the tabs) before that happened, so the tab came back 56 pixels short, and it stayed short after Hand back.

The fix fits the window to the page the live screen is about to show, every time it starts showing one (agent-computer/src/index.ts), and to the tab in front after each tab action (bot-browser-host.ts). The fit is measured from the tab as it is laid out at that moment, so it corrects itself. I probed it four times against a built computer and frames stayed 1280x800 through opening a tab, closing it and Hand back.

I also tried turning the bar off with Chromium's CommandLineFlagSecurityWarningsEnabled policy. That removes the bar, but Chromium loads policies after launch, so in half the launches the bar appeared briefly and the window was fitted around it (856 high). Fitting when the screen starts works with or without the bar, so the policy is not in this PR.

  • Journey: computer-take-control checks that the screen fills its frame with no band after the tab closes and after Hand back, and that it stays filled for 2 s, because the short frame arrives about a second after the close.

4. The Reset dialog's title flickered as it closed

Answering cleared the row the dialog was about, but the dialog redraws its words on every frame of the fade-out, so the title fell back to "Reset the computer?". The row is now kept while the dialog closes (app/src/components/manage/computers-page.tsx), for both Reset and Update computer.

  • Journey: computer-take-control records every title the dialog shows from Reset it until the dialog is gone.
  • Unit: app/tests/personal-settings.test.tsx.

5. The masked secret box couldn't be used for a password

The instruction was wrong. The rule was right.

  • The rule (safetyRequirement in server/src/approvals/policy.ts) hands a Bot's own click or typing on a password field to the person, because that is the Bot entering a credential on a website. That is correct, and the computer-sign-in journey depends on it.
  • The masked path never needed a click. The Bot names the field by ref and snapshot, and when the person sends the value, the computer clicks and fills that field itself (/human/secret in agent-computer/src/index.ts). The request isn't gated, so nothing is handed over.
  • But the standing instructions, the browser's computer_request_secret and the server's version all told the Bot to click the field first. A Bot that did as told handed the whole browser over on that click and never reached the masked box. A recording against the real model with main's wording did exactly that.

The instructions now say to give the field's ref and not to click it first. The wording is one sentence in shared/computer-control.ts (SECRET_FIELD_BY_REF), used by both tools, and the standing instructions say the same (shared/bot-prompt.ts).

  • Journey: new computer-secret. A Bot asks for the password with the masked box, the person types it in, and the Bot signs in and answers. No step is handed over.
  • Unit: server/tests/computer-secret-without-a-click.test.ts checks the rule (a Bot's click on a password field is still handed over) and that no instruction tells the Bot to click first.

The Bot's tool instructions changed, so I recorded computer-secret against the real model (that journey's fixtures only; core-loop's were put back). With the new wording the Bot types the username, calls computer_request_secret on the password field's ref, clicks Sign in and reads the page.

6. No Activity line for "Allow for this conversation"

The banner said the order was allowed for the conversation, but Activity said nothing about it. A declined step gets "You declined it." and a step done by the person gets "You did it yourself.", but an allowed one got no line. The step's own answer is the computer's, which doesn't say how the person answered. The approvals inbox the conversation already reads holds the person's decision for each step, so Activity now takes it from there (decisionsIn) and adds a line: "Allowed once.", "Allowed for this conversation." or "Always allowed here." (activity-log.tsx).

  • Journey: approvals-this-conversation checks the line after the first order, and after a reload ("Allowed once." on the last order too).
  • Unit: app/tests/activity-allowed-line.test.tsx, plus the Activity test above.

7. Times disappeared on reload

This is covered by the answers now recording when they finished (bug 2), for steps run by the browser and by the server alike. A step declined or handed over keeps its time too: its answer is a sentence for the Bot, so the time comes from the decision in the same inbox. That also closes the one gap noted under bug 2. Conversations from before this change still have no times in their old answers.

  • Journeys: computer-browse and approvals-this-conversation check that every row keeps its time after a reload, and approvals-allow-once checks the same after its reload, including the declined row.

Not here

The reply after an approval that gets stored with a piece missing belongs to the run stream and is going to that team. Nothing here touches the stream or persistence.

Checks

  • Format, lint and typecheck are clean. Unit and integration tests for these areas pass, and so does the full bun run test:ci apart from failures that are environmental here and fail on main too (Helm charts, desktop signing, live providers).
  • Red runs: each new journey assertion fails on main (Declined f2e13, Clicked on page e14, no times after the approval, a 43 px band after Hand back, the extra "Reset the computer?" title, a hand-over card instead of the masked box, no "Allowed for this conversation." line, no times after a reload).
  • bun run test:e2e: two full runs pending; results below.

- Activity names a declined or handed-over step's control from the
  snapshot its ref came from, not the ref ("Declined Place order").
- Every Activity row has its time: a step's answer says when it
  finished (stamped by the server for the steps it runs after an
  approval, and by the browser for its own), and a step the person
  answered is timed by their answer.
- The Bot's screen fills its frame after a tab closes and after Hand
  back: the window is fitted to the page the live screen starts showing,
  so Chromium's --no-sandbox warning bar no longer cuts 56 px off the
  tab left in front.
- The Reset and Update computer dialogs keep their row while they
  close, so the title no longer falls back to "Reset the computer?".
- The masked secret box asks for the field by its ref: the instructions
  no longer tell the Bot to click the field first, which handed the step
  to the person.

Each fix has a journey assertion that fails on main, plus unit tests
where the rule is cheap to pin. New computer-secret journey, recorded
against the real model because the Bot's instructions changed.
…reload

- A step the person allowed on its card has a line saying how
  ("Allowed for this conversation."), as a declined one says "You
  declined it." The decision comes from the approvals inbox the
  conversation already reads.
- A step declined or done by the person keeps the time it was answered
  when the conversation is opened again: its answer is a sentence for
  the Bot, so the time comes from the same decision.
The browser now stamps each computer step's answer with when it finished,
so the wrapper's tests expect it beside the result they pin.

This branch has not been deployed

No deployments
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