diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c3f98f13..a738ef29 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,4 +1,4 @@ -# Copilot instructions for shopdb-flask +, # Copilot instructions for shopdb-flask Follow these when suggesting code. They are enforced by a naming/style hook and by CI (`.github/workflows/ci.yml`) - suggestions that break them fail the build. @@ -7,11 +7,12 @@ by CI (`.github/workflows/ci.yml`) - suggestions that break them fail the build. ## General Guidelines - Finish the in-flight task before starting new workstreams. - When asked to "take notes" on a future idea, capture it in the notes file only - do not implement it, and do not expand on it at length in replies. -- Use a temp file for multi-line git commit messages instead of heredoc syntax: `git commit -F `. When committing in PowerShell, never pass multi-line commit messages inline (no here-strings like @'...'@ or multiple -m flags). PowerShell mangles them and git errors with "did not match any file(s) known to git". Instead, write the commit message to a temp file in the WORKSPACE ROOT (e.g. `commitmsg.tmp`), run `git commit -F commitmsg.tmp`, then delete the file. Do NOT place the temp file inside .git/ - git prunes stray files there and the file disappears before commit runs. +- Use a temp file for multi-line git commit messages instead of heredoc syntax: `git commit -F `. When committing in PowerShell, always write the message to a temp file in the WORKSPACE ROOT (e.g. `commitmsg.tmp`), run `git commit -F commitmsg.tmp`, then delete the file. Passing a multi-line here-string to `git commit -m` silently no-ops: it reports success but does not create the commit. Always verify with `git log` after committing. Do NOT place the temp file inside .git/ - git prunes stray files there and the file disappears before commit runs. - Avoid using `powershell.exe -File` for launching scripts, as it fails if the script file is missing or quarantined. - Long-running commands (pytest suites, npm builds, container rolls) must be redirected to a log file under `devnotes/` (e.g. `devnotes/_pytest.log`) and launched non-blocking; never run as a blocking foreground command. Do not poll or tail the output; the user watches the file and will say when it is done. - Before taking any action that modifies code or files, warn the user first if a test run or build may be in progress, so discussion about the project can continue without altering files underneath a running test. - Always invoke shell scripts with the explicit Git Bash path: `& "C:\Program Files\Git\bin\bash.exe" scripts/ + + diff --git a/plugins/endpoints/frontend/views/systemScriptsSettings.spec.js b/plugins/endpoints/frontend/views/systemScriptsSettings.spec.js new file mode 100644 index 00000000..d86dde65 --- /dev/null +++ b/plugins/endpoints/frontend/views/systemScriptsSettings.spec.js @@ -0,0 +1,124 @@ +/** + * System Scripts settings page. + * + * Two things are worth pinning here and nothing else is: + * + * 1. The page must NOT offer a run context. Scripts run as SYSTEM because the + * agent is a service with no user session; exposing a picker would let + * someone author a script that reads as "run as the logged-on user" and + * silently does not. + * 2. The SYSTEM warning must be present on both the list and the add/edit + * form. Someone pasting a script into the modal may never have read the + * page copy above it. + * + * Asserted against the file text, matching configurationPolicyEditable.spec.js. + */ +import { describe, it, expect } from 'vitest' +import { readFileSync } from 'fs' +import { join, dirname } from 'path' +import { fileURLToPath } from 'url' + +const here = dirname(fileURLToPath(import.meta.url)) +const source = readFileSync(join(here, 'SystemScriptsSettings.vue'), 'utf8') +const routes = readFileSync(join(here, '..', 'routes.js'), 'utf8') + +describe('SystemScriptsSettings', () => { + it('warns that scripts run as SYSTEM on the list page and in the form', () => { + const warnings = source.match(/runs? as SYSTEM/g) || [] + expect(warnings.length).toBeGreaterThanOrEqual(2) + }) + + it('does not offer a run context picker', () => { + expect(source).not.toMatch(/runcontext/i) + }) + + it('does not send a runcontext in its save payload', () => { + const payload = source.slice(source.indexOf('const payload = {')) + expect(payload.slice(0, 200)).not.toMatch(/runcontext/i) + }) + + it('flags a script with no body as unrunnable', () => { + expect(source).toMatch(/hasbody/) + expect(source).toMatch(/cannot run/i) + }) + + it('is gated on the standalone script permission, not admin', () => { + const route = routes.slice(routes.indexOf("'settings/endpoints/scripts'")) + expect(route).toMatch(/requiresPermission: 'endpoints\.runscript'/) + expect(route.slice(0, 300)).not.toMatch(/requiresAdmin/) + }) + + it('offers an optional working directory and sends it on save', () => { + expect(source).toMatch(/form\.workingdirectory/) + const payload = source.slice(source.indexOf('const payload = {')) + expect(payload.slice(0, 300)).toMatch(/workingdirectory/) + }) + + it('shows the staging default rather than an empty cell', () => { + // A blank cell reads as "unknown". The folder is the difference between a + // corporate tool being found and the script failing further in, so the + // list has to state the default out loud. + expect(source).toMatch(/Agent staging folder/) + }) + + it('offers an error mode and sends it on save', () => { + expect(source).toMatch(/form\.errormode/) + const payload = source.slice(source.indexOf('const payload = {')) + expect(payload.slice(0, 300)).toMatch(/errormode/) + }) + + it('defaults a new script to the permissive mode', () => { + // Strict changes how a body behaves partway through. Defaulting to it + // would silently alter every script authored from here on, so a new + // script has to start where the old ones already are. + const empty = source.slice(source.indexOf('function emptyForm')) + expect(empty.slice(0, 300)).toMatch(/errormode: 'default'/) + }) + + it('names both error modes by their consequence, not by their flag', () => { + // "default" and "strict" mean nothing to the person choosing. What they + // need to know is which one can report success having done nothing. + expect(source).toMatch(/Continue - report success if the script reaches the end/) + expect(source).toMatch(/Stop on the first error and fail the command/) + }) + + it('explains the non-terminating error trap that justifies the choice', () => { + // The whole feature exists because PowerShell exits zero after an error. + // If the page never says so, the dropdown looks like pedantry and stays + // on the setting that produces false green. + expect(source).toMatch(/non-terminating/i) + expect(source).toMatch(/exits? zero/i) + }) + + it('offers the explainer collapsed so it does not bury the page', () => { + expect(source).toMatch(/
/) + expect(source).toMatch(/How does this work\?/) + expect(source).not.toMatch(/
/) + }) + + it('collapses the long per-field prose in the form', () => { + // The modal is already tall enough to scroll. Three paragraphs of standing + // explanation between the inputs pushes Save off screen and gets skimmed + // past, which is the opposite of what the copy is for. + const helps = source.match(/
/g) || [] + expect(helps.length).toBeGreaterThanOrEqual(2) + expect(source).toMatch(/How this works<\/summary>/) + }) + + it('styles itself from defined theme tokens only', () => { + // --surface and --text-primary are not defined by this app's theme, so a + // var() fallback silently rendered a white panel on the dark background. + // A fallback colour is the bug: it looks correct in light mode and only + // shows up as a bright block for dark-mode users. + const styles = source.slice(source.indexOf('