mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-11-05 02:43:44 +00:00
codex/BUG-3994
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
0a32eff274 |
fix(scripts): install-refresh removes pad.pid only when it is provably not a live sibling's (BUG-4068) (#2311)
* fix(scripts): install-refresh removes pad.pid only when it is provably not a live sibling's (BUG-4068) The refresh removed <data dir>/pad.pid unconditionally after the install, whether or not it stopped anything and whoever the file named. A second server sharing the data dir lost its record, so `pad server stop` and `pad server status` could no longer find it. Now the file is removed when it names the pid this refresh stopped, or a pid that is provably dead (kill -0 fails and ps finds no such process, so EPERM is not read as dead). It is kept when it names any live process, when its liveness is unknown, and when it does not parse as a pid record (JSON or the legacy bare integer). A kept stale file is the safe failure: server stop refuses a record it cannot prove (BUG-2969). The BUG-4064 tests, which clear a dead-pid file with nothing stopped, stand unchanged. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * test(buildtools): pin install-refresh's pad.pid removal rule (BUG-4068) Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * fix(scripts,buildtools): pad.pid is read no more loosely than pad writes it, and only ESRCH is dead (BUG-4068 r1) r1 items, as ruled: 1, 3: pid_file_pid accepts only MarshalIndent's record (one "pid", at the two-space top-level indent) or a bare integer with surrounding whitespace, at most 10 digits. Duplicate or nested pid keys, compact JSON, "1 2" and huge values no longer parse, so the file is kept. 2: pid_liveness reads the builtin kill's own message under LC_ALL=C. Only "No such process" is dead; EPERM and anything else are unknown. ps is not asked, so a missing, busybox or hidepid-restricted ps cannot call a live sibling dead. 4: STOPPED_PID is recorded only when kill -TERM is delivered. 5: the decision is pid_file_removable, tested on its own, so the stopped-pid leg (a zombie or a quick reuse) has a row of its own. 6: the dead-pid row uses a reaped pid, not 999999. Also: the harness guard's testTempScope cuts the test name to 64 bytes, as Go's makeTempDir does. A top-level test name of 64 bytes or more was refused by its own temp dir; this unit's long test names found it. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * fix(scripts): pad.pid's reader checks the whole record's line shape and caps the pid (BUG-4068 r2) r2 items, as ruled: 1: pid_file_pid now checks the record's line SHAPE end to end, not only the pid line: first non-blank line exactly "{", last exactly "}", every line between a two-space-indent key line, one of them the pid. A "\r" is dropped only at a line end. This is still a shape check, not full JSON validation. The r1 commit's "accepts only MarshalIndent's record" said more than r1 did; this commit is what makes the shape claim true. 2: a pid above 4194304 (Linux's largest pid_max) does not parse, so it can never wrap as a pid_t. 3: pid_liveness states that ESRCH is per pid namespace. 5: the stopped-pid test comment is corrected: the e2e pre-server is a zombie until cleanup reaps it, so the e2e stopped-server rows reach that leg too. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * docs(scripts,buildtools): pid_file_pid's comments state the inner-line check exactly (BUG-4068 r3) Comment-only. pid_file_pid checks that each line between the braces STARTS like a two-space-indent quoted key; the rest of such a line is unchecked. The comments said "every line between a two-space-indent key line", and "read no more loosely than the SHAPE", which claimed more than the pattern checks (r3, ruled a decline: the code stays as it is after the close). Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV |
||
|
xarmian
|
cba880ca17 |
fix(config): warn on a relative PAD_DATA_DIR / PAD_DB_PATH; anchor install-refresh's marker and config reads (BUG-4066 phase 1) (#2308)
* fix(config): warn once per process on a relative PAD_DATA_DIR / PAD_DB_PATH; install-refresh anchors its marker and config reads (BUG-4066 phase 1)
config.Load takes PAD_DATA_DIR verbatim, or PAD_DB_PATH's dirname, with no
filepath.Abs, so every process resolves a relative value against its own cwd:
a CLI and a server started in different directories silently use different
data (database, attachments, key). Phase 1 (lead ruling 4b4cb4ff) leaves the
resolution unchanged and makes it loud:
- One helper, envDataDir(), is the only reader of the two variables (Load and
userConfigPath both use it). A relative value warns ONCE per process,
naming the absolute path this process resolved, that a CLI and a server in
different directories will use different data, and that a future release
refuses it (TASK-4072). A literal '~' gets a hint. A CLI prints it to
stderr; `pad server start` logs it through slog; `pad watch --stream
--for-session` (the plugin monitor) suppresses it.
- install-refresh anchors the refresh marker and config_value the way 2a
anchors the log (${SERVER_CWD:-$PWD}), so the marker is written and
cleaned up in one place (it used to leak under the caller's cwd), and warns
before anything is read or stopped.
- docs/deployment.md: both variables must be absolute; relative is
deprecated and warns.
Tests: the warning rule (and Windows, GOOS-guarded), the marker and the
warning-before-stop end to end, config_value at function level, and
child-process tests for the monitor's silence and the server's slog line.
Harness guard (after this unit's draft test stopped the box's shared :7777
server twice): every install-refresh exec in internal/buildtools now passes
requireSafeScriptEnv first, which refuses an environment whose resolved port
is missing (the script would fall back to 7777) or is 7777, whose HOME or
data dir is the real user's or outside a test temp dir, or whose port answers
on a LAN address without this test's random stub token.
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
* fix(config,mcp,scripts): the relative data-dir warning names the server's path and stays out of agent-facing errors; harden the test guard (BUG-4066 r1)
r1 rulings (c0ef54e8):
- install-refresh's warning now runs once SERVER_CWD is captured (still
before the marker and the stop) and names the anchored path
(${SERVER_CWD:-$PWD}/<dir>, a leading ./ stripped); when the caller's cwd
differs it also names the caller's reading. It used to name $PWD/<dir>,
which the server does not use.
- `pad mcp serve` checks the data dir once on its own stderr; the stdio
dispatcher's `pad` children get PAD_INTERNAL_NO_DATA_DIR_WARNING=1, so the
warning is not folded into a failed tool call's agent-facing error. The
variable is internal (code-documented only), so users cannot silence the
deprecation. Shell completion no longer prints the warning mid-Tab.
- Test guard: refuses a relative data dir with a config-sourced port (the
script reads that port from the server's cwd, which the guard cannot see);
compares symlink-resolved temp paths (macOS); asks for the stub token on
the LAN address that answered; compares 7777 numerically; and a source-scan
test fails if any install-refresh exec in the package is unguarded.
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
* fix(config,mcp,scripts): the relative-dir warning names the data dir; completion and MCP fetchers stay quiet; harden the test guard (BUG-4066 r2)
- PAD_DB_PATH warning names the resolved data dir (Dir of the value), in Go and in install-refresh.sh; the script normalises . and .. in pure shell.
- Shell completion (__complete, __completeNoDesc) sets the silent sink in main.
- MCP resource and bootstrap fetchers exec with dispatchChildEnv; a population scan pins every pad exec in internal/mcp.
- Harness guard: numeric 7777 refusal before any dial (asserted by message), PAD_HOST must be empty or loopback, temp-dir check scoped to this test, hardened and self-tested source scan, marker-presence probe in the restarted stub, child timeout, sink reset.
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
* fixup! fix(config,mcp,scripts): the relative-dir warning names the data dir; completion and MCP fetchers stay quiet; harden the test guard (BUG-4066 r2)
test(buildtools): the config-file 7777 row asserts the numeric refusal by message too (BUG-4066 r2 item 12, same class)
* test(buildtools,cmd): close the m15, m24 and m28 survivors (BUG-4066 r2)
m24: a scanner self-test case whose variable never runs in its function,
with a guard in a later function, so the function boundary is what flags it.
m28: insideTestTempDir refuses another test's directory, and admits a
subtest's, which t.TempDir names <Parent><sub...> with no separator.
m15: the --workspace completion func no longer sets its own sink. main()
silences every completion request, which made that line dead code.
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
* fix(cli,buildtools): an auto-started server does not inherit the warning silencer; the guard reads the config port like the script (BUG-4066 r3)
Item 2: an MCP tool-call child carries PAD_INTERNAL_NO_DATA_DIR_WARNING=1
and is often the process that auto-starts the server, so that long-lived
server never logged the relative data-dir warning. serverStartCommand drops
that one variable from the spawn env, and keeps every other one. It is the
only `server start` spawn in internal/cli; a source test pins both facts.
Item 7: the harness guard took the leading digits of config.toml's port, so
"8080abc" read as 8080 where install-refresh rejects it and falls back to
7777. It now reads the first port line the way config_value does, and a
non-numeric value is refused as unreadable.
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
* fix(scripts): install-refresh's relative-dir warning is one line; the completion tests run main() (BUG-4066 r4)
Item 1: when the server's cwd differs from the caller's, the warning printed
a second "Warning:" line naming the shell's-directory reading, and a third
for a leading ~. Phase 1's contract is one warning per process, as
config.Load prints it, so both now ride in the one line. The test asserts
both paths sit in that line and that it prints once.
Item 6: the completion child cases called silenceWarningsForCompletion
themselves, so deleting main()'s call passed every test. They now set
os.Args and run main().
Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV
|
||
|
xarmian
|
3e7f9e18cf |
fix(scripts): install-refresh restarts the server logging to cfg.LogFile() (BUG-4064) (#2304)
* fix(scripts): install-refresh restarts the server logging to cfg.LogFile() (BUG-4064) The restart appended to $HOME/.pad/server.log, a file nothing else names: `pad server info`, the daemonized start and every "Check <log>" message use cfg.LogFile(), <data dir>/logs/server.log. Anyone following server info read a log days stale. The old line also ignored PAD_DATA_DIR. The log path now comes from the script's data_dir(), which mirrors config.Load, and its directory is created before anything is stopped. The refresh names the log in its report and on a failed probe. An existing ~/.pad/server.log is left untouched and named in a one-line note. Tests: both restart branches, with and without PAD_DATA_DIR, find the restarted process's own stdout line in the configured file and nothing at the old path; the old file survives byte-for-byte. stubEnv now pins PAD_DATA_DIR and clears PAD_DB_PATH so the developer's environment cannot move the data dir out of the test HOME. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * fix(scripts): install-refresh proves the log writable before the stop; pid file in the data dir (BUG-4064 r1) - The log is opened for append (create or append) in a new section 2a, before the marker and the stop. An unwritable log used to stop the old server and then fail the restart's redirect, leaving nothing on the port. Now the refresh refuses with the path and the running server untouched. - The stale pid file is cleared at "$(data_dir)/pad.pid", where `pad server start` claims cfg.PIDFile() (cmd/pad/cmd_server.go, config.go), not at $HOME/.pad/pad.pid regardless of PAD_DATA_DIR. - The "mkdir ~/.pad before the redirect" block, which this change made untrue, is rewritten. A PAD_DATA_DIR refresh no longer creates a stray ~/.pad. Tests: a refusal on an unwritable log (a directory in its place) leaves the pre-server answering as itself; a stale pad.pid in PAD_DATA_DIR is gone after the refresh. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV * fix(scripts): install-refresh anchors a relative data dir to the server's cwd for the log and pid file (BUG-4064 r2) config.Load takes PAD_DATA_DIR verbatim, so the restarted server resolves a relative data dir against its own cwd. The writability proof and the pid-file removal now use that same anchored path (SERVER_CWD, else $PWD), so the file checked before the stop is the file opened after it. Also corrects the "root-owned" comment, and adds TestInstallRefresh_AnchorsARelativeDataDirToTheServersCwd. Mutation run (~/.cache/rook/4064-mut4.log, plus 4064-mut4-m8.log): baseline passes, and m1-m11 are all DETECTED. Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV |
||
|
xarmian
|
9e6791d4f1 |
fix(scripts): install-refresh asks git in the caller's checkout, not the server's cwd (BUG-3403) (#1790)
* fix(scripts): install-refresh asks git in the caller's checkout, not the server's cwd (BUG-3403) Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS * fix(scripts): anchor the web/ pathspec at the repo root (codex r1 on BUG-3403) Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS |
||
|
xarmian
|
bf0631d22f |
feat(ops): health reports which web bundle is served, and install-refresh warns when it is not a clean build of the commit (TASK-3233) (#1583)
The binary's commit stamp says nothing about web/build: it is generated,
gitignored and embedded as it stands. A bundle built from uncommitted edits
(a negative-control build, BUG-3129) or left over from an older commit
therefore ships under a clean commit stamp, and nothing afterwards could say
which bundle a server served.
- The server digests the web FS it embeds (sha256 over each file's path and
content hash) once in SetWebUI. /api/v1/health reports it as an additive
`web: {sha256, files, source_commit?, source_dirty?}`.
- `vite build` writes build-source.json (the commit, and the count of
`git status` entries under web/) into static/, which SvelteKit copies into
the bundle. It never fails a build: with no git, both fields are null
("unknown"), never clean. The stamp is gitignored.
- install-refresh.sh step 7 logs the served bundle and WARNS when the stamp
is missing, when it was built with web/ dirty, or when web/ changed
between the stamp's commit and the expected one. A backend-only commit on
top of a bundle is correct and is not flagged. It warns after the
restart; it does not prevent it.
Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
|
||
|
xarmian
|
d4ec587de9 |
fix(cli): a CLI call during an install-refresh bounce waits for the refresh instead of auto-starting a server over it (BUG-3215) (#1544)
install-refresh.sh stops the server, installs, and restarts it with the
captured argv. Any pad CLI call in between found nothing answering and
auto-started a server of its own: the caller's binary, cwd and default
flags. That server held the port against the refresh's restart (day 79:
127.0.0.1:7777 went dark and the script exited 1).
- The script writes <DataDir>/refresh.lock {pid, port, deadline} by rename
before the stop and removes it in its EXIT trap, only while it is still
its own. The deadline is 10s + PAD_PROBE_TIMEOUT (30s by default).
- EnsureServer defers to a live marker for its own port (writer pid alive
AND deadline not passed): it polls health and never spawns. If the
writer is still alive past the deadline it errors naming the pid. A dead
writer, a passed deadline, another port or an unparseable file means
the marker is ignored.
- The script names a holder that took the port before the restart spawns
(and does not spawn it), and refuses success when the listener after
the probe is not the restart it spawned. A health probe answers for
whoever holds the address.
- The script-test dependency guard now reads every install_refresh*_test.go.
Claude-Session: https://claude.ai/code/session_01A3rXV7GDLAhBe9mL4V6z2m
|
||
|
xarmian
|
b2bee3242f |
fix(scripts): install-refresh restores the server's own cwd and matches the running server by identity (BUG-3196) (#1512)
* fix(scripts): install-refresh restores the server's own cwd with its argv, refuses a deleted one, and matches the running server by identity rather than the built binary's name (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): one existence check refuses a deleted server cwd, naming the path without the kernel's suffix (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * test(scripts): the no-/proc refresh test declares lsof, which now reads the server's cwd there (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * test(scripts): the lsof dependency sits on the requireScriptDeps line the guard reads (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * docs(make): the install target's guarantees include the restored cwd (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): the server's cwd is re-checked immediately before the stop, and the capture's limits are stated (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): a failed restart cd says what state it leaves, not that no server runs (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): an unreadable server cwd is refused naming what was read and its error, and the case is tested (BUG-3196) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N |
||
|
xarmian
|
45dddb8f8f |
fix(scripts): install-refresh replaces the server listening on its port, by pid, and leaves other pad servers running (BUG-3194) (#1504)
* fix(scripts): install-refresh replaces the server listening on its port, by pid, and leaves other pad servers running (BUG-3194) The script counted every `pad server start` on the box and refused on more than one, so a sibling seat's e2e server (a worktree binary on its own port) made the scripted refresh unusable whenever anyone ran e2e. Its stop was a system-wide `pkill -x`, which would also have killed that server and any `pad` CLI call in flight. The target is now the `server start` process listening on the port the server itself would resolve without a flag (PAD_PORT, config, 7777), read with ss or lsof. It is stopped by pid, after its argv is re-read and matched. Still refused: the port held by something that is not a captured server, several servers on it, or several servers and none on it. One server with nothing on the port is still that server. Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * docs(make): the install caution names npm ci, the hazard that remains; the stop is no longer system-wide (BUG-3194) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): refuse a foreign or unreadable listener on the port, re-check before each signal, keep empty trailing arguments (BUG-3194, codex round 1) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * fix(scripts): the one-server fallback needs an explicit --port, so a lone sibling server is never replaced (BUG-3194, codex round 2) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N * docs(make): reflow the install comment (BUG-3194) Claude-Session: https://claude.ai/code/session_01RYiqbqXmxQMoKHsGm8B77N |
||
|
xarmian
|
110045578c |
fix(build): make install proves what it installed and what it restarted (BUG-2897, TASK-2787) (#1272)
`make install` made three claims it did not check.
1. It brought the server back BY SIDE EFFECT -- `pad auth whoami` triggers
an auto-start, which does not know the killed process's argv. A server
running `--host 0.0.0.0` came back bound to the default host alone:
curl 127.0.0.1:7777 -> 000 while the LAN address -> 200, with the
process count and the version both reading correct (BUG-2897).
2. `cp` copied whatever was at the repo path, not what the invocation
built. Two sessions sharing the checkout interleave and the loser's
build is installed by the winner, every exit code green (TASK-2787).
3. "Server restarted." was printed after a command ending in `|| true`,
with no probe of any kind. Not "the wrong address went unverified" --
nothing was verified. Found reading the recipe; neither filing names it,
and it is what made the other two invisible.
The logic moves to scripts/install-refresh.sh for one reason above
readability: a script can be TESTED. internal/buildtools drives it against
a compiled stub `pad`, including the branches where a check must FAIL.
Recipe-inline logic is only exercisable by running `make install`, which
stops the developer's server -- a test nobody runs twice, which is how this
target accumulated three unverified claims.
The script checks OUTCOMES rather than steps: what got installed, and what
is answering afterwards. Two commit checks, deliberately, answering
different questions -- the ARTIFACT before the kill, so a wrong build costs
an error instead of an outage, and the DESTINATION after the copy, which is
the shared-path race TASK-2787 names. The restart uses the argv read from
/proc before the kill, and nothing is printed about a restart until the
server answers on BOTH 127.0.0.1 and the configured host (`--host 0.0.0.0`
resolving to loopback plus the primary LAN address, since 0.0.0.0 is a bind
spec, not something to curl).
Three defects were found in the fix itself, by its own tests and by the
first real run:
- The restart redirected to $HOME/.pad/server.log with nothing creating
that directory. On a fresh HOME the redirect fails, the server never
starts, and the probe reports "did not answer" -- true, and three steps
downstream. Invisible to every hand-run this script could have had,
because a developer's box has ~/.pad by luck of history.
- The post-copy check SURVIVED its mutant: both fixtures reported the same
version from source and destination, so the guard and its absence were
indistinguishable. Fixed by making the stub's version depend on its path.
- Comparing commits by equality rejected a healthy build. `git rev-parse
--short` returns the shortest UNAMBIGUOUS prefix, so its width grows with
the object database: the binary embedded `a3a1d58` and the Makefile
produced `a3a1d586` minutes later. Now a prefix comparison in either
direction, with a negative leg pinning that a genuinely different commit
of the same width is still refused.
Five mutants, each verified to compile, each detected by its own leg.
Verified end to end on the real box: captured `--host 0.0.0.0`, installed
|