Add lp-task-names, a guard against unnamed task commands
Task titles come from one table whose final fallback returns the raw command
string, so a dispatched command with no matching row does not error — it just
renders as "libreportal instance remove bookstack_work" beside properly named
neighbours. That silence is why this kept being fixed and kept coming back.
The guard reads BOTH files as source — the command templates from
task-commands.js and the pattern table from tasks-format.js — so it fails on a
command added without a name rather than leaving it to be noticed in the UI.
Two checks, both from source rather than guessed from rendered text:
1. Nothing falls through: a title equal to its command, or still starting with
"libreportal ", means the raw fallback was reached.
2. Every `libreportal app <verb>` verb has an actionMap entry. Without one the
generic branch composes "<Verb> Application", which is how "Up Application"
and "Down Application" shipped.
The second check reads the actionMap keys instead of pattern-matching the title,
which a first attempt did and which was wrong: "Reload Application" is both a
correct hand-written label and what the generic branch emits, so the rendered
text cannot distinguish them and the heuristic failed a title that was fine.
Verified by breaking it deliberately in both directions — adding a command with
no pattern, and deleting an actionMap verb. Each is caught, named, and pointed at
the file to edit; both files were restored byte-identical afterwards.
Lives in scripts/dev, which .gitattributes marks export-ignore, so it never ships
in a release tarball. Needs a node and borrows the running container's when the
host has none, the same constraint lp-shot works around for chromium.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
9a74b841d9
commit
60a2fc3b0b
20
CLAUDE.md
20
CLAUDE.md
@ -35,3 +35,23 @@ repo: `/libreportal-containers/libreportal/frontend/` is bind-mounted into the
|
||||
container, so copying changed files there (owned `dockerinstall:dockerinstall`)
|
||||
takes effect on the next browser load — no rebuild or restart. Diff before you
|
||||
copy; the live tree can hold changes the repo doesn't.
|
||||
|
||||
## Check task names after adding a dispatched command
|
||||
|
||||
Task titles all come from one table in `components/tasks/js/tasks-format.js`,
|
||||
whose final fallback returns the raw command string. A command added to
|
||||
`core/tasks/js/task-commands.js` without a matching row does not error — it just
|
||||
renders as `libreportal instance remove bookstack_work` next to properly named
|
||||
neighbours like `Bookstack - Create Backup`. That silence is why bad titles kept
|
||||
coming back.
|
||||
|
||||
`lp-task-names` reads both files and fails if any dispatched command has no
|
||||
proper name:
|
||||
|
||||
```
|
||||
scripts/dev/lp-task-names # table of every command and its title
|
||||
scripts/dev/lp-task-names --quiet # failures only
|
||||
```
|
||||
|
||||
Run it after touching either file. It needs a node and borrows the running
|
||||
container's if the host has none.
|
||||
|
||||
@ -51,6 +51,24 @@
|
||||
background: transparent;
|
||||
}
|
||||
|
||||
|
||||
/* Placeholder default for every input in the app.
|
||||
Element-level ON PURPOSE. Placeholders had no baseline, so any input whose
|
||||
class carried no ::placeholder rule fell through to the browser default — a
|
||||
grey chosen for light backgrounds, which is close to invisible on these
|
||||
themes. That is a per-input accident waiting to recur every time someone adds
|
||||
a field, so it is fixed once here rather than six more times.
|
||||
Written against --text-rgb so it follows the theme (white on nebula/dark-blue,
|
||||
dark on light) where a fixed grey could only ever suit one of them, and
|
||||
opacity is pinned because Firefox dims placeholders on top of the colour.
|
||||
Any class-level ::placeholder rule still wins on specificity, so the surfaces
|
||||
that deliberately chose a dimmer or brighter value keep it. */
|
||||
input::placeholder,
|
||||
textarea::placeholder {
|
||||
color: rgba(var(--text-rgb), 0.6);
|
||||
opacity: 1;
|
||||
}
|
||||
|
||||
body {
|
||||
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif;
|
||||
display: flex;
|
||||
|
||||
186
scripts/dev/lp-task-names
Executable file
186
scripts/dev/lp-task-names
Executable file
@ -0,0 +1,186 @@
|
||||
#!/usr/bin/env python3
|
||||
"""lp-task-names — fail if any dispatched command has no task title.
|
||||
|
||||
lp-task-names # check, print a table, exit 1 on any failure
|
||||
lp-task-names --quiet # only print failures
|
||||
|
||||
Task titles come from ONE place: formatCommandForUser() in
|
||||
components/tasks/js/tasks-format.js, a declarative table of command patterns
|
||||
whose final fallback returns the raw command string. That fallback is why bad
|
||||
titles keep reappearing — a command shape with no row does not error, it just
|
||||
renders as "libreportal instance remove bookstack_work" next to properly named
|
||||
neighbours like "Bookstack - Create Backup".
|
||||
|
||||
This reads BOTH files as source, so a command added to task-commands.js without
|
||||
a matching row in tasks-format.js fails here rather than in front of a user.
|
||||
|
||||
Two things are checked, both read from source rather than guessed from the
|
||||
rendered text:
|
||||
|
||||
1. No command falls through. A title that equals its command, or still starts
|
||||
with "libreportal ", means the raw fallback was reached.
|
||||
|
||||
2. Every `libreportal app <verb>` verb has an actionMap entry. Without one the
|
||||
generic branch composes "<Verb> Application", which is how "Up Application"
|
||||
and "Down Application" shipped — technically named, but not English. This
|
||||
is deliberately NOT inferred from the title: "Reload Application" is both a
|
||||
correct hand-written label and what the generic branch would emit, so only
|
||||
the source says which it is.
|
||||
|
||||
Needs a node. Uses the host's if present, otherwise borrows the one inside the
|
||||
running libreportal container. DEV TOOL — scripts/dev is export-ignored, so this
|
||||
never ships.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
ROOT = os.path.realpath(os.path.join(os.path.dirname(__file__), "..", ".."))
|
||||
FRONTEND = os.path.join(ROOT, "containers", "libreportal", "frontend")
|
||||
COMMANDS = os.path.join(FRONTEND, "core", "tasks", "js", "task-commands.js")
|
||||
FORMAT = os.path.join(FRONTEND, "components", "tasks", "js", "tasks-format.js")
|
||||
|
||||
# Stand-ins for the {placeholders} in a command template. Values only have to be
|
||||
# shaped like the real thing — the patterns match on structure, not content.
|
||||
SAMPLES = {
|
||||
"appName": "bookstack",
|
||||
"type": "bookstack",
|
||||
"name": "work",
|
||||
"service": "bookstack-db",
|
||||
"config": "",
|
||||
"backupId": "backup_20260101",
|
||||
"toolId": "list_users",
|
||||
"args": "",
|
||||
}
|
||||
|
||||
|
||||
def die(msg, code=1):
|
||||
print(f"lp-task-names: {msg}", file=sys.stderr)
|
||||
sys.exit(code)
|
||||
|
||||
|
||||
def node_runner():
|
||||
"""Return a callable that runs a JS file and gives back its stdout."""
|
||||
host = shutil.which("node")
|
||||
if host:
|
||||
return lambda path: subprocess.run(
|
||||
[host, path], capture_output=True, text=True, timeout=120)
|
||||
|
||||
# No host node: hand the script to the container's on stdin, so nothing has
|
||||
# to be copied into a bind mount the caller may not be able to write.
|
||||
def via_container(path):
|
||||
inner = ("export DOCKER_HOST=unix:///run/user/$(id -u)/docker.sock; "
|
||||
"docker exec -i libreportal-service sh -c "
|
||||
"'cat > /tmp/lp-task-names.js && node /tmp/lp-task-names.js'")
|
||||
with open(path, "rb") as fh:
|
||||
return subprocess.run(["sudo", "-n", "-u", "dockerinstall", "bash", "-c", inner],
|
||||
stdin=fh, capture_output=True, text=True, timeout=180)
|
||||
return via_container
|
||||
|
||||
|
||||
def action_map_verbs():
|
||||
"""Verb keys of the actionMap in tasks-format.js's generic-command branch."""
|
||||
src = open(FORMAT, encoding="utf-8").read()
|
||||
m = re.search(r"const actionMap\s*=\s*\{(.*?)\};", src, re.S)
|
||||
if not m:
|
||||
die("could not find actionMap in tasks-format.js — has its shape changed?")
|
||||
return set(re.findall(r"'([a-z_]+)'\s*:", m.group(1)))
|
||||
|
||||
|
||||
def command_templates():
|
||||
"""Every uncommented `key: 'libreportal …'` entry in task-commands.js."""
|
||||
src = open(COMMANDS, encoding="utf-8").read()
|
||||
out = []
|
||||
for line in src.splitlines():
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("//"):
|
||||
continue
|
||||
m = re.match(r"^([A-Za-z_][A-Za-z0-9_]*)\s*:\s*'(libreportal [^']*)'", stripped)
|
||||
if m:
|
||||
out.append((m.group(1), m.group(2)))
|
||||
return out
|
||||
|
||||
|
||||
def fill(template):
|
||||
def sub(m):
|
||||
key = m.group(1)
|
||||
if key not in SAMPLES:
|
||||
die(f"template uses unknown placeholder {{{key}}} — add it to SAMPLES")
|
||||
return SAMPLES[key]
|
||||
return re.sub(r"\{(\w+)\}", sub, template).strip()
|
||||
|
||||
|
||||
def main():
|
||||
quiet = "--quiet" in sys.argv
|
||||
for path in (COMMANDS, FORMAT):
|
||||
if not os.path.isfile(path):
|
||||
die(f"missing {path}")
|
||||
|
||||
cases = [(key, fill(tpl)) for key, tpl in command_templates()]
|
||||
if not cases:
|
||||
die("no command templates found in task-commands.js — has its shape changed?")
|
||||
|
||||
harness = os.path.join("/tmp", f"lp-task-names-{os.getpid()}.js")
|
||||
with open(harness, "w", encoding="utf-8") as fh:
|
||||
# window.getAppDisplayName is the WebUI's slug -> title helper; the real
|
||||
# one reads window.apps, which does not exist outside a browser.
|
||||
fh.write("global.window = { getAppDisplayName: (s) => "
|
||||
"s.split('_').map(w => w.charAt(0).toUpperCase() + w.slice(1)).join(' \\u00b7 ') };\n")
|
||||
fh.write("class TasksManager {}\n")
|
||||
fh.write(open(FORMAT, encoding="utf-8").read())
|
||||
fh.write("\nconst t = new TasksManager();\n")
|
||||
fh.write("const cases = " + json.dumps([c for _, c in cases]) + ";\n")
|
||||
fh.write("console.log(JSON.stringify(cases.map(c => t.formatCommandForUser({command: c}))));\n")
|
||||
|
||||
try:
|
||||
res = node_runner()(harness)
|
||||
finally:
|
||||
try:
|
||||
os.unlink(harness)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
if res.returncode != 0:
|
||||
die(f"could not run the formatter:\n{res.stderr.strip() or res.stdout.strip()}")
|
||||
try:
|
||||
titles = json.loads(res.stdout.strip().splitlines()[-1])
|
||||
except (ValueError, IndexError):
|
||||
die(f"formatter produced no usable output:\n{res.stdout}\n{res.stderr}")
|
||||
|
||||
verbs = action_map_verbs()
|
||||
failures = []
|
||||
width = max(len(c) for _, c in cases)
|
||||
for (key, command), title in zip(cases, titles):
|
||||
why = None
|
||||
if title == command or title.startswith("libreportal "):
|
||||
why = "no pattern — fell through to the raw command"
|
||||
else:
|
||||
m = re.match(r"^libreportal app ([a-z_]+)\b", command)
|
||||
if m and m.group(1) not in verbs:
|
||||
why = (f"verb '{m.group(1)}' has no actionMap entry — the generic branch "
|
||||
f"would compose '<Verb> Application'")
|
||||
if why:
|
||||
failures.append((key, command, title, why))
|
||||
if not quiet:
|
||||
print(f" {'FAIL' if why else 'ok '} {command.ljust(width)} -> {title}")
|
||||
|
||||
print()
|
||||
if failures:
|
||||
print(f" {len(failures)} of {len(cases)} dispatched command(s) have no proper task name:\n")
|
||||
for key, command, title, why in failures:
|
||||
print(f" {key}: {command}")
|
||||
print(f" renders as : {title}")
|
||||
print(f" problem : {why}")
|
||||
print("\n Fix: add a PATTERNS row (or an actionMap entry) in")
|
||||
print(f" {os.path.relpath(FORMAT, ROOT)}")
|
||||
return 1
|
||||
|
||||
print(f" all {len(cases)} dispatched commands have a proper task name.")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Loading…
x
Reference in New Issue
Block a user