From b0a85e64711366fd43a8232c50e626566476c841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Wed, 4 Mar 2026 12:22:50 +0000 Subject: [PATCH] fix: restore git status update in workspace polling loop (#364) --- apps/backend/cmd/mock-agent/scenarios.go | 97 +++++++++++++- .../server/process/workspace_monitor.go | 3 +- apps/web/e2e/tests/diff-update.spec.ts | 121 ++++++++++++++++++ 3 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 apps/web/e2e/tests/diff-update.spec.ts diff --git a/apps/backend/cmd/mock-agent/scenarios.go b/apps/backend/cmd/mock-agent/scenarios.go index 0ecdcfb12f..42603a2789 100644 --- a/apps/backend/cmd/mock-agent/scenarios.go +++ b/apps/backend/cmd/mock-agent/scenarios.go @@ -30,8 +30,12 @@ func emitPredefinedScenario(enc *json.Encoder, scanner *bufio.Scanner, name stri scenarioMultiTurn(enc) case "diff-expansion-setup": scenarioDiffExpansionSetup(enc) + case "diff-update-setup": + scenarioDiffUpdateSetup(enc) + case "diff-update-modify": + scenarioDiffUpdateModify(enc) default: - emitTextBlock(enc, "Unknown e2e scenario: "+name+". Available: simple-message, read-and-edit, permission-flow, error, subagent, all-tools, multi-turn, diff-expansion-setup", "") + emitTextBlock(enc, "Unknown e2e scenario: "+name+". Available: simple-message, read-and-edit, permission-flow, error, subagent, all-tools, multi-turn, diff-expansion-setup, diff-update-setup, diff-update-modify", "") } } @@ -443,3 +447,94 @@ func scenarioDiffExpansionSetup(enc *json.Encoder) { fixedDelay(100) emitTextBlock(enc, "diff-expansion-setup complete: expansion_test.go has two-hunk uncommitted diff (lines 50 and 150 of 200).", "") } + +// scenarioDiffUpdateSetup creates a simple file, commits it, then modifies it +// to leave an uncommitted diff. This is the first step of the diff-update test. +// The file content is simple so assertions can verify exact diff content. +func scenarioDiffUpdateSetup(enc *json.Encoder) { + fixedDelay(50) + + wd, err := os.Getwd() + if err != nil { + emitTextBlock(enc, "diff-update-setup: getwd failed: "+err.Error(), "") + return + } + + filePath := "diff_update_test.txt" + originalContent := "line 1: original\nline 2: unchanged\nline 3: original\n" + + // Write original content + if err := os.WriteFile(filePath, []byte(originalContent), 0o644); err != nil { + emitTextBlock(enc, "diff-update-setup: write failed: "+err.Error(), "") + return + } + + gitEnv := append(os.Environ(), + "GIT_AUTHOR_NAME=Mock Agent", + "GIT_AUTHOR_EMAIL=mock@test.local", + "GIT_COMMITTER_NAME=Mock Agent", + "GIT_COMMITTER_EMAIL=mock@test.local", + ) + + runGitCmd := func(args ...string) error { + cmd := exec.Command("git", append([]string{ + "-c", "commit.gpgsign=false", + "-c", "tag.gpgsign=false", + }, args...)...) + cmd.Dir = wd + cmd.Env = gitEnv + out, cmdErr := cmd.CombinedOutput() + if cmdErr != nil { + fmt.Fprintf(os.Stderr, "mock-agent: git %v failed: %v\nOutput: %s\n", args, cmdErr, out) + } + return cmdErr + } + + // Clean up from any previous test run + _ = runGitCmd("rm", "--force", filePath) + _ = runGitCmd("commit", "-m", "cleanup diff_update_test.txt") + + // Re-write and commit original content + if err := os.WriteFile(filePath, []byte(originalContent), 0o644); err != nil { + emitTextBlock(enc, "diff-update-setup: re-write failed: "+err.Error(), "") + return + } + + if err := runGitCmd("add", filePath); err != nil { + emitTextBlock(enc, "diff-update-setup: git add failed", "") + return + } + if err := runGitCmd("commit", "-m", "add diff_update_test.txt"); err != nil { + emitTextBlock(enc, "diff-update-setup: git commit failed", "") + return + } + + // Modify the file to create an uncommitted diff + modifiedContent := "line 1: FIRST_MODIFICATION\nline 2: unchanged\nline 3: original\n" + if err := os.WriteFile(filePath, []byte(modifiedContent), 0o644); err != nil { + emitTextBlock(enc, "diff-update-setup: write modified failed: "+err.Error(), "") + return + } + + fixedDelay(100) + emitTextBlock(enc, "diff-update-setup complete: diff_update_test.txt has FIRST_MODIFICATION", "") +} + +// scenarioDiffUpdateModify modifies the diff_update_test.txt file again, +// changing the diff content. This tests that the UI updates the diff display +// when files change after the initial diff was shown. +func scenarioDiffUpdateModify(enc *json.Encoder) { + fixedDelay(50) + + filePath := "diff_update_test.txt" + + // Modify the file again with different content + modifiedContent := "line 1: SECOND_MODIFICATION\nline 2: unchanged\nline 3: ALSO_CHANGED\n" + if err := os.WriteFile(filePath, []byte(modifiedContent), 0o644); err != nil { + emitTextBlock(enc, "diff-update-modify: write failed: "+err.Error(), "") + return + } + + fixedDelay(100) + emitTextBlock(enc, "diff-update-modify complete: diff_update_test.txt now has SECOND_MODIFICATION", "") +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_monitor.go b/apps/backend/internal/agentctl/server/process/workspace_monitor.go index 15e45bb2c2..5e72bc85ae 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_monitor.go +++ b/apps/backend/internal/agentctl/server/process/workspace_monitor.go @@ -46,7 +46,8 @@ func (wt *WorkspaceTracker) monitorLoop(ctx context.Context) { lastState = currentState wt.logger.Debug("workspace state changed, updating") - // Update file list and notify subscribers + // Update git status (includes diff data) and file list, then notify subscribers + wt.updateGitStatus(ctx) wt.updateFiles(ctx) wt.notifyWorkspaceStreamFileChange(types.FileChangeNotification{ Timestamp: time.Now(), diff --git a/apps/web/e2e/tests/diff-update.spec.ts b/apps/web/e2e/tests/diff-update.spec.ts new file mode 100644 index 0000000000..f9f10ab64d --- /dev/null +++ b/apps/web/e2e/tests/diff-update.spec.ts @@ -0,0 +1,121 @@ +import { test, expect } from "../fixtures/test-base"; +import { SessionPage } from "../pages/session-page"; +import type { ApiClient } from "../helpers/api-client"; +import type { SeedData } from "../fixtures/test-base"; +import type { Page } from "@playwright/test"; + +/** + * Seed a task using the diff-update-setup mock scenario and navigate to + * its session page, waiting for the agent turn to complete. + * + * The scenario creates a simple text file, commits it, then modifies line 1 + * to contain "FIRST_MODIFICATION", leaving an uncommitted diff. + */ +async function seedDiffUpdateTask( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, +): Promise<{ session: SessionPage; sessionId: string }> { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Diff Update E2E", + seedData.agentProfileId, + { + description: "/e2e:diff-update-setup", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + + await testPage.goto(`/s/${task.session_id}`); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + + // Wait for the first turn to complete + await expect(session.chat.getByText("diff-update-setup complete", { exact: false })).toBeVisible({ + timeout: 45_000, + }); + + return { session, sessionId: task.session_id }; +} + +/** Click the Changes dockview tab. */ +async function openChangesTab(testPage: Page) { + const changesTab = testPage.locator(".dv-default-tab", { hasText: "Changes" }); + await expect(changesTab).toBeVisible({ timeout: 10_000 }); + await changesTab.click(); +} + +/** Click the file row for diff_update_test.txt to open its diff view. */ +async function openDiffUpdateFileDiff(testPage: Page) { + const fileRow = testPage + .locator("button, [role='button'], [class*='file']") + .filter({ hasText: "diff_update_test.txt" }) + .first(); + await expect(fileRow).toBeVisible({ timeout: 10_000 }); + await fileRow.click(); +} + +test.describe("Diff update on file change", () => { + test.describe.configure({ retries: 1, timeout: 120_000 }); + + test("shows initial diff with FIRST_MODIFICATION", async ({ testPage, apiClient, seedData }) => { + await seedDiffUpdateTask(testPage, apiClient, seedData); + await openChangesTab(testPage); + await openDiffUpdateFileDiff(testPage); + + // The Pierre Diffs viewer should show the initial modification + const diffsContainer = testPage.locator("diffs-container"); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + }); + + test("diff updates when agent modifies file again", async ({ testPage, apiClient, seedData }) => { + const { session } = await seedDiffUpdateTask(testPage, apiClient, seedData); + await openChangesTab(testPage); + await openDiffUpdateFileDiff(testPage); + + // Verify initial diff content (scoped to diffs-container to avoid matching chat text) + const diffsContainer = testPage.locator("diffs-container"); + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + await expect(diffsContainer.getByText("FIRST_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // Click on the Agent tab to make the chat input visible again + await session.clickTab("Agent"); + + // Send another message to trigger the second modification + await session.sendMessage("/e2e:diff-update-modify"); + + // Wait for the second turn to complete + await expect( + session.chat.getByText("diff-update-modify complete", { exact: false }), + ).toBeVisible({ timeout: 45_000 }); + + // Switch back to Changes tab and click on the diff file again to see the updated diff. + // The git status (with diff data) should have been updated via polling when + // the file changed - this is the bug we're testing for. + await openChangesTab(testPage); + await openDiffUpdateFileDiff(testPage); + + // Wait for the diffs container to be visible again + await expect(diffsContainer).toBeVisible({ timeout: 15_000 }); + + // The diff should now show SECOND_MODIFICATION instead of FIRST_MODIFICATION. + await expect(diffsContainer.getByText("SECOND_MODIFICATION", { exact: true })).toBeVisible({ + timeout: 15_000, + }); + + // Also verify the additional change on line 3 + await expect(diffsContainer.getByText("ALSO_CHANGED", { exact: true })).toBeVisible({ + timeout: 10_000, + }); + }); +});