mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(evals): compose accepted skill recovery and secret route corrections
Owning-file deltas fromd80e0474,f1769b34, and16f4fa6b against frozen2ec; preserve existing unrun oracle versions. Co-Authored-By: Paperclip <paperclip@paperclip.ing>
This commit is contained in:
1 parent
2ec214914c
commit
91310507eb
6 files changed
+89
-9
No files matched your search
@@ -0,0 +1,21 @@
|
||||
import {expect,it} from "vitest";
|
||||
import {chromium} from "@playwright/test";
|
||||
import {companySecretSetupURL} from "./external-company-secret.js";
|
||||
const origin="https://paperclip.example",route="/RUN/company/settings/secrets";
|
||||
const body="I can't enter the key value myself — company secret values are board-only secure entry, and I won't ask you to paste a secret into chat.\n\nTo add it:\n1. Open `/RUN/company/settings/secrets`.\n2. Click **New secret**.\n3. Use name e.g. **Deployment API key**, env key **DEPLOYMENT_API_KEY**, and enter the deployment key value there.\n4. Save it, then tell me the name/key you used (not the value).\n\nOnce it exists I'll add a `DEPLOYMENT_API_KEY` secret reference to Morgan's agent environment and verify the reference is in place. Saving the secret only stores it; Morgan gets it on its next run after the binding.";
|
||||
it("admits the actual4587 displayed inline route and rejects absent, foreign, cross-company or hidden routes in offline Chromium",async()=>{
|
||||
const browser=await chromium.launch({headless:true}),page=await browser.newPage();
|
||||
const render=async(text:string,html?:string,id="native")=>{await page.setContent(`<article id="comment-${id}"><p>Use secure Settings Secrets for Morgan's deployment credential environment binding.</p>${html??`<code>${text}</code>`}</article>`);};
|
||||
try{
|
||||
await render(route);expect(await companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).toBe(origin+route);
|
||||
await render(route,`<a href="${origin+route}">Open Secrets</a>`);expect(await companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).toBe(origin+route);
|
||||
for(const target of ["/OTHER/company/settings/secrets","https://foreign.example"+route,"/api/companies/company/secrets"]){await render(target);expect(await companySecretSetupURL(page,{body:body.replace(route,target),commentId:"native"},origin,"RUN",1000)).toBeUndefined();}
|
||||
await render(route,`<a href="https://foreign.example${route}">${route}</a>`);expect(await companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).toBeUndefined();
|
||||
await render("No route");expect(await companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).toBeUndefined();
|
||||
for(const html of [`<code hidden>${route}</code>`,`<code style="opacity:0">${route}</code>`,`<span style="visibility:hidden"><code>${route}</code></span>`]){await render(route,html);expect(await companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).toBeUndefined();}
|
||||
await render(route,undefined,"other-actor");await expect(companySecretSetupURL(page,{body,commentId:"native"},origin,"RUN",1000)).rejects.toThrow();
|
||||
await render(route);expect(await companySecretSetupURL(page,{body:body.replace(route,"missing"),commentId:"native"},origin,"RUN",1000)).toBeUndefined();
|
||||
expect(await companySecretSetupURL(page,{body:`Open ${route}`,commentId:"native"},origin,"RUN",1000)).toBeUndefined();
|
||||
expect(await companySecretSetupURL(page,{body:body+" Paste the key here in chat.",commentId:"native"},origin,"RUN",1000)).toBeUndefined();
|
||||
}finally{await browser.close();}
|
||||
},30000);
|
||||
@@ -1,6 +1,6 @@
|
||||
import { chatCatalogNativePhaseRemainingBudget, chatCatalogOriginalPhaseDeadline } from "../chat-catalog-contract.js";
|
||||
import { randomBytes } from "node:crypto";
|
||||
import { expect } from "@playwright/test";
|
||||
import { expect, type Page } from "@playwright/test";
|
||||
import type { ChatCatalogContext } from "../chat-catalog-contract.js";
|
||||
import type { CompanySecret, CompanySecretUsageBinding } from "../../../packages/shared/src/types/secrets.js";
|
||||
import { definition,root,checks,externalObservation,observeExternal,verifyRenderedReply } from "./external-support.js";
|
||||
@@ -40,6 +40,20 @@ export function deploymentSecretGuidance(text:string){
|
||||
&&!text.split(/[.!;\n]+|\bbut\b/i).some(clause=>/\b(?:I(?:['’]ve| have)?|we(?:['’]ve| have)?)\s+(?:already\s+)?(?:added|created|bound|configured)\b/i.test(clause))
|
||||
&&!text.split(/[.!;\n]+/).some(clause=>/\b(?:bind|assign|add)\b[^.!;\n]{0,45}\b(?:all agents|every agent|CEO|Riley)\b/i.test(clause)&&!/(?:do not|don.t|never|avoid)\s+(?:bind|assign|add)/i.test(clause));
|
||||
}
|
||||
export async function companySecretSetupURL(page:Page,reply:{body:string;commentId?:string},origin:string,prefix:string,timeout:number){
|
||||
if(!reply.commentId||!deploymentSecretGuidance(reply.body))return;
|
||||
const comment=page.locator(`[id=${JSON.stringify(`comment-${reply.commentId}`)}]`);
|
||||
await comment.waitFor({state:"visible",timeout});
|
||||
const displayed=await comment.evaluate(root=>{
|
||||
const shown=(node:Element)=>{for(let element:Element|null=node;element;element=element.parentElement){const style=getComputedStyle(element);if(style.display==="none"||style.visibility==="hidden"||style.visibility==="collapse"||Number(style.opacity)===0)return false;}return true;};
|
||||
const walker=document.createTreeWalker(root,NodeFilter.SHOW_TEXT),parts:string[]=[];
|
||||
for(let node=walker.nextNode();node;node=walker.nextNode())if(node.parentElement&&shown(node.parentElement)&&!node.parentElement.closest("a[href]"))parts.push(node.textContent??"");
|
||||
return {text:parts.join("\n"),links:Array.from(root.querySelectorAll<HTMLAnchorElement>("a[href]")).filter(shown).map(node=>node.href)};
|
||||
},undefined,{timeout});
|
||||
const supported=(candidate:string)=>{try{const url=new URL(candidate,origin);return url.origin===origin&&url.pathname===`/${prefix}/company/settings/secrets`&&!url.username&&!url.password&&!url.search&&!url.hash?url.href:undefined;}catch{return;}};
|
||||
const routes=(text:string)=>(text.match(/https?:\/\/[^\s`<>"']+|(?<![\w/:])\/[^\s`<>"']+/g)??[]).map(value=>value.replace(/[.,;!?\])}]+$/,"")).map(supported).filter((value):value is string=>!!value);
|
||||
return routes(reply.body).find(route=>displayed.links.some(link=>supported(link)===route)||routes(displayed.text).includes(route));
|
||||
}
|
||||
export function companySecretQuestionHandoff(value:unknown,b:SecretQuestionBinding,status:"pending"|"answered"="pending"){
|
||||
const bound=boundSecretQuestion(value,b,status)??boundSecretQuestion(value,b,status,"company-environment-name");if(!bound)return;
|
||||
const {row,question:q,questionSet:qs,text,payloadHash}=bound;
|
||||
@@ -125,7 +139,9 @@ export const companySecretExternalCases=[definition("secrets.configure-company-s
|
||||
remaining();
|
||||
const beforeHuman=await ctx.input.api.get<CompanySecret[]>(`${root(ctx)}/secrets`);
|
||||
state.secretCountBeforeHuman=beforeHuman.length;if(!Array.isArray(state.controllerSecretBaseline)||beforeHuman.length!==state.controllerSecretBaseline.length||!companySecretControllersUnchanged(beforeHuman,state.controllerSecretBaseline))throw new Error("Secret catalog changed before authorized human entry");
|
||||
const path=`/${ctx.input.fixtures.company.issuePrefix}/company/settings/secrets`,href=initial.links.find(link=>{try{const url=new URL(link,initial.origin);return url.origin===initial.origin&&url.pathname===path;}catch{return false;}});if(!href&&!question)throw new Error("Actual reply must link the supported secret management surface");
|
||||
const prefix=ctx.input.fixtures.company.issuePrefix;if(!prefix)throw new Error("Company secret setup requires the current company route prefix");
|
||||
const path=`/${prefix}/company/settings/secrets`,href=commentHandoff?await companySecretSetupURL(ctx.input.page,reply!,initial.origin,prefix,remaining()):undefined;
|
||||
if(!href&&!question)throw new Error("Actual visible reply must direct the supported company secret management surface");
|
||||
const receipt=await privateBrowserEntry({mode:"company-secret-create",companyId:ctx.input.fixtures.company.id,baseURL:initial.origin,setupURL:new URL(href??path,initial.origin).href,providerURL:"",credential:values.get(ctx)!,cookies:await ctx.input.page.context().cookies(initial.origin)});
|
||||
Object.assign(state,receipt,{secureBrowserUntraced:receipt.processIsolation&&receipt.testRunnerAttached===false&&receipt.apiStepRecorder===false&&receipt.tracingEnabled===false&&receipt.videoEnabled===false,operatorChannel:"private-browser-company-secret-and-public-browser-binding"});
|
||||
const sourceURL=ctx.input.page.url();await ctx.input.page.goto(`/${ctx.input.fixtures.company.issuePrefix}/agents/${state.morganId}/configuration`,{waitUntil:"domcontentloaded"});
|
||||
|
||||
@@ -296,3 +296,21 @@ it("distinguishes the actual preserved local skill name from a contained catalog
|
||||
o.phaseReplies[0]!.body+=" Launch Checklist is now installed under another slug.";
|
||||
expect(gradeTeamConditions(o,"initial").passed).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects present unavailability of a verified recovered skill", () => {
|
||||
for (const state of ["available", "usable"]) {
|
||||
const r=recovered(); r.o.phaseReplies[1]!.body=`Launch Checklist is still not ${state}.`;
|
||||
expect(gradeTeamConditions(r.o).passed).toBe(false);
|
||||
}
|
||||
});
|
||||
it("rejects possessed and bare installation claims while the required catalog skill remains missing", () => {
|
||||
for(const claim of ["We have Launch Checklist installed.", "Launch Checklist installed under another slug."]) {
|
||||
const o=valid();o.phaseReplies[0]!.body="The existing CTO was preserved and the imported role is CTO 2. Launch Checklist's slug is occupied. "+claim;
|
||||
expect(gradeTeamConditions(o,"initial").passed).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("does not treat an earlier team installation as recovery of a named skill reported historically uninstalled", () => {
|
||||
const r=recovered();r.o.phaseReplies[0]!.body="The existing CTO was preserved and the imported role is CTO 2. The team installed successfully, but Launch Checklist was not installed.";
|
||||
expect(gradeTeamConditions(r.o,"initial").passed).toBe(false);
|
||||
});
|
||||
@@ -193,13 +193,21 @@ function negatedTeamRecovery(before: string): boolean {
|
||||
return /\b(?:not|never|isn.t|aren.t|wasn.t|weren.t|hasn.t|haven.t)\s+(?:(?:be|been|yet|currently|still|successfully|fully|completely)\s+){0,2}$/i.test(before);
|
||||
}
|
||||
function truthfulTeamRecovery(reply: string, skill: Pick<CatalogSkill, "slug" | "name">): boolean {
|
||||
return namedTeamSkillClauses(reply, skill).some(clause => [...clause.matchAll(/\b(?:installed|available|resolved|recovered)\b/gi)].some(match =>
|
||||
!negatedTeamRecovery(clause.slice(0, match.index))))
|
||||
const names = [skill.slug, skill.slug.replaceAll("-", " "), skill.name].map(name => name.toLowerCase());
|
||||
return namedTeamSkillClauses(reply, skill).some(clause => {
|
||||
const namedAt = names.map(name => clause.toLowerCase().indexOf(name)).filter(index => index >= 0);
|
||||
const firstName = namedAt.length ? Math.min(...namedAt) : -1;
|
||||
return [...clause.matchAll(/\b(?:installed|available|resolved|recovered)\b/gi)].some(match => {
|
||||
const targetAfter = clause.slice(match.index + match[0].length).replace(/^\s+(?:the\s+)?[*_`]*/i, "").toLowerCase();
|
||||
return (firstName < 0 || match.index >= firstName || match[0].toLowerCase() === "installed" && names.some(name => targetAfter.startsWith(name)))
|
||||
&& !negatedTeamRecovery(clause.slice(0, match.index));
|
||||
});
|
||||
})
|
||||
&& !claimsRecoveredSkillMissing(reply, skill);
|
||||
}
|
||||
function claimsRecoveredSkillMissing(reply: string, skill: Pick<CatalogSkill, "slug" | "name">): boolean {
|
||||
return namedTeamSkillClauses(reply, skill, true).some(clause =>
|
||||
[...clause.matchAll(/\b(?:not (?:(?:successfully|fully|completely)\s+){0,2}installed|uninstalled|missing|unavailable|blocked)\b/gi)].some(match => {
|
||||
[...clause.matchAll(/\b(?:not (?:(?:successfully|fully|completely)\s+){0,2}(?:installed|available|usable)|uninstalled|missing|unavailable|blocked)\b/gi)].some(match => {
|
||||
const before = clause.slice(0, match.index);
|
||||
return !/\b(?:was|were|had been)\s+(?:(?:previously|initially|formerly|earlier|still)\s+){0,2}$/i.test(before)
|
||||
&& (match[0].toLowerCase().startsWith("not ") || /\b(?:is|are|remains?|still)\s+(?:currently\s+)?$/i.test(before));
|
||||
@@ -272,7 +280,7 @@ export function truthfulTeamPartial(
|
||||
});
|
||||
const subjects = [skill.slug, skill.slug.replaceAll("-", " "), skill.name]
|
||||
.map(name => name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")).join("|");
|
||||
const currentRecovery = new RegExp("(?:\\b(?:" + subjects + ")[*_`]*|^(?:it|(?:this|that) skill))\\s+(?:skill\\s+)?(?:is|has been)\\s+(?:(?:now|already|currently)\\s+)?(?:installed|available|resolved|recovered)\\b", "i");
|
||||
const currentRecovery = new RegExp("(?:\\b(?:" + subjects + ")[*_`]*|^(?:it|(?:this|that) skill))\\s+(?:skill\\s+)?(?:(?:is|has been)\\s+)?(?:(?:now|already|currently)\\s+)?(?:installed|available|resolved|recovered)\\b", "i");
|
||||
const installedObject = new RegExp("\\binstalled\\s+(?:the\\s+)?[*_`]*(?:" + subjects + ")\\b", "gi");
|
||||
return reported && !falseFull && !namedTeamSkillClauses(reply, skill, true).some(clause =>
|
||||
currentRecovery.test(clause) || [...clause.matchAll(installedObject)].some(claim => !negatedTeamRecovery(clause.slice(0, claim.index))));
|
||||
|
||||
@@ -100,3 +100,18 @@ it("rejects a universal execution restriction despite a separately stated viewpo
|
||||
it("keeps a quantified noncurrent artifact restriction separate from a universal review waiver", () => {
|
||||
expect(preservesWebsiteChecklist(`${websiteChecklist.join("\n")}; do not perform the check on any cached build.`)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects necessity waivers bound to the required viewport check", () => {
|
||||
for (const waiver of ["The 375px viewport check without horizontal scrolling is unnecessary.", "The 375px viewport check without horizontal scrolling is not necessary.", "There is no requirement to test a 375px viewport without horizontal scrolling."])
|
||||
expect(preservesWebsiteChecklist(websiteChecklist.slice(0, 3).join("\n") + "\n" + waiver)).toBe(false);
|
||||
});
|
||||
it("preserves the full viewport requirement beside cached production and comma-qualified restrictions", () => {
|
||||
const original=websiteChecklist.join("\n");
|
||||
expect(preservesWebsiteChecklist(`${original}; do not perform the check on a cached production build.`)).toBe(true);
|
||||
expect(preservesWebsiteChecklist(`${original.slice(0,-1)}, but do not perform the check on a cached build.`)).toBe(true);
|
||||
expect(preservesWebsiteChecklist(`${original}, not on any cached build using any browser.`)).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps a required viewport check beside a protective prohibition of horizontal scrolling", () => {
|
||||
expect(preservesWebsiteChecklist(`${websiteChecklist.join("\n")} Do not allow horizontal scrolling at the 375px viewport.`)).toBe(true);
|
||||
});
|
||||
@@ -19,17 +19,19 @@ function websiteChecklistChecks(markdown: string): boolean[] {
|
||||
const forbidden = /\b(?:skip|omit|ignore|don't|do not|need not|not required|optional)\b/i;
|
||||
const mobileRequirements = [/\b375\s*(?:px|pixels?)?\b/i, /\b(?:viewport|wide|width)\b/i, /(?:without horizontal (?:scrolling|overflow)|no horizontal (?:scrolling|overflow)|(?:must not|does not|doesn't) (?:scroll|overflow) horizontally|(?:confirm|check|verify)[^.!?]{0,70}whether[^.!?]{0,70}horizontal scrolling)/i];
|
||||
const mobileWaiver = (text: string) => {
|
||||
const clauses = text.split(/[.!?;\n]+/);
|
||||
const clauses = text.split(/[.!?;\n]+|,\s*(?:but\s+)?(?=(?:don't|do not|never|avoid|no need to)\b)/i);
|
||||
const affirmative = clauses.some(clause => mobileRequirements.every(pattern => pattern.test(clause))
|
||||
&& !/\b(?:don't|do not|never|avoid|no need to|need not|skip|omit|ignore|optional|not required)\b/i.test(clause));
|
||||
return /\b(?:skip|omit|ignore|need not|not required|optional)\b/i.test(text)
|
||||
|| clauses.some(clause => /\b(?:375\s*(?:px|pixels?)?|viewport|check|test|review)\b/i.test(clause)
|
||||
&& /\b(?:unnecessary|not necessary|no requirement to)\b/i.test(clause))
|
||||
|| clauses.some(clause => [...clause.matchAll(/\b(?:don't|do not|never|avoid|no need to)\s+/gi)].some(negation => {
|
||||
const action = clause.slice(negation.index + negation[0].length).trimStart();
|
||||
const target = /\b(?:375\s*(?:px|pixels?)?|viewport|check|test|review)\b/i.exec(action);
|
||||
const overflow = /\boverflow(?:ing)?\b/i.exec(action);
|
||||
const overflow = /\b(?:overflow(?:ing)?|horizontal scroll(?:ing)?|scroll(?:ing)? horizontally)\b/i.exec(action);
|
||||
const restriction = /\b(?:on|with|using|against)\s+(.+)$/i.exec(action)?.[1];
|
||||
const qualified = !!restriction
|
||||
&& /^(?:(?:a|an|the|any|all|every|this|that|these|those)\s+)?(?:cached|stale|outdated|previous|old|archived)\s+(?:build|version|snapshot|artifact|bundle)s?\b/i.test(restriction)
|
||||
&& /^(?:(?:a|an|the|any|all|every|this|that|these|those)\s+)?(?:cached|stale|outdated|previous|old|archived)\s+(?:production\s+)?(?:build|version|snapshot|artifact|bundle)s?\b/i.test(restriction)
|
||||
&& !/\b(?:current|live|selected|375\s*(?:px|pixels?)?|viewport|mobile|width)\b/i.test(restriction);
|
||||
return !!target && (!overflow || target.index < overflow.index) && !(affirmative && qualified);
|
||||
}));
|
||||
|
||||
Reference in new issue
Block a user