diff options
| author | user <user@clank> | 2026-08-17 13:38:15 +0200 |
|---|---|---|
| committer | user <user@clank> | 2026-08-17 13:38:15 +0200 |
| commit | ea39e09a52240ce1d5166e9137e39b7caad499fd (patch) | |
| tree | 8f841f2ff3f14abce054136ac9a4cf3fe47162fe | |
| parent | sl0ppty: a skill for driving it, and the two variables that made one possible (diff) | |
| download | slosh-ea39e09a52240ce1d5166e9137e39b7caad499fd.tar.gz slosh-ea39e09a52240ce1d5166e9137e39b7caad499fd.tar.xz slosh-ea39e09a52240ce1d5166e9137e39b7caad499fd.zip | |
sl0ppty: a --script pane is told no session rather than somebody else's
`server.c` set `SL0PPTY_SESSION` for its panes and carried a comment saying the
value was "deliberately unset under `--script`". Nothing unset it. A value never
set is a value inherited, so a script driver started from inside a pane handed its
own panes the *outer* session's name -- and the recipe the skill documents,
`-s ${SL0PPTY_SESSION:-main} cmd`, then sent its commands to a session it was not
in. Not a fallback to a guess: a confident wrong answer, because the name it read
was a real session that really answers.
Both variables now go through one `session_env()`, called by the server with its
name and by the headless driver with NULL, because the two modes have to agree
about what a pane is told and a comment in one of them is not agreement.
`SL0PPTY_BIN` is set there too rather than inherited, so it names the binary that
actually made the pane instead of one that made some pane somewhere -- which is
the whole reason that variable exists.
Found while explaining a stale pane title, from a check in `test_skill.py` that
failed under `make test` and passed on its own. It was not flaky: the suite runs
from inside a pane, where these are set, and the tool I ran it from by hand had an
environment where they were not. A test whose answer depends on where you run it
looks exactly like a race.
So it now passes the lie in deliberately: the session under test is started with
`SL0PPTY_SESSION=somebody-elses-session`, which makes the difference between
"never set" and "cleared" visible from any shell. The mirror check is next to it --
a real session tells its panes its *own* name, even when started from inside
another one -- so the obvious wrong fix, unsetting it in both modes, fails. Both
were verified by making both wrong versions and watching each test bite.
The two title checks now wait for the line instead of sampling after 120ms. That
was not the bug (a shell prints in about 12ms here, even under disk churn) and it
is one assumption fewer.
| -rw-r--r-- | .agents/skills/driving-sl0ppty/SKILL.md | 15 | ||||
| -rw-r--r-- | src/headless.c | 6 | ||||
| -rw-r--r-- | src/server.c | 52 | ||||
| -rw-r--r-- | src/server.h | 4 | ||||
| -rwxr-xr-x | tests/test_skill.py | 62 |
5 files changed, 105 insertions, 34 deletions
diff --git a/.agents/skills/driving-sl0ppty/SKILL.md b/.agents/skills/driving-sl0ppty/SKILL.md index 150c3e8..1aa4997 100644 --- a/.agents/skills/driving-sl0ppty/SKILL.md +++ b/.agents/skills/driving-sl0ppty/SKILL.md @@ -32,12 +32,19 @@ Every pane gets these: | variable | means | |---|---| | `SL0PPTY=1` | this process is running inside a sl0ppty pane | -| `SL0PPTY_SESSION` | the name of the session it is in | +| `SL0PPTY_SESSION` | the name of the session it is in — empty under `--script` | | `SL0PPTY_BIN` | the binary that started it | Use `$SL0PPTY_BIN`, not `sl0ppty`: a session may have been started from a build -tree, and `sl0ppty` is then not on your `PATH`. `SL0PPTY_SESSION` is unset under -`--script`, which has no socket. +tree, and `sl0ppty` is then not on your `PATH`. Both are *set* by the thing that +made your pane, never inherited from whatever started it: `SL0PPTY_SESSION` is +unset under `--script`, which has no socket, rather than carrying the name of a +session the pane is not in. So empty means there is nothing to send commands to, +and it is worth checking before you build a command line out of it: + +```bash +[ -n "$SL0PPTY_SESSION" ] || { echo "no session to talk to"; exit 1; } +``` From *outside* a pane, list sessions: @@ -55,7 +62,7 @@ distinguishable from a call that answered `{"ok":false}`. ```bash SL=${SL0PPTY_BIN:-sl0ppty} -S="$SL -s ${SL0PPTY_SESSION:-main} cmd" +S="$SL -s $SL0PPTY_SESSION cmd" # empty means no session, not `main` $S '{"cmd":"panes"}' $S '{"cmd":"new-tab","name":"build"}' diff --git a/src/headless.c b/src/headless.c index 451996c..e1fcaf5 100644 --- a/src/headless.c +++ b/src/headless.c @@ -18,6 +18,7 @@ #include "app.h" #include "cmd.h" +#include "server.h" #define MAX_PANES 64 @@ -59,6 +60,11 @@ static void settle(app_t *a, int quiet, screen_t *snap) { int run_headless(const char *const argv[], uint16_t cols, uint16_t rows, int idle_ms, bool script, const char *layout) { + /* No socket, so no session name -- and explicitly not one inherited from a + * session this driver merely happens to have been started inside. See + * session_env(). */ + session_env(NULL); + app_t *a = app_new(argv, cols, rows); if (!a) { fprintf(stderr, "sl0ppty: cannot spawn pane\n"); diff --git a/src/server.c b/src/server.c index f552f30..f728036 100644 --- a/src/server.c +++ b/src/server.c @@ -237,6 +237,39 @@ static bool watch_hit(const watchset_t *w, const char *name) { return false; } +/* What every pane is told about the session around it. Panes inherit this + * process's environment, so this is how a program *inside* one learns which + * session it is in -- and therefore which socket to talk to. `SL0PPTY=1` + * (pty.c) says "you are in a pane"; this says which one of possibly several, + * which is the part a program cannot work out for itself: `ls` lists them all + * and none of them is labelled "the one you are in". + * + * NULL for a mode with no socket, and that case is why this is a function both + * modes call rather than two setenvs in the mode that has one. `--script` has + * always been *documented* as leaving `SL0PPTY_SESSION` unset -- but a value it + * never set was a value it inherited, and a script driver started from inside a + * pane handed its own panes the *outer* session's name. Anything following the + * documented recipe (`-s ${SL0PPTY_SESSION:-main} cmd`) then sent its commands + * to a session it was not in, confidently, because the name it read was real. + * + * `SL0PPTY_BIN` is set in both modes for the same reason it is set in either: a + * program in a pane that finds `sl0ppty` missing from its PATH -- a session + * started from a build tree, an agent with a trimmed environment -- can + * otherwise see the session it is in and have no way to reach it. Read from + * /proc rather than argv[0], which is whatever the caller felt like passing, + * and set rather than inherited so it names the binary that actually made this + * pane, not one that made some pane somewhere. */ +void session_env(const char *name) { + if (name) setenv("SL0PPTY_SESSION", name, 1); + else unsetenv("SL0PPTY_SESSION"); + char self[512]; + ssize_t sn = readlink("/proc/self/exe", self, sizeof self - 1); + if (sn > 0) { + self[sn] = 0; + setenv("SL0PPTY_BIN", self, 1); + } +} + int server_run(const char *name, const char *const argv[], uint16_t cols, uint16_t rows, const char *layout, bool watch) { char path[512]; @@ -247,24 +280,7 @@ int server_run(const char *name, const char *const argv[], uint16_t cols, return 1; } - /* Every pane inherits this process's environment, so setting it here is how a - * program *inside* a pane learns which session it is in -- and therefore which - * socket to talk to. `SL0PPTY=1` (pty.c) says "you are in a pane"; this says - * which one of possibly several sessions, which is the part a program cannot - * work out for itself: `ls` lists them all and none of them is labelled "the - * one you are in". Deliberately unset under `--script`, which has no socket. */ - setenv("SL0PPTY_SESSION", name, 1); - /* ...and which binary to talk to it with. A program in a pane that finds - * `sl0ppty` missing from its PATH -- a session started from a build tree, an - * agent with a trimmed environment -- can otherwise see the session it is in - * and have no way to reach it. Read from /proc rather than argv[0], which is - * whatever the caller felt like passing. */ - char self[512]; - ssize_t sn = readlink("/proc/self/exe", self, sizeof self - 1); - if (sn > 0) { - self[sn] = 0; - setenv("SL0PPTY_BIN", self, 1); - } + session_env(name); struct sigaction sa = {.sa_handler = on_sig}; sigaction(SIGTERM, &sa, NULL); diff --git a/src/server.h b/src/server.h index 3e7de4c..6b58aa6 100644 --- a/src/server.h +++ b/src/server.h @@ -10,6 +10,10 @@ /* Run the session in this process (the daemonised half). `watch` re-reads the * config when the file changes; see server.c for why that is a flag and not a * config setting. */ +/* What panes are told about the session around them: its name, or NULL for a + * mode that has no socket, which must clear it rather than inherit a lie. */ +void session_env(const char *name); + int server_run(const char *name, const char *const argv[], uint16_t cols, uint16_t rows, const char *layout, bool watch); /* Fork a server, then wait for its socket to answer. Returns a connected fd. */ diff --git a/tests/test_skill.py b/tests/test_skill.py index 5bf348b..edb3303 100755 --- a/tests/test_skill.py +++ b/tests/test_skill.py @@ -14,6 +14,8 @@ import os import re import subprocess import sys +import tempfile +import time sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) from harness import BIN, Session, check, report @@ -70,23 +72,59 @@ def test_the_environment_it_promises_is_the_environment_panes_get(): for var in ("SL0PPTY", "SL0PPTY_SESSION", "SL0PPTY_BIN"): check("the skill documents " + var, var in body, "missing " + var) - # SL0PPTY is set by pty.c for every pane, in every mode. - with Session(["/bin/sh", "-c", 'echo "V=[$SL0PPTY]"; read x']) as s: - s.settle(120) - check("a pane really is told it is in one", - "V=[1]" in s.snapshot().screen(), s.snapshot().screen()) - - # The other two are set by the server, which --script is not. The skill says - # so; this is that sentence being true. - with Session(["/bin/sh", "-c", 'echo "S=[$SL0PPTY_SESSION]"; read x']) as s: - s.settle(120) - check("and that --script has no session to name", - "S=[]" in s.snapshot().screen(), s.snapshot().screen()) + # Started from an environment that already names a session and a binary, + # because that is the case the claim is *about*. This suite normally runs + # inside a pane, where both are set, so a driver that merely never sets them + # is indistinguishable from one that clears them -- until it hands its own + # panes the outer session's name and a script in there sends its commands + # somewhere else. Passing the lie in makes the difference visible from any + # shell, rather than only from one that happens to have them set. + lie = {"SL0PPTY_SESSION": "somebody-elses-session", "SL0PPTY_BIN": "/nope/x"} + probe = ["/bin/sh", "-c", + 'echo "V=[$SL0PPTY] S=[$SL0PPTY_SESSION] B=[$SL0PPTY_BIN]"; read x'] + with Session(probe, cols=120, rows=8, env=lie) as s: + s.until_text("V=[") + screen = s.snapshot().screen() + check("a pane really is told it is in one", "V=[1]" in screen, screen) + check("--script has no socket, so it names no session", + "S=[]" in screen, screen) + check("...and clears an inherited one rather than passing it on", + "somebody-elses-session" not in screen, screen) + check("the binary is the one that made the pane, not an inherited path", + "/nope/x" not in screen and "sl0ppty" in screen.split("B=[")[1], + screen) flat = " ".join(body.split()) # the file is wrapped; the claim is not check("which the skill says out loud", "unset under `--script`" in flat, "not documented") +def test_a_real_session_names_itself_to_its_panes(): + """The other half, and the reason `--script` clearing it is not just + `unsetenv` everywhere: a session *does* tell its panes which one it is, and it + is its own name even when it was started from inside another session.""" + out = os.path.join(tempfile.mkdtemp(prefix="sl0ppty-env-"), "seen") + name = "envclaim-%d" % os.getpid() + env = dict(os.environ) + env["SL0PPTY_SESSION"] = "somebody-elses-session" + try: + subprocess.run([BIN, "-s", name, "--", "/bin/sh", "-c", + 'echo "S=[$SL0PPTY_SESSION]" > %s; read x' % out], + capture_output=True, text=True, timeout=30, input="", env=env) + deadline = time.time() + 5 + seen = "" + while time.time() < deadline: + if os.path.exists(out): + seen = open(out).read().strip() + if seen: + break + time.sleep(0.05) + check("a pane in a real session is told which session that is", + seen == "S=[%s]" % name, repr(seen)) + finally: + subprocess.run([BIN, "-s", name, "cmd", '{"cmd":"quit"}'], + capture_output=True, text=True) + + def test_the_panes_fields_it_tells_agents_to_poll_are_real(): """The skill's central recipe polls `alive` and `exit_code` off `panes` and filters on `purpose`. If any of those three moved, the recipe is a hang.""" |
