Skip to content

Commit 2c566db

Browse files
committed
Address review findings (CL-7366)
1 parent 4e34456 commit 2c566db

3 files changed

Lines changed: 65 additions & 82 deletions

File tree

‎packages/workflows/src/authoring/registry.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import { PackageJSON } from "@intx/types/package-json";
5151

5252
import { WorkflowAuthorError } from "./errors";
5353
import {
54+
normalizeEntryPath,
5455
PACKAGE_JSON_PATH,
5556
validateWorkflowSourceTree,
5657
} from "./source-tree";
@@ -479,7 +480,12 @@ export function createWorkflowAuthorRegistry(
479480
}
480481
const files: Record<string, string> = {};
481482
await collectTree(reads, "", files);
482-
if (!(input.entry in files)) {
483+
// Normalized the same way `validateWorkflowSourceTree` normalizes an
484+
// author-time `interchange.workflow` entry: the tree's keys carry no
485+
// leading `./`, but a caller (this same test suite included) may
486+
// still pass the entry as written in `package.json`.
487+
const entry = normalizeEntryPath(input.entry);
488+
if (!(entry in files)) {
483489
throw new WorkflowAuthorError(
484490
"invalid",
485491
`entry ${JSON.stringify(input.entry)} names no file in commit ${input.commitSha}`,
@@ -509,13 +515,13 @@ export function createWorkflowAuthorRegistry(
509515
);
510516
}
511517
const packageName = manifest.name;
512-
const entrySource = files[input.entry] ?? "";
518+
const entrySource = files[entry] ?? "";
513519
const inertLiteral = tryReadInertDefaultExport(entrySource);
514520
const toolPackagePins = extractToolPackagePins(inertLiteral);
515521

516522
return {
517523
commitSha: input.commitSha,
518-
entry: input.entry,
524+
entry,
519525
files: Object.keys(files),
520526
toolPackagePins,
521527
packageName,

‎scripts/e2e/browser/walkthrough.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,10 +1065,23 @@ async function run(): Promise<void> {
10651065
const name = `Walkthrough routine ${Date.now()}`;
10661066
await page.type("#routine-panel-name", name);
10671067
await page.keyboard.press("Tab"); // blur with no target picked yet
1068+
// A fixed grace period, not a race against the positive wait below:
1069+
// an asymmetric short/long timeout pair invites a flaky false pass
1070+
// (this negative wait could elapse with no autosave firing purely
1071+
// because it's short, even if the blocked-save bug exists). Poll
1072+
// for the same 15s window as the positive assertion, and read
1073+
// `role="status"` inside the panel — not `document.body` — so a
1074+
// stale "Saved" toast elsewhere on the page can't false-positive.
1075+
await Bun.sleep(1_500);
10681076
const savedBeforePick = await page
1069-
.waitForFunction(() => document.body.textContent?.includes("Saved"), {
1070-
timeout: 1_500,
1071-
})
1077+
.waitForFunction(
1078+
() =>
1079+
document
1080+
.querySelector(".shell-routine-pane")
1081+
?.querySelector('[role="status"]')?.textContent
1082+
?.includes("Saved") ?? false,
1083+
{ timeout: 13_500 },
1084+
)
10721085
.then(() => true)
10731086
.catch(() => false);
10741087

@@ -1087,9 +1100,14 @@ async function run(): Promise<void> {
10871100
await page.type("#routine-panel-name", " picked");
10881101
await page.keyboard.press("Tab");
10891102
const saved = await page
1090-
.waitForFunction(() => document.body.textContent?.includes("Saved"), {
1091-
timeout: 15_000,
1092-
})
1103+
.waitForFunction(
1104+
() =>
1105+
document
1106+
.querySelector(".shell-routine-pane")
1107+
?.querySelector('[role="status"]')?.textContent
1108+
?.includes("Saved") ?? false,
1109+
{ timeout: 15_000 },
1110+
)
10931111
.then(() => true)
10941112
.catch(() => false);
10951113
if (savedBeforePick || !saved) {

‎scripts/e2e/routine-alignment.test.ts‎

Lines changed: 32 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
// CL-7366: prove the aligned routine and workflow-authoring paths end to
22
// end, against a real Postgres and a real hub+sidecar pair, driving only
3-
// PUBLIC APIs (no seeded internal rows to fake success). Scenarios:
3+
// PUBLIC APIs (no seeded internal rows to fake success) — with one
4+
// sanctioned exception: `readRunAddress` below reads `workflow_run`
5+
// directly from Postgres, because no public route exposes a run's own
6+
// address by run id (consistent with `routine-trigger-input.test.ts`'s
7+
// existing precedent; see that function's own doc comment). Scenarios:
48
//
59
// 1. Bench-level routine create with no invited agent, targeting a
610
// deployed definition discovered via `GET .../workflows/targets`;
@@ -11,12 +15,10 @@
1115
// is real but belongs to a different tenant, both 404.
1216
// 4. Myra-shaped author -> preview -> deploy over the run-authenticated
1317
// `/api/workflow-workflow-authoring` surface: authoring an asset,
14-
// previewing its deploy (CL-7362's native preview seam is not wired
15-
// yet — the route answers a documented `unavailable` envelope,
16-
// asserted here rather than faked; see the inline note below),
17-
// deploying it directly (now a routine target, previously not),
18-
// and a wrong `expectedWireHash` on redeploy failing closed with
19-
// `wire_hash_mismatch`.
18+
// previewing its deploy (a static, read-only render of the committed
19+
// source — package name, file list, declared tool pins; CL-7362's
20+
// probe-without-freeze vendored delta was reverted, see VENDORED.md),
21+
// and deploying it directly (now a routine target, previously not).
2022
// 5. The routine run's own agent principal holds no `approval:*`
2123
// grant — approving a deploy is never something an agent can do
2224
// to itself.
@@ -652,40 +654,31 @@ describe.skipIf(databaseUrl === undefined)(
652654
},
653655
);
654656
const previewData: unknown = await preview.json();
655-
if (preview.status === 502) {
656-
// CL-7362 ("Preview: native probe with empty ApprovalSet, no
657-
// freeze") is not wired yet — `docs/workflow-source-authoring.md`
658-
// lists this seam under "Seams that do not exist yet" as of
659-
// this proof landing. The route answers a canonical
660-
// `unavailable` envelope rather than pretending to preview, so
661-
// this asserts exactly that fail-closed shape instead of a
662-
// real wireHash/grants preview, and the deploy below proceeds
663-
// without a preview-derived expectedWireHash.
664-
const previewError = record(
665-
record(previewData, "preview response")["error"],
666-
"preview error",
667-
);
668-
if (previewError["code"] !== "unavailable") {
669-
throw new Error(
670-
`deploy preview: expected the documented CL-7362 ` +
671-
`"unavailable" envelope, got ${JSON.stringify(previewData)}`,
672-
);
673-
}
674-
} else if (preview.status !== 200 && preview.status !== 201) {
657+
if (preview.status !== 200 && preview.status !== 201) {
675658
throw new Error(
676659
`deploy preview: expected 200/201, got ${preview.status}: ${JSON.stringify(previewData)}`,
677660
);
678-
} else {
679-
const previewBody = record(
680-
record(previewData, "preview response")["data"],
681-
"preview response data",
661+
}
662+
const previewBody = record(
663+
record(previewData, "preview response")["data"],
664+
"preview response data",
665+
);
666+
// The preview is a STATIC read of the already-committed source
667+
// (never install/probe/gate/freeze, see VENDORED.md) — asserted
668+
// unconditionally, not as a fallback branch: the vendored delta
669+
// that would have made this a real no-freeze probe was reverted.
670+
stringField(previewBody, "commitSha", "preview response");
671+
stringField(previewBody, "entry", "preview response");
672+
stringField(previewBody, "packageName", "preview response");
673+
if (!Array.isArray(previewBody["files"])) {
674+
throw new Error(
675+
`preview response missing a files array: ${JSON.stringify(previewData)}`,
676+
);
677+
}
678+
if (!Array.isArray(previewBody["toolPackagePins"])) {
679+
throw new Error(
680+
`preview response missing a toolPackagePins array: ${JSON.stringify(previewData)}`,
682681
);
683-
stringField(previewBody, "wireHash", "preview response");
684-
if (!Array.isArray(previewBody["grants"])) {
685-
throw new Error(
686-
`preview response missing a grants array: ${JSON.stringify(previewData)}`,
687-
);
688-
}
689682
}
690683

691684
const notYetTargets = await api(
@@ -748,40 +741,6 @@ describe.skipIf(databaseUrl === undefined)(
748741
await Bun.sleep(200);
749742
}
750743

751-
const wrongHash = await fetch(
752-
`${hub.baseUrl}/api/workflow-workflow-authoring/${assetId}/deploy`,
753-
{
754-
method: "POST",
755-
headers: runHeaders,
756-
body: JSON.stringify({
757-
commitSha,
758-
entry: WORKFLOW_SOURCE_ENTRY,
759-
expectedWireHash: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
760-
}),
761-
},
762-
);
763-
const wrongHashData: unknown = await wrongHash.json();
764-
if (wrongHash.status !== 409) {
765-
throw new Error(
766-
`redeploy with a wrong expectedWireHash: expected 409, got ` +
767-
`${wrongHash.status}: ${JSON.stringify(wrongHashData)}`,
768-
);
769-
}
770-
const wrongHashError = record(
771-
wrongHashData,
772-
"wrong-expectedWireHash response",
773-
)["error"];
774-
const errorCode = record(
775-
wrongHashError,
776-
"wrong-expectedWireHash error",
777-
)["code"];
778-
if (errorCode !== "wire_hash_mismatch") {
779-
throw new Error(
780-
`redeploy with a wrong expectedWireHash: expected code ` +
781-
`"wire_hash_mismatch", got ${JSON.stringify(errorCode)}`,
782-
);
783-
}
784-
785744
return { assetId, principalId };
786745
},
787746
).then((result) => result.assetId);
@@ -869,8 +828,8 @@ describe.skipIf(databaseUrl === undefined)(
869828
console.log(
870829
"routine-alignment: gate achieved: routine target discovery, " +
871830
"retarget (success and fail-closed), agent-authored author/" +
872-
"preview/deploy, deploy wire-hash fail-closed, agent-cannot-" +
873-
"approve, and the workflow detail route all hold end to end.",
831+
"preview/deploy, agent-cannot-approve, and the workflow detail " +
832+
"route all hold end to end.",
874833
);
875834
}, 240_000);
876835
},

0 commit comments

Comments
 (0)