mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 06:04:11 +01:00
Wait for the saved Studio file before editing a skill (#18305)
## Thinking Path > - Paperclip lets people manage AI agents and use their saved work. > - Product E2E journeys check these user paths in classic and Cloud environments. > - The shared Skill Studio helper edits a skill created by an agent. > - Studio can mount the editor before its saved file finishes loading. > - Typing into that empty draft can replace or lose content when the file arrives. > - This change waits for the existing saved body before it starts the edit. > - Browser regressions check delayed loading and refusal to edit an unloaded file. ## Linked Issues or Issue Description Refs #18222 for the shared Product E2E journeys. No duplicate hydration fix was found. **What happened?** The shared Studio helper started typing as soon as the editor control was visible. A delayed file query could replace that draft. The helper could then lose the original content or fail to save its complete marker. **Expected behavior** The helper must load the original file before editing it. It must preserve that content, save the new marker, reload, and read the result from the task card. **Steps to reproduce** Run the controlled Studio browser fixture with a 1.5-second file load. The old helper saves the marker without the original `Verify checks.` body. With no file load, it still sends input events to the empty editor. ## What Changed - Wait for the original saved body before typing in the shared Studio helper. - Add browser checks for immediate load, delayed load, and an unavailable file. - Include these checks in the credential-free browser support suite. - Document the saved-file readiness requirement. ## Verification - Against the old helper, both new failure cases fail: the delayed case loses the original body, and the unloaded case sends 13 input events. - With the change, all three browser checks pass. They verify the saved original body and marker, reload, task-card readback, and zero input or save calls for an unloaded file. - `node node_modules/typescript/bin/tsc -p tests/runner-e2e/tsconfig.json --noEmit` passes. - `git diff --check` passes. Full repository CI passes in [41058312896](https://github.com/paperclipai/paperclip/actions/runs/41058312896), including typecheck, tests, build and all eight E2E shards. Greptile reviewed this exact head at 5/5 with no open threads. No live acceptance result is claimed by these controlled fixtures. ## Risks This adds a readiness assertion to an existing test helper. Both callers already use the same skill fixture and verify `Verify checks.` in the task card. An unavailable file now fails before an edit can occur. The change does not alter application behavior, model instructions, grading, or deadlines. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The host does not expose the exact underlying model version or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added tests where applicable - [x] I have updated relevant documentation - [x] I have considered and documented risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all reviewer comments before merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
cd5f930558
commit
0c70cedce9
4 files changed
+53
-1
No files matched your search
@@ -33,6 +33,10 @@ never start a server or depend on a tenant filesystem.
|
||||
| `stop-redirect` | Native agent; three work runs; qualified Docker oracle | Download a verified checkpoint, Stop an active revision, require native cancellation acknowledgement, submit a new direction once and verify completion without original-run activity |
|
||||
| `cloud-onboarding` | Signed-in admitted Cloud account; fresh workspace; qualified bounded provider route | Create workspace in the portal, connect API key and native Codex agent in the wizard, complete a task, download and independently check its artifact |
|
||||
|
||||
The shared Studio helper waits for the original saved file body before typing.
|
||||
An editor control can mount before the file query completes; editing that empty
|
||||
draft can otherwise lose the original content when the file arrives.
|
||||
|
||||
Supported acquisitions are managed self-hosted (`local`), registered existing
|
||||
Cloud (`local` or `daytona` when present), and new Cloud. New Cloud must start
|
||||
with onboarding. Its context can then satisfy the identical task-properties
|
||||
|
||||
@@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test";
|
||||
/** Browser-only harness regressions: no Paperclip instance or provider credentials. */
|
||||
export default defineConfig({
|
||||
testDir: ".",
|
||||
testMatch: ["native-task-reply.spec.ts", "screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts"],
|
||||
testMatch: ["skill-studio-flow.spec.ts", "native-task-reply.spec.ts", "screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "github-review-check.spec.ts", "antigravity-journey-browser.spec.ts"],
|
||||
workers: 1,
|
||||
retries: 0,
|
||||
timeout: 10_000,
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
import { editCreatedSkillThroughStudio } from "./skill-studio-flow.js";
|
||||
|
||||
for (const hydrationMs of [0, 1500]) test(`Studio preserves the loaded file before editing (${hydrationMs}ms hydration)`, async ({ page }) => {
|
||||
const skillId = "owned-skill", skillName = "owned-qa-skill", marker = "Studio edit marker: owned-campaign";
|
||||
let saved = "Verify checks.", saves = 0;
|
||||
await page.exposeFunction("saveSkill", (body: string) => { saved = body; saves++; });
|
||||
await page.route("https://fixture.test/**", async route => {
|
||||
const studio = new URL(route.request().url()).pathname.includes("/skills/studio/");
|
||||
const body = studio ? `<button role="combobox">${skillName}</button>
|
||||
<div role="textbox" aria-label="editable markdown" contenteditable="true"></div>
|
||||
<button id="save">Save</button>
|
||||
<script>
|
||||
const editor=document.querySelector('[contenteditable]');
|
||||
setTimeout(()=>{editor.innerText=${JSON.stringify(saved)}},${hydrationMs});
|
||||
document.querySelector('#save').onclick=()=>window.saveSkill(editor.innerText);
|
||||
</script>` : `<article aria-label="Skill created: ${skillName}"><button id="card">View skill</button></article>
|
||||
<section id="detail" hidden><h2>${skillName}</h2>${saved.split(/\n+/).map(line => `<p>${line}</p>`).join("")}
|
||||
<a role="button" href="/QA/skills/studio/${skillId}">Open in Skill Studio</a></section>
|
||||
<script>document.querySelector('#card').onclick=()=>document.querySelector('#detail').hidden=false</script>`;
|
||||
await route.fulfill({ contentType: "text/html", body });
|
||||
});
|
||||
await page.goto("https://fixture.test/QA/issues/QA-1");
|
||||
await editCreatedSkillThroughStudio({ page, skillId, skillName, marker, deadlineAt: Date.now() + 8000,
|
||||
read: async () => ({ content: saved }) });
|
||||
expect(saves).toBe(1);
|
||||
expect(saved).toContain("Verify checks.");
|
||||
expect(saved).toContain(marker);
|
||||
});
|
||||
|
||||
test("Studio never edits or saves an unhydrated file", async ({ page }) => {
|
||||
let saves = 0, inputs = 0;
|
||||
await page.exposeFunction("savedUnloadedFile", () => { saves++; });
|
||||
await page.exposeFunction("editedUnloadedFile", () => { inputs++; });
|
||||
await page.route("https://fixture.test/**", route => route.fulfill({ contentType: "text/html", body:
|
||||
`<article aria-label="Skill created: owned"><button onclick="document.querySelector('section').hidden=false">View</button></article>
|
||||
<section hidden><h2>owned</h2><p>Verify checks.</p><a role="button" href="/QA/skills/studio/id">Open in Skill Studio</a></section>
|
||||
<button role="combobox">owned</button><div contenteditable="true" role="textbox" aria-label="editable markdown" oninput="window.editedUnloadedFile()"></div>
|
||||
<button onclick="window.savedUnloadedFile()">Save</button>` }));
|
||||
await page.goto("https://fixture.test/QA/issues/QA-1");
|
||||
await expect(editCreatedSkillThroughStudio({ page, skillId: "id", skillName: "owned", marker: "New content", deadlineAt: Date.now() + 7000,
|
||||
read: async () => ({}) })).rejects.toThrow();
|
||||
expect(inputs).toBe(0);
|
||||
expect(saves).toBe(0);
|
||||
});
|
||||
@@ -16,6 +16,9 @@ export async function editCreatedSkillThroughStudio(input: {
|
||||
await expect(page).toHaveURL(new RegExp(`/skills/studio/${input.skillId}$`));
|
||||
await expect(page.getByRole("combobox").filter({ hasText: skillName })).toBeVisible();
|
||||
const editor = page.getByRole("textbox", { name: "editable markdown", exact: true });
|
||||
// Studio mounts the editor before its file query completes. Wait for the
|
||||
// saved body, otherwise hydration can replace edits made to the empty draft.
|
||||
await expect(editor).toContainText("Verify checks.");
|
||||
await editor.click();
|
||||
await editor.press("ControlOrMeta+End");
|
||||
await editor.press("Enter");
|
||||
|
||||
Reference in new issue
Block a user