diff --git a/RELEASE.md b/RELEASE.md index 224d0c9..f99e10b 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -8,6 +8,57 @@ shows you how to drive the shell. --- +## v2.7.5 + +**Fixed** + +- **The shell notices a release published since its last check** (#62). The + report was a screenshot: + + hellish 2.7.3 ... + ✓ 2.7.3 up to date · via user binary · 50m ago + + 2.7.4 was out. The shell had checked 50 minutes earlier, when 2.7.3 really + was the newest thing there was — and the check interval was a flat 24 + hours, so it would not look again until the next day. Every session in + between reported "up to date" with complete confidence, and the only way + to find out was typing `update` by hand: the exact chore a background + check exists to remove. + + The interval is now adaptive, because the two states are not the same + question. When an update is **already known pending** there is nothing + left to learn — the badge is on your prompt and the banner has said so — + and it keeps the long interval. When the shell **believes it is current**, + that is the only state in which a release can exist without it knowing, so + it looks again every quarter of an hour instead. + + The asymmetry is what keeps it cheap: the frequent interval applies only + while there is genuinely something to find, and stops the moment it is + found. If you have an update pending, this changes nothing — still one + request a day. + + Two guards came with it, because "check more often" must not become + "check on every shell". A failed check now backs off like a successful + one, instead of re-firing on every startup forever on a machine that + cannot reach the release server. And the attempt is claimed before the + fork, so twenty terminals opened at once make one request rather than + twenty. + + Unchanged: the check is a detached child and the prompt never waits on the + network. A dead release server still costs the shell nothing. + +**Tests** + +`update_freshness_test.py` drives real ptys against a counting local release +server — no network, so it is deterministic anywhere. It pins the report +itself, the discovering session announcing the release without a restart, +the badge on the next one, the absence of a re-check when an update is +already known, six concurrent shells making at most two requests, a failed +check recording the attempt but not claiming a success, and startup timing +against a black-holed endpoint. + +--- + ## v2.7.4 Two bug reports from real sessions. One of them could leave your terminal diff --git a/incs/update.h b/incs/update.h index e0fc571..9310310 100644 --- a/incs/update.h +++ b/incs/update.h @@ -47,6 +47,7 @@ typedef struct s_upd_state long header_rev; char header_ver[64]; char announced[64]; + long attempted; } t_upd_state; /* Load the persisted update state; zeroes `s` and returns 0 when absent. */ @@ -118,6 +119,14 @@ int read_cached_latest(char *out, size_t n); fetches the latest tag from GitHub and rewrites the cache. Never blocks. */ void maybe_spawn_update_check(t_shell *state); +/* Should we ask the release server again, and may we? See update_gate.c: + the interval is short while we believe we are current and long once an + update is already known, and the attempt is claimed before forking so + many shells starting at once make one request. */ +long check_interval(const t_upd_state *s); +int cache_is_fresh(void); +void claim_attempt(void); + /* The background worker: fetch the latest tag and write it to the cache. */ void run_bg_update_check(void); diff --git a/incs/version.h b/incs/version.h index ecb1a8a..b5ed479 100644 --- a/incs/version.h +++ b/incs/version.h @@ -15,7 +15,7 @@ /* The single source of truth for the running shell's version. Bumped in step with the git tag / GitHub release / npm package / docker image. */ -# define HELLISH_VERSION "2.7.4" +# define HELLISH_VERSION "2.7.5" /* Where releases live; used by the `update` builtin and the daily check. */ # define HELLISH_REPO "Univers42/hellish" diff --git a/npm/package.json b/npm/package.json index 14c5bd3..171e943 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "hellish-shell", - "version": "2.7.4", + "version": "2.7.5", "description": "hellish — a fast, POSIX-compliant shell, with horns", "bin": { "hellish": "bin/hellish.js" diff --git a/src/platform/posix/update_cache.c b/src/platform/posix/update_cache.c index 09a36ac..465e676 100644 --- a/src/platform/posix/update_cache.c +++ b/src/platform/posix/update_cache.c @@ -40,26 +40,6 @@ long update_last_check_age(void) return ((long)time(NULL) - s.checked); } -/* True when the last check is recent enough to skip a new one. A missing - record is "stale", so the very first interactive run starts a check. - HELLISH_UPDATE_TTL overrides the interval (seconds) -- the test suite - uses it to force a re-check without waiting a day. */ -static int cache_is_fresh(void) -{ - long age; - const char *ttl; - long limit; - - age = update_last_check_age(); - if (age < 0) - return (0); - limit = 86400; - ttl = getenv("HELLISH_UPDATE_TTL"); - if (ttl && *ttl) - limit = ft_atoi(ttl); - return (age <= limit); -} - /* Interactive only: if the last check is stale, fork a fully detached child (double-fork) to refresh it in the background. Returns at once; the prompt is never delayed by the network, which is the hard requirement in issue @@ -73,6 +53,7 @@ void maybe_spawn_update_check(t_shell *state) return ; if (getenv("HELLISH_NO_UPDATE_CHECK") || cache_is_fresh()) return ; + claim_attempt(); pid = fork(); if (pid != 0) { diff --git a/src/platform/posix/update_gate.c b/src/platform/posix/update_gate.c new file mode 100644 index 0000000..bfba199 --- /dev/null +++ b/src/platform/posix/update_gate.c @@ -0,0 +1,88 @@ +/* ************************************************************************** */ +/* */ +/* ::: :::::::: */ +/* update_gate.c :+: :+: :+: */ +/* +:+ +:+ +:+ */ +/* By: dlesieur +#+ +:+ +#+ */ +/* +#+#+#+#+#+ +#+ */ +/* Created: 2026/08/23 00:00:00 by dlesieur #+# #+# */ +/* Updated: 2026/08/23 00:00:00 by dlesieur ### ########.fr */ +/* */ +/* ************************************************************************** */ + +#include "update.h" +#include +#include + +/* Split from update_cache.c only because the norm caps a file at 5 + functions. This is the whole of "should we look again, and may we". */ + +/* How long to wait before asking again, and the two answers are not the + same question. + + ALREADY PENDING: nothing left to learn. The badge is on the prompt and + the banner has announced it; a second opinion changes nothing a user can + see. A day is plenty. + + BELIEVED CURRENT: this is the ONLY state in which a release can exist + without us knowing, so it is the only one where asking buys anything. + A flat day here is what produced issue #62 -- checked at noon, 2.7.4 + published at half past, and every session until the next day reported + "up to date" with total confidence. A quarter of an hour instead. + + The asymmetry is what keeps it cheap: the frequent interval applies only + while there is genuinely something to find, and stops the moment it is + found. HELLISH_UPDATE_TTL still overrides both, which is how the older + tests force a re-check without waiting. */ +long check_interval(const t_upd_state *s) +{ + const char *ttl; + + ttl = getenv("HELLISH_UPDATE_TTL"); + if (ttl && *ttl) + return (ft_atoi(ttl)); + if (update_available(s)) + return (86400); + return (900); +} + +/* True when the last ATTEMPT is recent enough to skip a new one. + + Attempts, not successes. run_bg_update_check() writes `checked` only + after it has actually learned something, so keying the interval off it + meant a machine that could not reach the release server re-forked a + check on every single startup, forever -- the one shape of this code + that really would hammer. `attempted` backs off on failure too, while + `checked` keeps its meaning of "last time we learned something", which + is what the banner's "50m ago" reports. A state file from an older + hellish has no `attempted`, so fall back to `checked` rather than + treating it as never-attempted and stampeding on first run. */ +int cache_is_fresh(void) +{ + t_upd_state s; + long last; + + if (!update_state_load(&s)) + return (0); + last = s.attempted; + if (last <= 0) + last = s.checked; + if (last <= 0) + return (0); + return ((long)time(NULL) - last <= check_interval(&s)); +} + +/* Claim the check before forking, in the PARENT. + Twenty terminals opened at once would otherwise all read the same stale + record and all fork their own request. Recording the attempt here makes + the first one win and the other nineteen see a fresh cache. It is one + small temp-file-and-rename, at most once per interval, and it happens + off the prompt's path entirely. */ +void claim_attempt(void) +{ + t_upd_state s; + + update_state_load(&s); + s.attempted = (long)time(NULL); + update_state_save(&s); +} diff --git a/src/platform/posix/update_state.c b/src/platform/posix/update_state.c index 46833cb..52dbcdb 100644 --- a/src/platform/posix/update_state.c +++ b/src/platform/posix/update_state.c @@ -82,6 +82,8 @@ static void state_set(t_upd_state *s, char *key, char *val) ft_strlcpy(s->header_ver, val, sizeof(s->header_ver)); else if (!ft_strcmp(key, "announced")) ft_strlcpy(s->announced, val, sizeof(s->announced)); + else if (!ft_strcmp(key, "attempted")) + s->attempted = ft_atoi(val); } /* Split the file into lines and feed each key=value pair to state_set. */ diff --git a/src/platform/posix/update_state2.c b/src/platform/posix/update_state2.c index b1f3522..438d018 100644 --- a/src/platform/posix/update_state2.c +++ b/src/platform/posix/update_state2.c @@ -27,6 +27,18 @@ and a foreground `update` can run at the same time, and a half-written record read by the prompt would announce a version that does not exist; rename is atomic, so a reader sees either the old record or the new one. */ +/* The record itself, one key=value per line. Split out only because the + norm caps a function at 25 lines and the field list keeps growing. */ +static int format_record(const t_upd_state *s, char *buf, size_t n) +{ + return (ft_snprintf(buf, n, "latest=%s\nchecked=%d\n" + "notified=%d\nheader_shown=%d\nheader_rev=%d\n" + "header_ver=%s\nannounced=%s\nattempted=%d\n", s->latest, + (int)s->checked, (int)s->notified, (int)s->header_shown, + (int)s->header_rev, s->header_ver, s->announced, + (int)s->attempted)); +} + int update_state_save(const t_upd_state *s) { char path[512]; @@ -43,11 +55,7 @@ int update_state_save(const t_upd_state *s) fd = open(tmp, O_CREAT | O_TRUNC | O_WRONLY, 0644); if (fd < 0) return (0); - len = ft_snprintf(buf, sizeof(buf), "latest=%s\nchecked=%d\n" - "notified=%d\nheader_shown=%d\nheader_rev=%d\n" - "header_ver=%s\nannounced=%s\n", s->latest, (int)s->checked, - (int)s->notified, (int)s->header_shown, (int)s->header_rev, - s->header_ver, s->announced); + len = format_record(s, buf, sizeof(buf)); if (len <= 0 || write(fd, buf, (size_t)len) != len) return (close(fd), unlink(tmp), 0); close(fd); diff --git a/tests/update_freshness_test.py b/tests/update_freshness_test.py new file mode 100755 index 0000000..82d93ed --- /dev/null +++ b/tests/update_freshness_test.py @@ -0,0 +1,298 @@ +#!/usr/bin/env python3 +"""Regression test: a release published AFTER the last check -- issue #62. + +Reported twice, and the second time with a screenshot that says it all: + + hellish 2.7.3 ... + ✓ 2.7.3 up to date · via user binary · 50m ago + +2.7.4 was out. The shell had checked 50 minutes earlier, when 2.7.3 really +was the newest thing there was, and cache_is_fresh() used a flat 24 hour +TTL -- so it would not look again until the next day. Every session in +between confidently reported "up to date". The only way out was typing +`update` by hand, which is exactly what a background update check exists to +save you from. + +The interval is now adaptive, because the two states are not the same +question: + + * an update is ALREADY known pending -- there is nothing to learn, the + badge is on the prompt and the banner has said so. Keep the long + interval; asking again buys nothing. + * we believe we are CURRENT -- this is the ONLY state in which a new + release can exist without us knowing, so it is the only one where + asking is worth anything. Short interval. + +That asymmetry is what makes this cheap: the frequent case is one request +per quarter hour per machine, from a detached child, and it stops entirely +the moment an update is found. + +Two hammering guards come with it, because "check more often" must not +become "check on every shell": + + * the attempt is recorded BEFORE the fetch, so a failing network backs + off like a successful one instead of re-firing on every single startup; + * and it is recorded by the PARENT, so twenty terminals opened at once + produce one request rather than twenty. + +`attempted` is kept separate from `checked` on purpose: `checked` still +means "last time we successfully learned something", which is what the +banner's "50m ago" reports. A failed attempt must not be able to claim it. + +Usage: python3 update_freshness_test.py /path/to/hellish +""" +import fcntl +import http.server +import os +import pty +import re +import select +import shutil +import struct +import subprocess +import sys +import tempfile +import termios +import threading +import time + +SHELL = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 + else "build/bin/hellish") +FAILS = [] +ESC = re.compile(r"\x1b\[[0-9;?]*[A-Za-z]|\x1b\][^\x07]*\x07") + + +def check(name, ok, detail=""): + print(("ok " if ok else "FAIL ") + name + (" " + detail if not ok + else "")) + if not ok: + FAILS.append(name) + + +def version(): + out = subprocess.run([SHELL, "--version"], capture_output=True, + text=True, timeout=30).stdout + return out.split("version ", 1)[1].split()[0].strip(" ,") + + +RUNNING = version() + + +def bump(v, part=2): + n = [int(x) for x in v.split(".")[:3]] + n[part] += 1 + return ".".join(str(x) for x in n) + + +NEWER = bump(RUNNING) + + +class Server: + """A release endpoint that counts how many times it is asked.""" + + def __init__(self, tag): + self.hits = 0 + outer = self + + class H(http.server.BaseHTTPRequestHandler): + def do_GET(self): + outer.hits += 1 + body = ('{"tag_name": "v%s", "name": "r"}' % tag).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *a): + pass + + self.srv = http.server.HTTPServer(("127.0.0.1", 0), H) + self.url = "http://127.0.0.1:%d/releases/latest" % \ + self.srv.server_address[1] + threading.Thread(target=self.srv.serve_forever, daemon=True).start() + + def stop(self): + self.srv.shutdown() + + +def seed(cache, **kv): + d = os.path.join(cache, "hellish") + os.makedirs(d, exist_ok=True) + rec = {"latest": "", "checked": 0, "notified": 0, "header_shown": 0, + "header_rev": 0, "header_ver": "", "announced": "", "attempted": 0} + rec.update(kv) + with open(os.path.join(d, "state"), "w") as f: + for k, v in rec.items(): + f.write("%s=%s\n" % (k, v)) + + +def read_state(cache): + p = os.path.join(cache, "hellish", "state") + out = {} + if os.path.exists(p): + for line in open(p): + if "=" in line: + k, v = line.rstrip("\n").split("=", 1) + out[k] = v + return out + + +def session(cache, api, settle=3.0, cmds=(b"echo MARK\n",)): + env = {"HOME": os.environ.get("HOME", "/tmp"), "PATH": os.environ["PATH"], + "TERM": "xterm-256color", "LANG": "C.UTF-8", + "XDG_CACHE_HOME": cache, "ASAN_OPTIONS": "detect_leaks=0", + "HELLISH_UPDATE_API": api} + t0 = time.time() + pid, fd = pty.fork() + if pid == 0: + os.environ.clear() + os.environ.update(env) + os.execv(SHELL, [SHELL]) + os._exit(127) + fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH", 40, 150, 0, 0)) + out = b"" + first = None + end = time.time() + settle + while time.time() < end: + r, _, _ = select.select([fd], [], [], 0.05) + if r: + try: + chunk = os.read(fd, 65536) + except OSError: + break + if first is None and chunk.strip(): + first = time.time() - t0 + out += chunk + for c in cmds: + os.write(fd, c) + end = time.time() + 1.5 + while time.time() < end: + r, _, _ = select.select([fd], [], [], 0.05) + if r: + try: + out += os.read(fd, 65536) + except OSError: + break + try: + os.killpg(pid, 9) + except OSError: + pass + try: + os.kill(pid, 9) + os.waitpid(pid, 0) + except OSError: + pass + os.close(fd) + return ESC.sub("", out.decode(errors="replace")), (first or 99.0) + + +def main(): + print("running %s, pretending %s is released\n" % (RUNNING, NEWER)) + now = int(time.time()) + + # ── 1. THE REPORT. Checked 50 minutes ago, believed current, and a new + # release has landed since. The shell must look again. + srv = Server(NEWER) + cache = tempfile.mkdtemp() + try: + seed(cache, latest=RUNNING, checked=now - 3000, attempted=now - 3000, + header_shown=now, header_rev=3, header_ver=RUNNING, + announced=RUNNING) + txt, _ = session(cache, srv.url) + st = read_state(cache) + check("a 50-minute-old check does not block a new one", + srv.hits >= 1, "the endpoint was never asked") + check("the newer release is discovered", st.get("latest") == NEWER, + "state still says latest=%s" % st.get("latest")) + # The session that DISCOVERS the release must say so without being + # restarted -- the whole complaint is having to find out by hand. + # Either channel counts: the one-shot notice between commands, or + # the prompt badge once its few-second cache turns over. Which one + # wins is a timing detail; being told is not. + check("the discovering session says so, unprompted", + "⬆" in txt and NEWER in txt, + "nothing announced in the session that found it: %r" + % txt[-300:]) + txt2, _ = session(cache, srv.url, settle=2.0) + check("and the next session carries the badge", "⬆" + NEWER in txt2, + "no badge: %r" % txt2[-300:]) + finally: + srv.stop() + shutil.rmtree(cache, ignore_errors=True) + + # ── 2. Not hammering. Once an update IS known pending there is nothing + # left to learn, so a fresh attempt must not fire every session. + srv = Server(NEWER) + cache = tempfile.mkdtemp() + try: + seed(cache, latest=NEWER, checked=now - 3000, attempted=now - 3000, + header_shown=now, header_rev=3, header_ver=RUNNING, + announced=NEWER) + session(cache, srv.url, settle=2.0) + check("a known pending update does not trigger a re-check", + srv.hits == 0, "asked %d times with nothing to learn" % srv.hits) + finally: + srv.stop() + shutil.rmtree(cache, ignore_errors=True) + + # ── 3. A very recent attempt is still respected -- the short interval is + # an interval, not "every startup". + srv = Server(NEWER) + cache = tempfile.mkdtemp() + try: + seed(cache, latest=RUNNING, checked=now - 10, attempted=now - 10, + header_shown=now, header_rev=3, header_ver=RUNNING, + announced=RUNNING) + session(cache, srv.url, settle=2.0) + check("a 10-second-old attempt is still fresh", srv.hits == 0, + "asked %d times seconds after the last attempt" % srv.hits) + finally: + srv.stop() + shutil.rmtree(cache, ignore_errors=True) + + # ── 4. Single flight. Terminals opened together must not each fire. + srv = Server(NEWER) + cache = tempfile.mkdtemp() + try: + seed(cache, latest=RUNNING, checked=now - 3000, attempted=now - 3000, + header_shown=now, header_rev=3, header_ver=RUNNING, + announced=RUNNING) + ts = [threading.Thread(target=session, args=(cache, srv.url, 2.5)) + for _ in range(6)] + for t in ts: + t.start() + for t in ts: + t.join() + check("six shells at once make at most two requests", srv.hits <= 2, + "%d requests -- one release per terminal" % srv.hits) + finally: + srv.stop() + shutil.rmtree(cache, ignore_errors=True) + + # ── 5. A failing endpoint backs off instead of retrying every startup, + # and `checked` is NOT advanced by a failure -- the banner's "Xm + # ago" must keep meaning "last time we actually learned something". + cache = tempfile.mkdtemp() + try: + seed(cache, latest=RUNNING, checked=now - 3000, attempted=now - 3000, + header_shown=now, header_rev=3, header_ver=RUNNING, + announced=RUNNING) + _, first = session(cache, "http://127.0.0.1:9/dead", settle=2.5) + st = read_state(cache) + check("a failed check still records the attempt", + int(st.get("attempted", 0)) > now - 100, + "attempted=%s -- every startup would retry" % st.get("attempted")) + check("a failed check does not claim a successful one", + int(st.get("checked", 0)) <= now - 2000, + "checked=%s was advanced by a failure" % st.get("checked")) + check("a dead endpoint never delays startup", first < 1.0, + "first output took %.2fs" % first) + finally: + shutil.rmtree(cache, ignore_errors=True) + + print("\n%d checks failed" % len(FAILS)) + sys.exit(1 if FAILS else 0) + + +main() diff --git a/wiki/context.md b/wiki/context.md index c0d1969..5dd5fb6 100644 --- a/wiki/context.md +++ b/wiki/context.md @@ -5,7 +5,7 @@ later session. The repo history is authoritative; this file records the things history cannot tell you — why a thing was done, what was measured, what was deliberately *not* done, and what is still open. -Branch to resume from: **`develop`**. Released version: **2.7.4**. +Branch to resume from: **`develop`**. Released version: **2.7.5**. The previous note on this file covered the issue-fixing session (#27, #32, #34, #42 and friends). All of those are closed; the tracker is empty. This