diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-review-visible.test.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-review-visible.test.ts
new file mode 100644
index 00000000000..daa0f6f1484
--- /dev/null
+++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-review-visible.test.ts
@@ -0,0 +1,25 @@
+// @vitest-environment jsdom
+import { afterEach, beforeEach, expect, it, vi } from "vitest";
+import { readVisibleSkillReview } from "./integrator-skill-update.js";
+const input = { runId: "run", humanCommentId: "human" };
+const thread = (body: string) => ``;
+const response = '
/);
+});
+it("rejects hidden, foreign, old and ambiguously anchored conversation surfaces", () => {
+ for (const body of [response.replace('run:turn:0', 'old:turn:0'), response.replace(' {
expect(gradeIntegratorSkillUpdate(changed).checks.find(check => check.id === "review-diff-before-update")?.passed).toBe(false);
});
+ it("admits only a visible committed current-run review alongside the shorter final comment", () => {
+ const value = valid(bytes.old, bytes.next);
+ const first = value.state.reviewCheckpoint as typeof value.state.reviewCheckpoint & { visibleReview?: unknown };
+ Object.assign(first.state, { sourceIssueId: "source" });
+ const summary = first.reply;
+ const run = { id: "review-run", runtimeMode: "native", companyId: "company", agentId: "ceo", status: "succeeded", nativeIssueId: "source",
+ contextSnapshot: { issueId: "source", commentId: "human" }, resultJson: { summary, finalizationPhase: "committed",
+ finalizationReasonCode: "conversation_turn_finished", prpRunTerminalState: "succeeded", prpTurnTerminalState: "completed",
+ nativeResult: { schema: "paperclip.run_result.v1", reportedWorkDisposition: "done", summary } } };
+ Object.assign(first, { runs: [run], reply: "The complete diff is shown above. No update applied yet." });
+ value.reply = `${first.reply}\nI updated the accessibility skill to the reviewed source version. The local copy is unchanged.`;
+ const check = () => gradeIntegratorSkillUpdate(value).checks.find(row => row.id === "review-diff-before-update")?.passed;
+ expect(check()).toBe(false); // An API-only summary never substitutes for visible review.
+ first.visibleReview = { runId: run.id, issueId: "source", humanCommentId: "human", summary, visibleText: summary };
+ expect(check()).toBe(true);
+ expect(gradeIntegratorSkillUpdate(value).passed).toBe(true); // Raw final-comment prefix still proves later apply.
+ for (const patch of [{ runId: "old" }, { issueId: "foreign" }, { humanCommentId: "old-human" }, { summary: "foreign" }, { visibleText: "" }, { visibleText: "Installed 0.4; latest 0.5 exists." }]) {
+ const original = first.visibleReview; first.visibleReview = { ...original as object, ...patch };
+ expect(check()).toBe(false); first.visibleReview = original;
+ }
+ for (const patch of [{ companyId: "foreign" }, { agentId: "foreign" }, { status: "failed" }, { nativeIssueId: "foreign" }, { contextSnapshot: { issueId: "source", commentId: "other" } }, { resultJson: { ...run.resultJson, finalizationPhase: "pending" } }]) {
+ const original = first.runs; Object.assign(first, { runs: [{ ...run, ...patch }] });
+ expect(check()).toBe(false); first.runs = original;
+ }
+ const originalVersion = first.upstream.currentVersionId; first.upstream.currentVersionId = "premature";
+ expect(check()).toBe(false); first.upstream.currentVersionId = originalVersion;
+ expect(acceptedSkillReviewRun(first, "foreign")).toBeUndefined();
+ });
+
it("rejects premature apply, listing-only review, stale installation and overwritten local work", () => {
const early = valid(bytes.old, bytes.next); early.state.reviewCheckpoint.upstream.currentVersionId = "new"; expect(gradeIntegratorSkillUpdate(early).passed).toBe(false);
const listing = valid(bytes.old, bytes.next); listing.state.reviewCheckpoint.reply = "Current installed 0.4. Latest 0.5 is available."; expect(gradeIntegratorSkillUpdate(listing).passed).toBe(false);
diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts
index 8a69d9f9b2d..f53ef111d31 100644
--- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts
+++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts
@@ -16,7 +16,46 @@ import { accessibilityUpdatePins } from "./integrator-skill-pins.js";
export { accessibilityUpdatePins } from "./integrator-skill-pins.js";
const hash = (text: string) => createHash("sha256").update(text).digest("hex");
const root = (ctx: ChatCatalogContext) => `/api/companies/${ctx.input.fixtures.company.id}`;
-interface UpdateObservation extends InternalObservation { upstream: CompanySkillDetail; localFork: CompanySkillDetail; upstreamVersions: CompanySkillVersion[]; forkVersions: CompanySkillVersion[]; source: SkillSource; updateStatus: Pick }
+interface VisibleSkillReview { runId: string; issueId: string; humanCommentId: string; summary: string; visibleText: string }
+interface UpdateObservation extends InternalObservation { visibleReview?: VisibleSkillReview; upstream: CompanySkillDetail; localFork: CompanySkillDetail; upstreamVersions: CompanySkillVersion[]; forkVersions: CompanySkillVersion[]; source: SkillSource; updateStatus: Pick }
+// Only a committed current native completion can contribute rendered review text.
+export function acceptedSkillReviewRun(o: InternalObservation, issueId: string) {
+ if (o.runs.length !== 1) return undefined;
+ const run = o.runs[0]!, result = run.resultJson, native = result?.nativeResult as Record | undefined;
+ return run.companyId === o.companyId && run.agentId === o.leadId && run.status === "succeeded" && run.runtimeMode === "native"
+ && run.contextSnapshot?.issueId === issueId && (run.nativeIssueId == null || run.nativeIssueId === issueId)
+ && typeof run.contextSnapshot?.commentId === "string"
+ && result?.finalizationPhase === "committed" && result.finalizationReasonCode === "conversation_turn_finished"
+ && result.prpRunTerminalState === "succeeded" && result.prpTurnTerminalState === "completed"
+ && native?.schema === "paperclip.run_result.v1" && native.reportedWorkDisposition === "done"
+ && typeof result.summary === "string" && result.summary.trim() && native.summary === result.summary ? run : undefined;
+}
+export function readVisibleSkillReview(input: { runId: string; humanCommentId: string }) {
+ const visible = (node: Element) => {
+ for (let element: Element | null = node; element; element = element.parentElement) {
+ const style = window.getComputedStyle(element);
+ if (element.hasAttribute("hidden") || element.hasAttribute("inert") || element.getAttribute("aria-hidden") === "true"
+ || style.display === "none" || style.visibility === "hidden" || style.opacity === "0") return false;
+ }
+ return node.getClientRects().length > 0;
+ };
+ const selector = '[data-testid="task-chat-thread"], [data-testid="thread-root"]';
+ const roots = [...document.querySelectorAll(selector)].filter(root => visible(root)
+ && [...root.querySelectorAll("[id]")].some(node => node.id === `comment-${input.humanCommentId}` && node.closest(selector) === root));
+ if (roots.length !== 1) return "";
+ return [...roots[0]!.querySelectorAll("[data-thread-anchor]")].filter(node => {
+ const anchor = node.getAttribute("data-thread-anchor");
+ return node.closest(selector) === roots[0] && (anchor === `${input.runId}:turn` || anchor?.startsWith(`${input.runId}:turn:`));
+ }).flatMap(turn => [...turn.querySelectorAll('[data-testid="task-chat-final-response"]')]
+ .filter(node => visible(node)).map(node => (node as HTMLElement).innerText)).join("\n");
+}
+export function skillReviewPreviewText(o: UpdateObservation) {
+ const evidence = o.visibleReview, issueId = o.state.sourceIssueId;
+ if (!evidence || typeof issueId !== "string") return o.reply;
+ const run = acceptedSkillReviewRun(o, issueId);
+ return run && evidence.runId === run.id && evidence.issueId === issueId && evidence.humanCommentId === run.contextSnapshot?.commentId
+ && evidence.summary === run.resultJson?.summary && evidence.visibleText.trim() ? `${o.reply}\n${evidence.visibleText}` : o.reply;
+}
const updateReceipts = new WeakMap();
async function observeUpdate(ctx: ChatCatalogContext, state: Record, checkpoint = false): Promise {
const base = await observeInternal(ctx, state); const api = ctx.input.api; const upstream = state.upstream as CompanySkillDetail; const local = state.localFork as CompanySkillDetail;
@@ -34,7 +73,18 @@ async function observeUpdate(ctx: ChatCatalogContext, state: Record(ctx.input.fixtures.company.id, upstream.id);
updateReceipts.set(ctx, { localDigest, status: { ...status } });
- return { ...base, ...saved, updateStatus: status };
+ let visibleReview: VisibleSkillReview | undefined;
+ const issueId = ctx.issue()?.id;
+ if (checkpoint && issueId) {
+ state.sourceIssueId = issueId;
+ const run = acceptedSkillReviewRun(base, issueId);
+ if (run) {
+ const humanCommentId = run.contextSnapshot!.commentId as string;
+ const visibleText = await ctx.input.page.evaluate(readVisibleSkillReview, { runId: run.id, humanCommentId });
+ if (visibleText.trim()) visibleReview = { runId: run.id, issueId, humanCommentId, summary: run.resultJson!.summary as string, visibleText };
+ }
+ }
+ return { ...base, ...saved, updateStatus: status, ...(visibleReview ? { visibleReview } : {}) };
}
function updateObservation(value: unknown): UpdateObservation | undefined { const o = value as UpdateObservation; return observation(value) && !!o.upstream && !!o.localFork && Array.isArray(o.upstreamVersions) && Array.isArray(o.forkVersions) && !!o.source && !!o.updateStatus ? o : undefined; }
function substantiveDiff(reply: string) {
@@ -50,11 +100,12 @@ function claimsAppliedUpdate(reply: string) {
}
function previewCorrect(o: UpdateObservation) {
const before = o.state.upstream as CompanySkillDetail;
+ const reply = skillReviewPreviewText(o);
return o.upstream.currentVersionId === before.currentVersionId && o.upstream.sourceRef === accessibilityUpdatePins.oldCommit && hash(o.upstream.markdown) === accessibilityUpdatePins.oldSha256
&& o.source.lastScanCommit === accessibilityUpdatePins.oldCommit && o.source.trackingRef === accessibilityUpdatePins.newCommit && o.source.automaticUpdates === false
&& o.updateStatus.supported && o.updateStatus.hasUpdate && o.updateStatus.currentRef === accessibilityUpdatePins.oldCommit && o.updateStatus.latestRef === accessibilityUpdatePins.newCommit
- && (o.reply.includes(accessibilityUpdatePins.oldCommit.slice(0, 7)) || /(?:installed|current|currently)[^\n]{0,80}\b0\.4\b|\b0\.4\b[^\n]{0,80}(?:installed|current)/i.test(o.reply))
- && substantiveDiff(o.reply) && !claimsAppliedUpdate(o.reply);
+ && (reply.includes(accessibilityUpdatePins.oldCommit.slice(0, 7)) || /(?:installed|current|currently)[^\n]{0,80}\b0\.4\b|\b0\.4\b[^\n]{0,80}(?:installed|current)/i.test(reply))
+ && substantiveDiff(reply) && !claimsAppliedUpdate(reply);
}
export function gradeIntegratorSkillUpdate(value: unknown) {
const c = checks(); const o = updateObservation(value); c.add("real-tracked-source-and-saved-versions", !!o, "Requires actual public GitHub source tracking, installed snapshot versions and preserved editable fork."); if (!o) return c.result();