diff --git a/modules/agent-box.nix b/modules/agent-box.nix index e95262c3..ddc598ef 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -6443,15 +6443,16 @@ fi # the event. Older receivers set nothing, and an entry with no config sets `{}`, # so both read as "no watch-level answer" and the box-wide setting stands. # -# --preamble has no delivery and so no variable: the settings page asks what a -# named TOPIC would start, so that mode reads the watch straight out of the -# dispatch file. Same precedence either way, because the page must not advertise -# a worker the spawn would not use. +# --preamble and --resolved-profile have no delivery and so no variable: the +# settings page asks what a named TOPIC would start, so both modes read the +# watch straight out of the dispatch file. Same precedence either way, +# because the page must not advertise a worker the spawn would not use. watch_profile="" watch_config="" if [ -n "''${LOCAL_WEBHOOK_SPAWN_CONFIG:-}" ]; then watch_config="$LOCAL_WEBHOOK_SPAWN_CONFIG" -elif [ "''${1:-}" = "--preamble" ] && [ -n "''${2:-}" ] && [ -n "''${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then +elif { [ "''${1:-}" = "--preamble" ] || [ "''${1:-}" = "--resolved-profile" ]; } \ + && [ -n "''${2:-}" ] && [ -n "''${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then # Best effort, like every other read here: no file, bad JSON or no such topic # all mean "this watch names no profile", never a failed render. watch_config=$("$JQ" -r --arg t "$2" \ @@ -6488,6 +6489,12 @@ profile_problem() { hook_profile="" hook_profile_source="" hook_profile_ignored="" +# Bare names alongside the prose above, for --resolved-profile (#582): a +# consumer that wants to warn about a named-but-missing profile needs the +# NAME, and hook_profile_ignored only ever carries it wrapped in a sentence +# meant for a human. Names are charset-restricted by profile_problem before +# they land here, so a plain space-joined list round-trips with no quoting. +hook_profile_ignored_names="" try_profile() { [ -n "$1" ] || return 0 [ -z "$hook_profile" ] || return 0 @@ -6499,6 +6506,7 @@ try_profile() { fi echo "agent-box-webhook-spawn: ignoring profile '$1' from $2 — $why" >&2 hook_profile_ignored="''${hook_profile_ignored:+$hook_profile_ignored; }$2 — IGNORED, $why" + hook_profile_ignored_names="''${hook_profile_ignored_names:+$hook_profile_ignored_names }$1" } try_profile "$watch_profile" "this watch's own spawnConfig.profile" @@ -6593,6 +6601,24 @@ root, running sessions keep the arguments they started with:" fallback." } +# --resolved-profile TOPIC [NOTE]: print, as one JSON object, the profile a +# match on TOPIC would actually use right now — {"profile": NAME or null, +# "missing": [NAME, ...]} — and spawn nothing. Same resolution as a real +# spawn and as --preamble's prose (the precedence above already ran by the +# time this checks $1), but MACHINE-readable: the settings page needs a bare +# name to badge a watch's row and to warn a profile delete out from under a +# live watch (#582), and scraping that out of --preamble's sentences would +# break every time the wording does. `missing` lists every named-but-unusable +# profile that was tried and passed over, most specific (the watch's own) +# first — empty when nothing was named, distinct from `profile` being null +# for "nothing resolved, so the box default harness starts". +if [ "''${1:-}" = "--resolved-profile" ]; then + "$JQ" -n --arg p "$hook_profile" --arg m "$hook_profile_ignored_names" \ + '{profile: (if $p == "" then null else $p end), + missing: (if $m == "" then [] else ($m | split(" ")) end)}' + exit 0 +fi + # --preamble TOPIC [NOTE]: print what a match on TOPIC would launch — the # launch command first, then the prompt — and spawn nothing. Everything a # delivery decides is left as a : the event key names the session @@ -13661,6 +13687,77 @@ def hook_preamble(topic, note, stamp): return proc.stdout.strip() if proc.returncode == 0 else "" +@functools.lru_cache(maxsize=64) +def hook_resolved_profile(topic, note, stamp): + """(profile, missing) for this standing watch, right now - profile is "" + when nothing resolves (a match starts the box default agent), missing is + a tuple of every named-but-unusable profile that was tried and passed + over, most specific first. + + A bare answer, not prose: hook_preamble's --preamble text answers the + same question for a human reading the fold, but a caller that wants to + badge a watch's row or warn a profile delete out from under a live watch + (#582) needs the NAME alone, and scraping it out of a sentence would + break every time the wording does. Same cache key as hook_preamble, for + the same reason - a changed env file or profiles directory has to + re-render both. + """ + if not HOOK_SPAWN_CMD: + return ("", ()) + try: + proc = subprocess.run( + [HOOK_SPAWN_CMD, "--resolved-profile", topic, note], + env=dict(os.environ, LOCAL_WEBHOOK_STATE_DIR=webhook_state_dir()), + check=False, + capture_output=True, + text=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError) as exc: + sys.stderr.write("webhook: resolved-profile: %s\n" % exc) + return ("", ()) + if proc.returncode != 0: + return ("", ()) + try: + data = json.loads(proc.stdout) + except ValueError: + return ("", ()) + if not isinstance(data, dict): + return ("", ()) + profile = data.get("profile") + missing = data.get("missing") + return ( + profile if isinstance(profile, str) else "", + tuple(m for m in missing if isinstance(m, str)) + if isinstance(missing, list) else (), + ) + + +def profile_usage(watches): + """profile name -> sorted topics whose standing watch resolves to it + right now, explicitly or via the box-wide AGENT_BOX_HOOK_PROFILE + fallback (#582). What `agent-box-profile rm`/the delete route never + checked: a profile deleted out from under a live watch does not stop + that watch, it just changes which worker future events start - silently, + and possibly onto a more expensive one than whoever deleted it expected. + + Reuses hook_resolved_profile's own cache, so a page that already renders + the watch list (which looks up the same answer per row) pays for this + once per (topic, note, stamp), not twice. + """ + stamp = hook_args_stamp() + usage = {} + for entry in watches: + topic = str(entry.get("topic") or "") + if not topic: + continue + note = str(entry.get("note") or "") + profile, _missing = hook_resolved_profile(topic, note, stamp) + if profile: + usage.setdefault(profile, []).append(topic) + return {name: sorted(topics) for name, topics in usage.items()} + + def webhook_entries(data, dispatch): """The topic entries of a `subscriptions` payload, session or dispatch side, as a list of dicts.""" @@ -17081,11 +17178,20 @@ def render_effort_options(selected=""): return "".join(items) -def render_profiles(profiles): +def render_profiles(profiles, usage=None): """The profiles list. Each row folds open onto its launch config and its environment KEY NAMES — never a value, the rule `agent-box-profile show` and `env ls` already keep, and the one that matters most here because a - profile is exactly where a token ends up.""" + profile is exactly where a token ends up. + + `usage` is profile_usage(watches) (#582): a profile a standing watch + resolves to right now says so on its own row, not only on the watch's, + because the delete button lives here and the cost of pressing it by + mistake is a watch quietly starting the box default agent instead - no + error, no dropped events, just a worker nobody chose. Defaults to none + in use, for a caller (a unit test, mainly) that has no watch list to + hand and does not care.""" + usage = usage or {} base = html.escape(BASE) rows = [] for name in sorted(profiles): @@ -17103,6 +17209,28 @@ def render_profiles(profiles): if env_keys: bits.append("%d custom setting%s" % ( len(env_keys), "" if len(env_keys) == 1 else "s")) + topics = usage.get(name) or [] + watch_warn = "" + if topics: + plural = "" if len(topics) == 1 else "es" + bits.append("used by %d standing watch%s" % (len(topics), plural)) + # RAW here, not html.escape()'d: this only ever reaches the + # confirm() dialog below, built through json.dumps() rather than + # hand-spliced into a quoted JS string. A webhook topic is a + # source:key the operator (or a hand edit of the dispatch file) + # chooses, and local-webhook's own key charset allows an + # apostrophe - html.escape() alone would leave `'` in the + # HTML attribute, which the BROWSER decodes back to `'` before + # handing the onsubmit text to the JS parser, breaking out of a + # hand-quoted confirm('...') string. + topic_list = ", ".join(topics) + watch_warn = ( + " It is used by %d standing watch%s: %s. After deleting, " + "%s the box default agent instead." + % (len(topics), plural, topic_list, + "that watch starts" if len(topics) == 1 + else "those watches start") + ) # Each bit was escaped as it went in, so the join must NOT be # escaped again: a MODEL holding "&" or "<" would render as visible # entity text ("&lt;"). @@ -17131,14 +17259,26 @@ def render_profiles(profiles): # stored, so an edit is an edit and not a retype. SYSTEM_PROMPT is a # textarea for the same reason it is one above — it can span lines. prompt_val = html.escape(res.get("SYSTEM_PROMPT") or "") + # json.dumps(), not hand-quoted JS: `name` is charset-restricted + # (PROFILE_NAME_RE), but watch_warn's topic names are not, so this is + # where the confirm() dialog's whole message gets ONE correct + # escaping instead of a second, wrong one — html.escape() on its own + # protects the HTML attribute but leaves the JS string quoting to + # hope, and a topic holding an apostrophe broke out of it. + confirm_js = html.escape( + json.dumps( + "Delete profile %s?%s Sessions already running keep what " + "they started with." % (name, watch_warn) + ), + quote=True, + ) rows.append( f'
  • ' f'{safe}' f'{meta}' f'
    ' + f'onsubmit="return confirm({confirm_js});">' f'' f'
    ' @@ -17621,6 +17761,20 @@ def render_webhook_endpoint(): ) +def display_hook_profile(profile, missing): + """The one-line answer to "what does this watch start" (#582), for the + row itself rather than the fold an operator has to open to find it. + `missing` being non-empty even when `profile` resolved is exactly the + case --preamble already reports via IGNORED - the watch's own name was + unusable and a less specific one took over - so it says so here too, + rather than only in the fold's prose.""" + bit = ("Profile: %s" % profile if profile + else "No profile - box default agent") + if missing: + bit += " (%s not found)" % ", ".join("'%s'" % m for m in missing) + return bit + + def render_webhooks(watches): """The standing watches. Session subscriptions are NOT here: they belong to a session and are folded into its row above. A standing @@ -17641,9 +17795,11 @@ def render_webhooks(watches): continue note = str(entry.get("note") or "") prompt = hook_preamble(topic, note, stamp) + profile, missing = hook_resolved_profile(topic, note, stamp) rows.append(render_webhook_row( topic, - [display_event_expiry(entry.get("expiresIn"))], + [display_event_expiry(entry.get("expiresIn")), + display_hook_profile(profile, missing)], # Only when the prompt could not be rendered: then the note is # the one thing left that says why this watch exists. "" if prompt else note, @@ -18105,6 +18261,11 @@ def render_page(message="", kind="ok"): # and the panel below it — the same rule the subscription pass above # states, for the same reason. profiles = read_profiles() + # Which of those profiles a standing watch is actually spending right + # now (#582) - computed once here, from the same `watches` the webhooks + # panel below already walks, so the profiles panel's delete warning and + # that panel's per-row badge never disagree about what "in use" means. + usage = profile_usage(watches) return ( render_head("Settings — " + html.escape(USER)) + STYLE @@ -18121,7 +18282,7 @@ def render_page(message="", kind="ok"): base=html.escape(BASE), harnesses=render_harness_options(), effort=render_effort_options(), - profiles=render_profiles(profiles), + profiles=render_profiles(profiles, usage), ), webhooks_section=( WEBHOOK_UNAVAILABLE_TPL.format(text=unavailable) diff --git a/modules/src/settings-daemon.py b/modules/src/settings-daemon.py index 7fcbdcde..4fa0e319 100644 --- a/modules/src/settings-daemon.py +++ b/modules/src/settings-daemon.py @@ -1547,6 +1547,77 @@ def hook_preamble(topic, note, stamp): return proc.stdout.strip() if proc.returncode == 0 else "" +@functools.lru_cache(maxsize=64) +def hook_resolved_profile(topic, note, stamp): + """(profile, missing) for this standing watch, right now - profile is "" + when nothing resolves (a match starts the box default agent), missing is + a tuple of every named-but-unusable profile that was tried and passed + over, most specific first. + + A bare answer, not prose: hook_preamble's --preamble text answers the + same question for a human reading the fold, but a caller that wants to + badge a watch's row or warn a profile delete out from under a live watch + (#582) needs the NAME alone, and scraping it out of a sentence would + break every time the wording does. Same cache key as hook_preamble, for + the same reason - a changed env file or profiles directory has to + re-render both. + """ + if not HOOK_SPAWN_CMD: + return ("", ()) + try: + proc = subprocess.run( + [HOOK_SPAWN_CMD, "--resolved-profile", topic, note], + env=dict(os.environ, LOCAL_WEBHOOK_STATE_DIR=webhook_state_dir()), + check=False, + capture_output=True, + text=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError) as exc: + sys.stderr.write("webhook: resolved-profile: %s\n" % exc) + return ("", ()) + if proc.returncode != 0: + return ("", ()) + try: + data = json.loads(proc.stdout) + except ValueError: + return ("", ()) + if not isinstance(data, dict): + return ("", ()) + profile = data.get("profile") + missing = data.get("missing") + return ( + profile if isinstance(profile, str) else "", + tuple(m for m in missing if isinstance(m, str)) + if isinstance(missing, list) else (), + ) + + +def profile_usage(watches): + """profile name -> sorted topics whose standing watch resolves to it + right now, explicitly or via the box-wide AGENT_BOX_HOOK_PROFILE + fallback (#582). What `agent-box-profile rm`/the delete route never + checked: a profile deleted out from under a live watch does not stop + that watch, it just changes which worker future events start - silently, + and possibly onto a more expensive one than whoever deleted it expected. + + Reuses hook_resolved_profile's own cache, so a page that already renders + the watch list (which looks up the same answer per row) pays for this + once per (topic, note, stamp), not twice. + """ + stamp = hook_args_stamp() + usage = {} + for entry in watches: + topic = str(entry.get("topic") or "") + if not topic: + continue + note = str(entry.get("note") or "") + profile, _missing = hook_resolved_profile(topic, note, stamp) + if profile: + usage.setdefault(profile, []).append(topic) + return {name: sorted(topics) for name, topics in usage.items()} + + def webhook_entries(data, dispatch): """The topic entries of a `subscriptions` payload, session or dispatch side, as a list of dicts.""" @@ -3528,11 +3599,20 @@ def render_effort_options(selected=""): return "".join(items) -def render_profiles(profiles): +def render_profiles(profiles, usage=None): """The profiles list. Each row folds open onto its launch config and its environment KEY NAMES — never a value, the rule `agent-box-profile show` and `env ls` already keep, and the one that matters most here because a - profile is exactly where a token ends up.""" + profile is exactly where a token ends up. + + `usage` is profile_usage(watches) (#582): a profile a standing watch + resolves to right now says so on its own row, not only on the watch's, + because the delete button lives here and the cost of pressing it by + mistake is a watch quietly starting the box default agent instead - no + error, no dropped events, just a worker nobody chose. Defaults to none + in use, for a caller (a unit test, mainly) that has no watch list to + hand and does not care.""" + usage = usage or {} base = html.escape(BASE) rows = [] for name in sorted(profiles): @@ -3550,6 +3630,28 @@ def render_profiles(profiles): if env_keys: bits.append("%d custom setting%s" % ( len(env_keys), "" if len(env_keys) == 1 else "s")) + topics = usage.get(name) or [] + watch_warn = "" + if topics: + plural = "" if len(topics) == 1 else "es" + bits.append("used by %d standing watch%s" % (len(topics), plural)) + # RAW here, not html.escape()'d: this only ever reaches the + # confirm() dialog below, built through json.dumps() rather than + # hand-spliced into a quoted JS string. A webhook topic is a + # source:key the operator (or a hand edit of the dispatch file) + # chooses, and local-webhook's own key charset allows an + # apostrophe - html.escape() alone would leave `'` in the + # HTML attribute, which the BROWSER decodes back to `'` before + # handing the onsubmit text to the JS parser, breaking out of a + # hand-quoted confirm('...') string. + topic_list = ", ".join(topics) + watch_warn = ( + " It is used by %d standing watch%s: %s. After deleting, " + "%s the box default agent instead." + % (len(topics), plural, topic_list, + "that watch starts" if len(topics) == 1 + else "those watches start") + ) # Each bit was escaped as it went in, so the join must NOT be # escaped again: a MODEL holding "&" or "<" would render as visible # entity text ("&lt;"). @@ -3578,14 +3680,26 @@ def render_profiles(profiles): # stored, so an edit is an edit and not a retype. SYSTEM_PROMPT is a # textarea for the same reason it is one above — it can span lines. prompt_val = html.escape(res.get("SYSTEM_PROMPT") or "") + # json.dumps(), not hand-quoted JS: `name` is charset-restricted + # (PROFILE_NAME_RE), but watch_warn's topic names are not, so this is + # where the confirm() dialog's whole message gets ONE correct + # escaping instead of a second, wrong one — html.escape() on its own + # protects the HTML attribute but leaves the JS string quoting to + # hope, and a topic holding an apostrophe broke out of it. + confirm_js = html.escape( + json.dumps( + "Delete profile %s?%s Sessions already running keep what " + "they started with." % (name, watch_warn) + ), + quote=True, + ) rows.append( f'
  • ' f'{safe}' f'{meta}' f'
    ' + f'onsubmit="return confirm({confirm_js});">' f'' f'
    ' @@ -4068,6 +4182,20 @@ def render_webhook_endpoint(): ) +def display_hook_profile(profile, missing): + """The one-line answer to "what does this watch start" (#582), for the + row itself rather than the fold an operator has to open to find it. + `missing` being non-empty even when `profile` resolved is exactly the + case --preamble already reports via IGNORED - the watch's own name was + unusable and a less specific one took over - so it says so here too, + rather than only in the fold's prose.""" + bit = ("Profile: %s" % profile if profile + else "No profile - box default agent") + if missing: + bit += " (%s not found)" % ", ".join("'%s'" % m for m in missing) + return bit + + def render_webhooks(watches): """The standing watches. Session subscriptions are NOT here: they belong to a session and are folded into its row above. A standing @@ -4088,9 +4216,11 @@ def render_webhooks(watches): continue note = str(entry.get("note") or "") prompt = hook_preamble(topic, note, stamp) + profile, missing = hook_resolved_profile(topic, note, stamp) rows.append(render_webhook_row( topic, - [display_event_expiry(entry.get("expiresIn"))], + [display_event_expiry(entry.get("expiresIn")), + display_hook_profile(profile, missing)], # Only when the prompt could not be rendered: then the note is # the one thing left that says why this watch exists. "" if prompt else note, @@ -4552,6 +4682,11 @@ def render_page(message="", kind="ok"): # and the panel below it — the same rule the subscription pass above # states, for the same reason. profiles = read_profiles() + # Which of those profiles a standing watch is actually spending right + # now (#582) - computed once here, from the same `watches` the webhooks + # panel below already walks, so the profiles panel's delete warning and + # that panel's per-row badge never disagree about what "in use" means. + usage = profile_usage(watches) return ( render_head("Settings — " + html.escape(USER)) + STYLE @@ -4568,7 +4703,7 @@ def render_page(message="", kind="ok"): base=html.escape(BASE), harnesses=render_harness_options(), effort=render_effort_options(), - profiles=render_profiles(profiles), + profiles=render_profiles(profiles, usage), ), webhooks_section=( WEBHOOK_UNAVAILABLE_TPL.format(text=unavailable) diff --git a/modules/src/webhook-spawn.sh b/modules/src/webhook-spawn.sh index 8010ba55..5df25b14 100644 --- a/modules/src/webhook-spawn.sh +++ b/modules/src/webhook-spawn.sh @@ -215,15 +215,16 @@ fi # the event. Older receivers set nothing, and an entry with no config sets `{}`, # so both read as "no watch-level answer" and the box-wide setting stands. # -# --preamble has no delivery and so no variable: the settings page asks what a -# named TOPIC would start, so that mode reads the watch straight out of the -# dispatch file. Same precedence either way, because the page must not advertise -# a worker the spawn would not use. +# --preamble and --resolved-profile have no delivery and so no variable: the +# settings page asks what a named TOPIC would start, so both modes read the +# watch straight out of the dispatch file. Same precedence either way, +# because the page must not advertise a worker the spawn would not use. watch_profile="" watch_config="" if [ -n "${LOCAL_WEBHOOK_SPAWN_CONFIG:-}" ]; then watch_config="$LOCAL_WEBHOOK_SPAWN_CONFIG" -elif [ "${1:-}" = "--preamble" ] && [ -n "${2:-}" ] && [ -n "${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then +elif { [ "${1:-}" = "--preamble" ] || [ "${1:-}" = "--resolved-profile" ]; } \ + && [ -n "${2:-}" ] && [ -n "${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then # Best effort, like every other read here: no file, bad JSON or no such topic # all mean "this watch names no profile", never a failed render. watch_config=$("$JQ" -r --arg t "$2" \ @@ -260,6 +261,12 @@ profile_problem() { hook_profile="" hook_profile_source="" hook_profile_ignored="" +# Bare names alongside the prose above, for --resolved-profile (#582): a +# consumer that wants to warn about a named-but-missing profile needs the +# NAME, and hook_profile_ignored only ever carries it wrapped in a sentence +# meant for a human. Names are charset-restricted by profile_problem before +# they land here, so a plain space-joined list round-trips with no quoting. +hook_profile_ignored_names="" try_profile() { [ -n "$1" ] || return 0 [ -z "$hook_profile" ] || return 0 @@ -271,6 +278,7 @@ try_profile() { fi echo "agent-box-webhook-spawn: ignoring profile '$1' from $2 — $why" >&2 hook_profile_ignored="${hook_profile_ignored:+$hook_profile_ignored; }$2 — IGNORED, $why" + hook_profile_ignored_names="${hook_profile_ignored_names:+$hook_profile_ignored_names }$1" } try_profile "$watch_profile" "this watch's own spawnConfig.profile" @@ -365,6 +373,24 @@ ${AGENT_BOX_HOOK_ARGS_OPTION_NAME:-the fleet-wide default}, which is the \ fallback." } +# --resolved-profile TOPIC [NOTE]: print, as one JSON object, the profile a +# match on TOPIC would actually use right now — {"profile": NAME or null, +# "missing": [NAME, ...]} — and spawn nothing. Same resolution as a real +# spawn and as --preamble's prose (the precedence above already ran by the +# time this checks $1), but MACHINE-readable: the settings page needs a bare +# name to badge a watch's row and to warn a profile delete out from under a +# live watch (#582), and scraping that out of --preamble's sentences would +# break every time the wording does. `missing` lists every named-but-unusable +# profile that was tried and passed over, most specific (the watch's own) +# first — empty when nothing was named, distinct from `profile` being null +# for "nothing resolved, so the box default harness starts". +if [ "${1:-}" = "--resolved-profile" ]; then + "$JQ" -n --arg p "$hook_profile" --arg m "$hook_profile_ignored_names" \ + '{profile: (if $p == "" then null else $p end), + missing: (if $m == "" then [] else ($m | split(" ")) end)}' + exit 0 +fi + # --preamble TOPIC [NOTE]: print what a match on TOPIC would launch — the # launch command first, then the prompt — and spawn nothing. Everything a # delivery decides is left as a : the event key names the session diff --git a/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings b/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings index 40b4035b..5b85c00e 100644 --- a/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings +++ b/tests/golden/web/payloads/agent-box-settings/bin/agent-box-settings @@ -1879,6 +1879,77 @@ def hook_preamble(topic, note, stamp): return proc.stdout.strip() if proc.returncode == 0 else "" +@functools.lru_cache(maxsize=64) +def hook_resolved_profile(topic, note, stamp): + """(profile, missing) for this standing watch, right now - profile is "" + when nothing resolves (a match starts the box default agent), missing is + a tuple of every named-but-unusable profile that was tried and passed + over, most specific first. + + A bare answer, not prose: hook_preamble's --preamble text answers the + same question for a human reading the fold, but a caller that wants to + badge a watch's row or warn a profile delete out from under a live watch + (#582) needs the NAME alone, and scraping it out of a sentence would + break every time the wording does. Same cache key as hook_preamble, for + the same reason - a changed env file or profiles directory has to + re-render both. + """ + if not HOOK_SPAWN_CMD: + return ("", ()) + try: + proc = subprocess.run( + [HOOK_SPAWN_CMD, "--resolved-profile", topic, note], + env=dict(os.environ, LOCAL_WEBHOOK_STATE_DIR=webhook_state_dir()), + check=False, + capture_output=True, + text=True, + timeout=10, + ) + except (OSError, subprocess.SubprocessError) as exc: + sys.stderr.write("webhook: resolved-profile: %s\n" % exc) + return ("", ()) + if proc.returncode != 0: + return ("", ()) + try: + data = json.loads(proc.stdout) + except ValueError: + return ("", ()) + if not isinstance(data, dict): + return ("", ()) + profile = data.get("profile") + missing = data.get("missing") + return ( + profile if isinstance(profile, str) else "", + tuple(m for m in missing if isinstance(m, str)) + if isinstance(missing, list) else (), + ) + + +def profile_usage(watches): + """profile name -> sorted topics whose standing watch resolves to it + right now, explicitly or via the box-wide AGENT_BOX_HOOK_PROFILE + fallback (#582). What `agent-box-profile rm`/the delete route never + checked: a profile deleted out from under a live watch does not stop + that watch, it just changes which worker future events start - silently, + and possibly onto a more expensive one than whoever deleted it expected. + + Reuses hook_resolved_profile's own cache, so a page that already renders + the watch list (which looks up the same answer per row) pays for this + once per (topic, note, stamp), not twice. + """ + stamp = hook_args_stamp() + usage = {} + for entry in watches: + topic = str(entry.get("topic") or "") + if not topic: + continue + note = str(entry.get("note") or "") + profile, _missing = hook_resolved_profile(topic, note, stamp) + if profile: + usage.setdefault(profile, []).append(topic) + return {name: sorted(topics) for name, topics in usage.items()} + + def webhook_entries(data, dispatch): """The topic entries of a `subscriptions` payload, session or dispatch side, as a list of dicts.""" @@ -5299,11 +5370,20 @@ def render_effort_options(selected=""): return "".join(items) -def render_profiles(profiles): +def render_profiles(profiles, usage=None): """The profiles list. Each row folds open onto its launch config and its environment KEY NAMES — never a value, the rule `agent-box-profile show` and `env ls` already keep, and the one that matters most here because a - profile is exactly where a token ends up.""" + profile is exactly where a token ends up. + + `usage` is profile_usage(watches) (#582): a profile a standing watch + resolves to right now says so on its own row, not only on the watch's, + because the delete button lives here and the cost of pressing it by + mistake is a watch quietly starting the box default agent instead - no + error, no dropped events, just a worker nobody chose. Defaults to none + in use, for a caller (a unit test, mainly) that has no watch list to + hand and does not care.""" + usage = usage or {} base = html.escape(BASE) rows = [] for name in sorted(profiles): @@ -5321,6 +5401,28 @@ def render_profiles(profiles): if env_keys: bits.append("%d custom setting%s" % ( len(env_keys), "" if len(env_keys) == 1 else "s")) + topics = usage.get(name) or [] + watch_warn = "" + if topics: + plural = "" if len(topics) == 1 else "es" + bits.append("used by %d standing watch%s" % (len(topics), plural)) + # RAW here, not html.escape()'d: this only ever reaches the + # confirm() dialog below, built through json.dumps() rather than + # hand-spliced into a quoted JS string. A webhook topic is a + # source:key the operator (or a hand edit of the dispatch file) + # chooses, and local-webhook's own key charset allows an + # apostrophe - html.escape() alone would leave `'` in the + # HTML attribute, which the BROWSER decodes back to `'` before + # handing the onsubmit text to the JS parser, breaking out of a + # hand-quoted confirm('...') string. + topic_list = ", ".join(topics) + watch_warn = ( + " It is used by %d standing watch%s: %s. After deleting, " + "%s the box default agent instead." + % (len(topics), plural, topic_list, + "that watch starts" if len(topics) == 1 + else "those watches start") + ) # Each bit was escaped as it went in, so the join must NOT be # escaped again: a MODEL holding "&" or "<" would render as visible # entity text ("&lt;"). @@ -5349,14 +5451,26 @@ def render_profiles(profiles): # stored, so an edit is an edit and not a retype. SYSTEM_PROMPT is a # textarea for the same reason it is one above — it can span lines. prompt_val = html.escape(res.get("SYSTEM_PROMPT") or "") + # json.dumps(), not hand-quoted JS: `name` is charset-restricted + # (PROFILE_NAME_RE), but watch_warn's topic names are not, so this is + # where the confirm() dialog's whole message gets ONE correct + # escaping instead of a second, wrong one — html.escape() on its own + # protects the HTML attribute but leaves the JS string quoting to + # hope, and a topic holding an apostrophe broke out of it. + confirm_js = html.escape( + json.dumps( + "Delete profile %s?%s Sessions already running keep what " + "they started with." % (name, watch_warn) + ), + quote=True, + ) rows.append( f'
  • ' f'{safe}' f'{meta}' f'
    ' + f'onsubmit="return confirm({confirm_js});">' f'' f'
    ' @@ -5839,6 +5953,20 @@ def render_webhook_endpoint(): ) +def display_hook_profile(profile, missing): + """The one-line answer to "what does this watch start" (#582), for the + row itself rather than the fold an operator has to open to find it. + `missing` being non-empty even when `profile` resolved is exactly the + case --preamble already reports via IGNORED - the watch's own name was + unusable and a less specific one took over - so it says so here too, + rather than only in the fold's prose.""" + bit = ("Profile: %s" % profile if profile + else "No profile - box default agent") + if missing: + bit += " (%s not found)" % ", ".join("'%s'" % m for m in missing) + return bit + + def render_webhooks(watches): """The standing watches. Session subscriptions are NOT here: they belong to a session and are folded into its row above. A standing @@ -5859,9 +5987,11 @@ def render_webhooks(watches): continue note = str(entry.get("note") or "") prompt = hook_preamble(topic, note, stamp) + profile, missing = hook_resolved_profile(topic, note, stamp) rows.append(render_webhook_row( topic, - [display_event_expiry(entry.get("expiresIn"))], + [display_event_expiry(entry.get("expiresIn")), + display_hook_profile(profile, missing)], # Only when the prompt could not be rendered: then the note is # the one thing left that says why this watch exists. "" if prompt else note, @@ -6323,6 +6453,11 @@ def render_page(message="", kind="ok"): # and the panel below it — the same rule the subscription pass above # states, for the same reason. profiles = read_profiles() + # Which of those profiles a standing watch is actually spending right + # now (#582) - computed once here, from the same `watches` the webhooks + # panel below already walks, so the profiles panel's delete warning and + # that panel's per-row badge never disagree about what "in use" means. + usage = profile_usage(watches) return ( render_head("Settings — " + html.escape(USER)) + STYLE @@ -6339,7 +6474,7 @@ def render_page(message="", kind="ok"): base=html.escape(BASE), harnesses=render_harness_options(), effort=render_effort_options(), - profiles=render_profiles(profiles), + profiles=render_profiles(profiles, usage), ), webhooks_section=( WEBHOOK_UNAVAILABLE_TPL.format(text=unavailable) diff --git a/tests/golden/web/payloads/agent-box-webhook-spawn/bin/agent-box-webhook-spawn b/tests/golden/web/payloads/agent-box-webhook-spawn/bin/agent-box-webhook-spawn index 1724e5dd..d2841cc6 100644 --- a/tests/golden/web/payloads/agent-box-webhook-spawn/bin/agent-box-webhook-spawn +++ b/tests/golden/web/payloads/agent-box-webhook-spawn/bin/agent-box-webhook-spawn @@ -701,15 +701,16 @@ fi # the event. Older receivers set nothing, and an entry with no config sets `{}`, # so both read as "no watch-level answer" and the box-wide setting stands. # -# --preamble has no delivery and so no variable: the settings page asks what a -# named TOPIC would start, so that mode reads the watch straight out of the -# dispatch file. Same precedence either way, because the page must not advertise -# a worker the spawn would not use. +# --preamble and --resolved-profile have no delivery and so no variable: the +# settings page asks what a named TOPIC would start, so both modes read the +# watch straight out of the dispatch file. Same precedence either way, +# because the page must not advertise a worker the spawn would not use. watch_profile="" watch_config="" if [ -n "${LOCAL_WEBHOOK_SPAWN_CONFIG:-}" ]; then watch_config="$LOCAL_WEBHOOK_SPAWN_CONFIG" -elif [ "${1:-}" = "--preamble" ] && [ -n "${2:-}" ] && [ -n "${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then +elif { [ "${1:-}" = "--preamble" ] || [ "${1:-}" = "--resolved-profile" ]; } \ + && [ -n "${2:-}" ] && [ -n "${LOCAL_WEBHOOK_STATE_DIR:-}" ]; then # Best effort, like every other read here: no file, bad JSON or no such topic # all mean "this watch names no profile", never a failed render. watch_config=$("$JQ" -r --arg t "$2" \ @@ -746,6 +747,12 @@ profile_problem() { hook_profile="" hook_profile_source="" hook_profile_ignored="" +# Bare names alongside the prose above, for --resolved-profile (#582): a +# consumer that wants to warn about a named-but-missing profile needs the +# NAME, and hook_profile_ignored only ever carries it wrapped in a sentence +# meant for a human. Names are charset-restricted by profile_problem before +# they land here, so a plain space-joined list round-trips with no quoting. +hook_profile_ignored_names="" try_profile() { [ -n "$1" ] || return 0 [ -z "$hook_profile" ] || return 0 @@ -757,6 +764,7 @@ try_profile() { fi echo "agent-box-webhook-spawn: ignoring profile '$1' from $2 — $why" >&2 hook_profile_ignored="${hook_profile_ignored:+$hook_profile_ignored; }$2 — IGNORED, $why" + hook_profile_ignored_names="${hook_profile_ignored_names:+$hook_profile_ignored_names }$1" } try_profile "$watch_profile" "this watch's own spawnConfig.profile" @@ -851,6 +859,24 @@ ${AGENT_BOX_HOOK_ARGS_OPTION_NAME:-the fleet-wide default}, which is the \ fallback." } +# --resolved-profile TOPIC [NOTE]: print, as one JSON object, the profile a +# match on TOPIC would actually use right now — {"profile": NAME or null, +# "missing": [NAME, ...]} — and spawn nothing. Same resolution as a real +# spawn and as --preamble's prose (the precedence above already ran by the +# time this checks $1), but MACHINE-readable: the settings page needs a bare +# name to badge a watch's row and to warn a profile delete out from under a +# live watch (#582), and scraping that out of --preamble's sentences would +# break every time the wording does. `missing` lists every named-but-unusable +# profile that was tried and passed over, most specific (the watch's own) +# first — empty when nothing was named, distinct from `profile` being null +# for "nothing resolved, so the box default harness starts". +if [ "${1:-}" = "--resolved-profile" ]; then + "$JQ" -n --arg p "$hook_profile" --arg m "$hook_profile_ignored_names" \ + '{profile: (if $p == "" then null else $p end), + missing: (if $m == "" then [] else ($m | split(" ")) end)}' + exit 0 +fi + # --preamble TOPIC [NOTE]: print what a match on TOPIC would launch — the # launch command first, then the prompt — and spawn nothing. Everything a # delivery decides is left as a : the event key names the session diff --git a/tests/webhook.nix b/tests/webhook.nix index 8d42e55d..e02538f3 100644 --- a/tests/webhook.nix +++ b/tests/webhook.nix @@ -1241,6 +1241,18 @@ f" {spawn_cmd} --preamble 'github:defangdevs/*'" ) assert "IGNORED, no such profile" in launch, launch + # ...and --resolved-profile says the same thing with no prose to parse: + # nothing usable resolved (a match starts the box default agent), but + # the box-wide name that was TRIED is still reported, so a caller can + # say "your AGENT_BOX_HOOK_PROFILE names a profile that no longer + # exists" rather than just falling silent. + resolved = machine.succeed( + "sudo -u agent env HOME=/home/agent" + f" {spawn_cmd} --resolved-profile 'github:defangdevs/*'" + ) + assert json.loads(resolved) == { + "profile": None, "missing": ["triage"], + }, resolved machine.succeed( f"sudo -u agent env HOME=/home/agent agent-box-session rm {ghosted}" ) @@ -1348,6 +1360,15 @@ ) assert "agent profile watchbot" in launch, launch assert "spawnConfig.profile" in launch, launch + # --resolved-profile answers the same question as MACHINE-readable JSON + # (#582), for a caller that wants the bare name rather than a sentence to + # scrape it out of — the settings page badges a watch's row with this. + resolved = machine.succeed( + "sudo -u agent env HOME=/home/agent" + " LOCAL_WEBHOOK_STATE_DIR=/home/agent/.local/state/local-webhook" + f" {spawn_cmd} --resolved-profile github:defangdevs/watch-profile" + ) + assert json.loads(resolved) == {"profile": "watchbot", "missing": []}, resolved # A topic with no watch-level profile still reports the box-wide one. launch = machine.succeed( "sudo -u agent env HOME=/home/agent" @@ -1355,6 +1376,12 @@ f" {spawn_cmd} --preamble github:defangdevs/no-watch-profile" ) assert "agent profile boxwide" in launch, launch + resolved = machine.succeed( + "sudo -u agent env HOME=/home/agent" + " LOCAL_WEBHOOK_STATE_DIR=/home/agent/.local/state/local-webhook" + f" {spawn_cmd} --resolved-profile github:defangdevs/no-watch-profile" + ) + assert json.loads(resolved) == {"profile": "boxwide", "missing": []}, resolved # An unusable WATCH profile must not cost the box-wide one. It used to: # the watch's name displaced the box-wide one BEFORE validation, so one @@ -1368,6 +1395,19 @@ ) assert "agent profile boxwide" in launch, launch assert "IGNORED, no such profile" not in launch, launch + # ...and --resolved-profile names the box-wide profile it fell back to + # AND the watch-level name it passed over — so a delete-time warning and + # a row badge can both say "boxwide (and 'ghost' not found)" rather than + # only ever seeing the winner. + resolved = machine.succeed( + "sudo -u agent env HOME=/home/agent" + " LOCAL_WEBHOOK_STATE_DIR=/home/agent/.local/state/local-webhook" + " LOCAL_WEBHOOK_SPAWN_CONFIG='{\"profile\": \"ghost\"}'" + f" {spawn_cmd} --resolved-profile github:defangdevs/no-watch-profile" + ) + assert json.loads(resolved) == { + "profile": "boxwide", "missing": ["ghost"], + }, resolved # A non-string value is ignored rather than stringified into a name. # webhook.py drops one when it reads the filter file, so no delivery # carries it — but that file is documented as hand-editable, and @@ -1886,6 +1926,10 @@ "1 event notification", # and counted on that session's row f'data-fold="subs-{hook_name}"', # a spawned session's seeded topic "Automatic session rule", # the shared dispatch list, its own panel + # #582: which profile a standing watch starts belongs on the ROW, + # not only in the fold an operator has to open to find it. + "Profile: panelbot", # panel-profile's own worker... + "No profile - box default agent", # ...and panel's, which names none # The endpoint half of the same panel: what to register in the # sender, per configured source, paste-ready with a copy button. # Until this, both halves were reachable only by running @@ -1915,8 +1959,13 @@ ) # The standing watches are NOT session-scoped, so they are the one thing - # that must not have moved into a session's fold. - watches = page.split("Standing watch")[-1] + # that must not have moved into a session's fold. Split on the panel's + # OWN heading, not the "Standing watch" wording the panel used to carry: + # that string left this page entirely when the panel was renamed to + # "Automations" (#259), so the split matched nothing and silently + # returned the whole page - every assertion below still passed, just + # with no isolation from the Sessions and Profiles panels above it. + watches = page.split("

    Automations

    ")[-1] assert "github:defangdevs/agent-box" in watches, watches # A watch row says what a match DOES, not why someone subscribed (#259): @@ -1928,6 +1977,18 @@ assert "You are a fresh agent session started by" in watches, watches assert "("standing watch: triage")" in watches, watches assert "hook-<key>-<hex>" in watches, watches + # And the row for panel-profile's own watch carries its badge, isolated + # to the Automations panel the same way (#582). + assert "Profile: panelbot" in watches, watches + + # The Profiles panel says the reverse (#582): panelbot is spent by a + # live watch, so deleting it is not a no-op, and the row says so before + # anyone has to press the button to find out. + profiles_html = page.split("

    Profiles

    ", 1)[-1].split( + "

    Automations

    ", 1)[0] + assert "used by 1 standing watch" in profiles_html, profiles_html + assert ("It is used by 1 standing watch: github:defangdevs/panel-profile" + in profiles_html), profiles_html # And the note is no longer a paragraph on the row itself. assert 'wh-note">standing watch: triage' not in page, page