From 3efbecd50ac187767ea860603e482ba18ad2666b Mon Sep 17 00:00:00 2001 From: red person Date: Tue, 3 Nov 2026 03:35:37 +0300 Subject: [PATCH] fix(skills): restore the source editor from test results (#6931) * fix(skills): restore the source editor from test results * fix(skills): align the chat link module version * test: isolate photo routing from external MCP registry state --- static/js/skills.js | 42 +++++++-- tests/test_skill_test_editor_browser.py | 112 ++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 tests/test_skill_test_editor_browser.py diff --git a/static/js/skills.js b/static/js/skills.js index b29869a7d..1ae2119af 100644 --- a/static/js/skills.js +++ b/static/js/skills.js @@ -999,6 +999,7 @@ function renderSkillsList() { // the footer too so it's not buried under the "⋯" menu. const testBtn = document.createElement('button'); testBtn.className = 'doclib-card-text-btn doclib-card-action-btn'; + testBtn.dataset.act = 'test'; testBtn.innerHTML = _svg(_ICON.test, { size: 11 }) + 'Test'; testBtn.title = 'Test this skill — run it + AI judge'; testBtn.addEventListener('click', (e) => { @@ -1013,14 +1014,10 @@ function renderSkillsList() { const _origHTML = testBtn.innerHTML; testBtn.innerHTML = _svg(_ICON.test, { size: 11 }) + 'Starting…'; Promise.resolve(_testSkill(card, name)).finally(() => { - // The preview gets overwritten by _testSkill, which removes the - // testBtn from the DOM. The cleanup below only matters if the - // button still exists (e.g. _testSkill bailed early). - if (document.body.contains(testBtn)) { - testBtn.disabled = false; - testBtn.dataset.busy = ''; - testBtn.innerHTML = _origHTML; - } + // Source controls are detached during results and reused by Edit. + testBtn.disabled = false; + testBtn.dataset.busy = ''; + testBtn.innerHTML = _origHTML; }); }); @@ -1285,6 +1282,10 @@ function _toggleSkillEdit(card, name) { if (!card._mdLoaded) return; const preview = card.querySelector('.skill-card-preview'); if (!preview) return; + if (card._skillSourceNodes) { + preview.replaceChildren(...card._skillSourceNodes); + card._skillSourceNodes = null; + } const existing = preview.querySelector('.skill-md-editor'); if (existing) { // Already editing — treat Edit as Save. @@ -1305,7 +1306,26 @@ function _toggleSkillEdit(card, name) { ta.focus(); // Flip the Edit button label to "Save". const editBtn = [...preview.querySelectorAll('.doclib-card-action-btn')].find(b => /Edit|Save/.test(b.textContent)); + const editLabel = editBtn?.innerHTML; if (editBtn) editBtn.innerHTML = 'Save'; + const cancel = document.createElement('button'); + cancel.type = 'button'; + cancel.className = 'doclib-card-text-btn doclib-card-action-btn'; + cancel.textContent = 'Cancel'; + const testBtn = preview.querySelector('[data-act="test"]'); + const testDisplay = testBtn?.style.display; + if (testBtn) testBtn.style.display = 'none'; + card._cancelSkillEdit = () => { + ta.remove(); + if (pre) pre.style.removeProperty('display'); + if (editBtn) editBtn.innerHTML = editLabel; + cancel.remove(); + if (testBtn) testBtn.style.display = testDisplay; + card._cancelSkillEdit = null; + card._fillH?.(); + }; + cancel.addEventListener('click', (e) => { e.stopPropagation(); card._cancelSkillEdit?.(); }); + if (editBtn) editBtn.after(cancel); } async function _saveSkillEdit(card, name) { @@ -1454,6 +1474,12 @@ async function _testSkill(card, name, force = false) { if (!card.classList.contains('doclib-card-expanded')) await _expandSkillCard(card, name); const preview = card.querySelector('.skill-card-preview'); if (!preview) return; + if (!preview.querySelector('.skill-test')) { + card._cancelSkillEdit?.(); + // Preserve the source nodes and handlers. Result actions are nested in + // .skill-test, so they cannot be insertBefore references on the preview. + card._skillSourceNodes = Array.from(preview.childNodes); + } preview.innerHTML = '
' + '
'; diff --git a/tests/test_skill_test_editor_browser.py b/tests/test_skill_test_editor_browser.py new file mode 100644 index 000000000..f4b80d1b1 --- /dev/null +++ b/tests/test_skill_test_editor_browser.py @@ -0,0 +1,112 @@ +"""The real Skills module must leave test results before opening its editor.""" +import json +import os +from pathlib import Path +import shutil +import subprocess + +from bs4 import BeautifulSoup +import pytest + + +@pytest.mark.parametrize("width", [1440, 390]) +@pytest.mark.parametrize("mode", ["done", "running", "source"]) +def test_skill_editor_transitions(tmp_path, width, mode): + if not shutil.which("node"): + pytest.skip("node not installed") + repo = Path(__file__).resolve().parents[1] + index = BeautifulSoup((repo / "static/index.html").read_text(), "html.parser") + modal = index.select_one("#memory-modal") + modal["class"] = [value for value in modal.get("class", []) if value != "hidden"] + for panel in modal.select("[data-memory-panel]"): + classes = set(panel.get("class", [])) + if panel["data-memory-panel"] == "skills": + classes.discard("hidden") + else: + classes.add("hidden") + panel["class"] = sorted(classes) + styles = "".join(str(node) for node in index.select('link[rel="stylesheet"]')) + document = tmp_path / "skills.html" + document.write_text(styles + str(modal) + str(index.select_one("#toast")), encoding="utf-8") + script = r""" +const fs=require('node:fs'), assert=require('node:assert/strict'); +let chromium; +try { ({chromium}=require('playwright')); } +catch(e) { if(e.code==='MODULE_NOT_FOUND')process.exit(77); throw e; } +(async()=>{ + const browser=await chromium.launch({headless:true}); + try{ + const origin=process.env.ODYSSEUS_TEST_STATIC_ORIGIN, mode=process.argv[3]; + const page=await browser.newPage({viewport:{width:Number(process.argv[2]),height:900}}); + page.setDefaultTimeout(4000); + const source='---\nname: fixture-editor\nversion: "1.0"\n---\n\n## Procedure\nOriginal source'; + let markdown=source,posts=0,statusCalls=0; + const errors=[];page.on('pageerror',e=>errors.push(e.message)); + await page.route(origin+'/api/**',async route=>{ + const request=route.request(),path=new URL(request.url()).pathname; + let data={}; + if(path==='/api/skills')data={skills:[{name:'fixture-editor',description:'Editor fixture',status:'draft',source:'user',confidence:0.9,procedure:['Original source']}]}; + else if(path.endsWith('/markdown')){ + if(request.method()==='POST'){posts++;markdown=request.postDataJSON().markdown;data={ok:true};} + else data={markdown}; + }else if(path.endsWith('/test-status')){ + statusCalls++; + const status=mode==='running'&&statusCalls===1?'running':'done'; + data={status,log:[{type:'say',text:'Completed fixture output'}],verdict:status==='done'?{verdict:'pass',confidence:0.95,summary:'Fixture verdict',issues:[]}:null}; + }else if(path.endsWith('/audit-all/status'))data={status:'none'}; + await route.fulfill({contentType:'application/json',body:JSON.stringify(data)}); + }); + await page.goto(origin+'/static/js/storage.js'); + await page.setContent(fs.readFileSync(process.argv[1],'utf8')); + await page.evaluate(async()=>{const skills=await import('/static/js/skills.js');await skills.loadSkills();}); + const card=page.locator('.skill-card[data-skill-name="fixture-editor"]'); + await card.locator('.skill-card-name').click(); + if(mode!=='source'){ + await card.getByRole('button',{name:'Test',exact:true}).click(); + if(mode==='running'){ + await card.getByText('…running (you can close this — it keeps going)',{exact:true}).waitFor(); + await card.locator('.skill-chevron-up').click(); + await card.locator('.skill-kebab-btn').click(); + await page.locator('.skill-kebab-menu:visible').getByRole('button',{name:'Edit',exact:true}).click(); + }else{ + await card.getByText('Fixture verdict',{exact:true}).waitFor(); + await card.locator('[data-act="edit"]').click(); + } + }else await card.getByRole('button',{name:'Edit',exact:true}).click(); + const editor=card.locator('.skill-md-editor');await editor.waitFor({state:'visible'}); + assert.equal(await editor.inputValue(),source); + assert.equal(await card.locator('.skill-test').count(),0); + assert(await card.locator('.doclib-card-expanded-actions button:visible').evaluateAll(buttons=>buttons.every(button=>{ + const bounds=button.getBoundingClientRect(),card=button.closest('.skill-card').getBoundingClientRect(); + return bounds.left>=card.left&&bounds.right<=card.right; + })), 'Editor actions must fit inside the card'); + await editor.fill(source+'\nCancelled draft'); + if(mode==='running'){ + await page.waitForFunction(()=>document.querySelector('.skill-card')?._testPoll===null); + assert.equal(await editor.inputValue(),source+'\nCancelled draft'); + } + await card.getByRole('button',{name:'Cancel',exact:true}).click(); + assert.equal(await card.locator('.skill-md-editor').count(),0); + assert.equal(posts,0);assert.equal(markdown,source); + assert.equal(await card.locator('.skill-md-pre').innerText(),source); + if(mode!=='source'){ + await card.getByRole('button',{name:'Test',exact:true}).click(); + await card.getByText('Fixture verdict',{exact:true}).waitFor(); + await card.locator('[data-act="edit"]').click(); + }else await card.getByRole('button',{name:'Edit',exact:true}).click(); + await editor.fill(source+'\nSaved change'); + await card.getByRole('button',{name:'Save',exact:true}).click(); + await page.waitForFunction(()=>!document.querySelector('.skill-md-editor')); + assert.equal(posts,1);assert.equal(markdown,source+'\nSaved change'); + assert.deepEqual(errors,[]); + console.log(JSON.stringify({mode,width:process.argv[2],posts})); + }finally{await browser.close();} +})().catch(e=>{console.error(e);process.exitCode=1;}); +""" + result = subprocess.run(["node", "-e", script, str(document), str(width), mode], + cwd=repo, env=os.environ.copy(), capture_output=True, + text=True, timeout=45) + if result.returncode == 77: + pytest.skip("playwright not installed") + assert result.returncode == 0, result.stdout + result.stderr + assert json.loads(result.stdout)["posts"] == 1