upstream(fujibee/agmsg)のmainを同期する - #90
Merged
Merged
Conversation
…ujibee#824) choco install sqlite -y --no-progress was one call with no retry, no cache and -- the part that did the damage -- no check. Measured on run 31869238895 (install helpers): steps 4, 5, 6 and 7 all success, step 8 failure. The install printed 'Chocolatey installed 0/0 packages' after a 503 from the community feed and EXITED ZERO; the version step's || echo turned the missing binary into a success line; the red landed three steps later on Run tests. Reading that check list, the failure belongs to the change under test. It did not. Worse than intermittent: the same outage decides a DIFFERENT step's colour depending on where chocolatey's two-phase resolution fails. Run 31870847862 on another branch got 499 on the IsLatestVersion query, was counted 0/1 with 1 failed, and exited non-zero -- so the install step went red there. Same cause, two different reds, neither of them reproducible in place. Three changes, one per problem: cache, keyed on a pinned version -- most runs never ask the feed retry, three attempts widening -- the failures are per-request, not an outage (both Windows legs run concurrently and on 31869238895 one installed fine while the other did not) a dedicated presence check that is the only step allowed to be red about this, named so the check list says chocolatey rather than tests Also drops the || echo fallback from Show tool versions. Absence is now decided by a step that may fail; that one only reports. The install step deliberately exits 0 after its retries: whether one choco invocation worked is a different question from whether the dependency is present, and only the second may decide the job's colour.
…not serve Stop condition 2 asks to see the CI behave correctly while the dependency cannot be fetched. The feed recovered before this change was written, so the condition is created rather than waited for: a nonexistent version busts the cache key AND makes the install fail, which is the same path a 503 takes. What is being observed is which step goes red. Expected: Install sqlite3 success (exits 0 after its retries) sqlite3 must be present, ... failure <- only this one Run tests skipped Today the same outage reddens either Install sqlite3 or Run tests depending on which phase of chocolatey's resolution fails. This commit exists to show that is now fixed, and it is reverted immediately after the observation.
…feed cannot serve" This reverts commit 41fd8ae.
…ee#822 The first probe ran before this branch was rebased onto fujibee#822, so both Windows legs it could observe required sqlite3 and there was no non-dependent leg to contrast against. Two reviewers pointed that out independently, and it is why stop condition 3 could not be measured on the previous head. Now driver input (fujibee#817) is present with sqlite: false. The same one-line failure should separate the two cases: install helpers (sqlite: true) sqlite3 must be present, ... failure <- the only red Run tests skipped driver input (fujibee#817) (sqlite: false) Restore / Install / PATH / presence all skipped Install bats success Run tests runs That second column is fujibee#822's contract -- a leg that never opens a store is not stopped by a package feed -- and the point is that hardening the install did not cancel it.
The previous probe's run started before GitHub's refs/pull/827/merge caught up with the rebase, so the driver-input leg executed the PRE-rebase Show tool versions step -- the one without the matrix.sqlite guard -- and went red on sqlite3: command not found. Measured rather than assumed: the merge ref now carries the guard (git show refs/pull/827/merge names it at the same line), and the head does too, but the job's own log printed the older text. A run cannot be re-pointed at a newer merge ref, so this creates a new one by changing the probe value. The observation being sought is unchanged.
Two defects from one misplaced edit, both found by reading the executed script in the job log rather than the file I thought I had changed. The version-step guard went into bats-shard, not bats-windows. I anchored the edit on the surrounding comment about the hang sampler, which belongs to the shard job; there are three Show tool versions steps in this file and the anchor matched the first. My own rule about not binding to the Nth match is what this violates. bats-windows left unguarded -> driver input (fujibee#817), which carries sqlite: false, went red on sqlite3: command not found. The exact coupling fujibee#822 removed, re-created one step lower. bats-shard silently broken -> that job has no matrix.sqlite at all, so the interpolation is empty, [ "" = "true" ] is false, and sqlite3 --version would never have run again on Linux or macOS. A diagnostic disabled with nothing to show for it. The shard step is restored to the destination's exact text -- diffed against origin/integration/remote, identical, not merely similar. The guard is now on bats-windows, verified by walking job headers rather than line numbers. I first attributed the red to a stale refs/pull/827/merge. That was wrong and measurable: the second probe carried a new pin value, the job log printed it, and the old step text was still there -- so the tree was current and the edit was in the wrong place.
install helpers (sqlite: true), pin pointed at a version the feed cannot serve Restore sqlite3 (cache) success miss, nothing restored Install sqlite3 success exits 0 after retries sqlite3 must be present, ... FAILURE the only red Install bats / Show tool versions skipped Run tests skipped never runs driver input (fujibee#817) (sqlite: false), same condition, same run Restore / Install / PATH / presence check skipped all four Install bats success Show tool versions success Run tests SUCCESS The second column is fujibee#822's contract and it survives: a leg that never opens a store is not stopped by a package feed. The first is this PR's: the red names chocolatey and Run tests does not execute, so it cannot be read as the change's failure. Both from run on b0b5a1a, one job each, same commit.
…t-not-look-like-a-test-failure ci(windows): a feed outage must not be reported as this change's red (fujibee#824)
* docs(key): stop telling every agent that key rotate is unavailable key.sh gained a rotate subcommand on 2026-07-22. Seven days later ten agent-readable surfaces began saying it was not available yet and refused unconditionally, so an agent asked to rotate a team key answered that the feature does not exist. cmd_rotate acquires the team lock, promotes the confirmed epoch, reads the identity journal and mints a replacement; tests/test_key.bats covers four of its paths. Route rotation on all ten surfaces instead, and keep the half of the sentence that was true: key request and key approve are not subcommands, so they exit 1 with usage. key.sh generate carried the same claim and sent a user holding an existing key to show; it now names rotate as well. The guard asserts both halves on all ten surfaces - a negative-only check passes on a file that says nothing - and binds them to the dispatch case and cmd_rotate, so removing rotation reports the docs it invalidates. * fix(templates): route rotate through the install name, not a literal agmsg A template is an installer input: install.sh renders it with sed s/__SKILL_NAME__/$CMD_NAME/g, and the generate, show, handoff and import routes beside this one already use the placeholder. The rotate route added here spelled the default name, so an install made with --cmd m would have sent the agent at a different install's key.sh - and rotation changes key state. The guard shared one substring across both surface kinds, which stayed green for either spelling and is what let this through. It now asserts the placeholder path on templates, rejects any literal skills/agmsg path there, and asserts the rendered literal on SKILL.md, which carries no placeholder.
fujibee#830) (fujibee#842) * fix(install): record provenance when git answers in another path space Refs fujibee#830. On Windows the describe branch was never taken, so every Git Bash install recorded the VERSION file instead of the `git describe` string: $SCRIPT_DIR /tmp/tmp.XXXX/agmsg MSYS form, from bash git --show-toplevel C:/Users/.../tmp.XXXX/agmsg native form, from git The equality guarding it compared one against the other, so it was always false there. SIXTH SITE OF THE SAME FAMILY. `agmsg_cmdline_names_path` in compat.sh already records five places that compared a shell path against a native one, four of them deciding whether to kill a stale watcher — which on Windows answered "not ours" and left it running. The fix here is the same second chance it takes: `cygpath -m`, the mixed form git reports. This file already uses that idiom for its writable paths. Off Windows there is no cygpath, `native` stays empty, and this is the plain comparison and nothing else. THE TEST TOOK TWO TRIES, AND BOTH FAILURES ARE WORTH THE LINES. First it asserted the recorded value merely looked like a version. It did not distinguish the two outcomes: the fallback writes `1.2.0-rc.6`, which passes any such check. Reverting the fix left it green. It now compares against the actual `git describe` output, and asserts up front that the describe string and the fallback string DIFFER — so the assertion is known to be able to tell them apart. Then it stubbed only half the platform. A `git` that answers `--show-toplevel` in native form models the disagreement, but Windows also ships `cygpath`, which is how the two forms are reconciled — and this host has none. The fix could not fire, and looked broken when what was incomplete was the model. Both are stubbed now. Measured: with both stubs in place, reverting the comparison to the single-space form turns the test red. * test(install): make the cygpath flag observable, and scope `native` Refs fujibee#830. Raised in review of fujibee#842. The cygpath stub answered `C:<last arg>` whatever it was handed. The flag is the whole claim — real cygpath picks the output path space from it, `-m` being the mixed form git reports while the default and `-u` are the Unix form the comparison already holds. So production could drop the flag or pass `-u`, reintroducing fujibee#830 exactly, and this test stayed green. It now refuses anything but `-m <source dir>`. Refusing is what makes the flag observable: a stub that answers everything blesses the broken calls too. Mutation matrix, all four rows measured: baseline (unmutated) GREEN comparison -> single-space form RED cygpath -m -> cygpath (no flag) RED cygpath -m -> cygpath -u RED Also declares `native` in `agmsg_source_version`'s `local`. It is a function-local value; nothing downstream reads a global of that name, so this is ownership made explicit rather than a fix. bats tests/test_install.bats: 54/54. * test(install): make the premise check able to fail (fujibee#670 ratchet) Refs fujibee#830. CI's `enforceable assertions` went red on this branch and green on main, so this came in with the new test. The offender was its own premise check: [[ "$output" == C:* ]] A non-last `[[ ]]` cannot fail the test on macOS bash 3.2. That line exists to stop an unnoticed pass — it asserts the git shim really does answer in the native form, so a green result cannot come from the shim being bypassed. It was a blind check guarding against blind checks, which is the entire subject of the test it sits in. Now `[ "${output#C:}" != "$output" ]`, which is enforced everywhere. Measured: breaking the shim so it stops prefixing `C:` turns the test red AT THAT LINE (934) — the final assertion alone would still have passed, so nothing but the premise check could have caught it. check-enforced-assertions: 638, at the baseline. The baseline is unchanged; this branch adds no exemption. * docs(install): state the capability, not the operating system Refs fujibee#830. Raised in review of fujibee#842. The comment said: Off Windows there is no cygpath, `native` stays empty, and this is the plain comparison and nothing else. That boundary is wider than the code. The condition is `command -v cygpath` — a CAPABILITY, not an operating system. This file's own test is the counterexample and it sits in the same change: it runs on macOS and Linux and drives the second branch by putting a cygpath stub on PATH. It also implied the old behaviour was a floor. It is not. Where cygpath is absent, fails, returns nothing, or returns a path unequal to git's toplevel, the fallback is recorded as before — but a wrong answer that happened to equal the toplevel would still take the describe branch. Those four conditions are what the code guarantees; "the worst case is the old behaviour" is not. THE RETRACTED SENTENCE WAS IN THREE PLACES. It was corrected in the PR body first, and survived here and in the message of commit 7238387 — which cannot be rewritten, so it stands as a record of the wider claim. This commit and the PR body now carry the narrower one; `grep -c 'Off Windows'` is 0 in both. Comment-only: the diff has no non-comment line. bats tests/test_install.bats 54/54.
…ing to do it (fujibee#821) The per-team critical section is now bounded in time, not only in scope. A local child that hangs used to hold the lock forever: the shell waited, the EXIT trap never ran, and the next start met `.config.lock: File exists` left by a process that would never finish. The wait has a ceiling (default 120s). Passing it is a failure with a name. Three things this took several rounds of review to get right: - Failing to BUILD the bound must not run without one. No FIFO still bounds by polling a sentinel; no temp file at all refuses and says why, with an explicit opt-out rather than a silent return to the old behaviour. - `read -t` returns 1 for both timeout and EOF on Bash 3.2, which is what the macOS runners execute, so the original `>128` ceiling was never taken there. A sentinel carries the child's status instead. - Releasing the lock is gated on ASKING whether the child is gone, not on having called `kill`. A pid is not an identity: liveness and the operation's own argv are both required, escalation is monotone, and a number that cannot be identified leaves the lock deliberately held with the path to clear it. Closes fujibee#821. Refs fujibee#817 — the Windows start-path mechanism is unchanged and that issue stays open.
…ingerprint is real, and doctor names a wedged lock (fujibee#868) * ci(windows): take the package feed off the Windows legs entirely (fujibee#824) fujibee#827 made a chocolatey outage legible -- retry, cache, and a presence check that is the only step allowed to be red about it. This removes the cause. The issue's own Directions ranked the retry last, "the cheapest change and the least durable", and that was right: on a cache miss the leg still needed community.chocolatey.org. What changed since is the weight. `bats` is a required check on `main` with enforce_admins true, so a red Windows leg now blocks every landing in the repository, and nobody can wave it through. sqlite3 now comes from one pinned sqlite.org URL, verified by hash: https://sqlite.org/2026/sqlite-tools-win-x64-3530400.zip sha256 f46ee2475de4cbe287e6e5f7d43c838796b14e7379cd216bdbb28d391429f9fc 6,557,792 bytes, contains sqlite3.exe All four measured by fetching it, not read off a page: the URL answered 200, the digest is of those bytes, and the archive was listed to confirm the binary is in it. What is gone is the FEED, not the third party -- sqlite.org is still somebody else's host. What it is not is a package index: no resolver, no two-phase lookup, and nothing that answers 503 for one concurrent job while serving another, which is the behaviour that made the original failure unreadable. The hash is load-bearing. Without it this would trade a feed that answers errors for a host that could answer anything, and the tests would run against whatever arrived. Kept from fujibee#827, deliberately: the presence check. Whatever supplies sqlite3, the step that decides the leg's colour must be the one that says the dependency is missing, not the one that runs the tests. Also corrects a comment fujibee#827 left behind, which still said a bump means "one trip to chocolatey, which is the only time the community feed is asked at all". There is no feed to ask. Not measured: whether the GitHub Windows image already ships sqlite3. It is absent from the Windows 2022 and 2025 image manifests, which is the published list rather than a `where sqlite3` on a live runner. * fix(remote): render embedded curl-config paths for the platform (fujibee#850) `_remote_http_post_json` writes a curl `-K` config so the request body — which holds the token — never reaches curl's argv. Two of the values it writes are paths: `dump-header` and `data = "@..."`. On Windows/Git Bash those paths are unopenable. MSYS translates POSIX paths to Windows form for a native binary's ARGV, and not for the contents of a file that binary reads. So `/tmp/...` written into the config stays `/tmp/...`, and native curl has no such path. curl fails, and the caller reports the "000" it reports for every failure, with no indication that a path was the problem. `cygpath -m`, not `-w`: -m yields a Windows drive path with FORWARD slashes, and curl's config parser treats a backslash as an escape — so the -w form is re-mangled by curl itself. Measured on the machine, not reasoned about. Gated on `command -v cygpath`, not on an OS name. Where cygpath does not exist the helper returns its argument unchanged, so macOS and Linux take exactly the path they took before — verified directly rather than inferred from a green suite: the helper is a pass-through for /tmp, $TMPDIR and a /private/tmp path on this machine. * fix(remote): dump headers to a file where a real fifo cannot exist (fujibee#850) The headers go through a fifo so a broken or hostile server cannot make us buffer an unbounded response: bounded-copy.py enforces the ceiling while the transfer is still running. That mechanism needs a real named pipe. On Windows/Git Bash there is no real named pipe to have. MSYS emulates mkfifo with a .lnk file that only MSYS-aware programs understand, and curl there is a NATIVE binary — it cannot open what mkfifo made. curl fails, and the caller sees the "000" it reports for every failure alike. Where cygpath exists, dump straight to the destination file and skip both the fifo and the copier. Gated on `command -v cygpath` rather than an OS name: what decides is whether a real fifo can be made, and that is what the probe asks. WHAT THIS GIVES UP, on that platform only: streaming enforcement of the header size ceiling. curl's own `max-filesize` still bounds the body; the header dump is what becomes unbounded. Written here rather than left implicit, because it is a real difference between the platforms. Also fixes a deletion the branch form makes dangerous. The cleanup ran `rm -f "$cfg" "$header_fifo"` unconditionally — and on the cygpath path `header_fifo` IS `header_file`, so it deleted the headers this function was asked to produce, before the caller read them. The fifo only exists when a copier was started, so that is what the removal is keyed on now. * fix(hash): SHA-256 without shasum, and no fingerprint it could not compute (fujibee#861) connect --e2ee registered the team with the server and then died on the way to the sync engine. The cause is one absent command: shasum is a Perl script that ships on macOS and most Linux and is NOT in Git for Windows' Git Bash. This repository already knew. scripts/lib/hash.sh exists for exactly this reason and its comment describes today's output -- "NOT in Git for Windows Git Bash, where it fails with shasum: command not found, leaving the hash empty". It was written for SHA-1 only, and the four SHA-256 sites never got the same treatment. Derived rather than taken on report: git grep shasum -- scripts/ gives six invocations across five files. Four are SHA-256 in production -- key.sh's two fingerprint helpers, key.sh's previous-snapshot digest, and remote.sh's age-v1 checkpoint -- and all four now go through the new helper. The fifth is hash.sh's own SHA-1 arm, unchanged. The sixth is release/update-cask.sh, which downloads a .dmg and pushes a Homebrew tap; it cannot run on the platform this is about, so it is deliberately left alone. THE LAST RESORT IS DIFFERENT ON PURPOSE. agmsg_sha1 falls through to cksum because its callers name a socket after the digest and need only stability on one machine. These callers need a real SHA-256: one value is the fingerprint two people read to each other over a separate channel, the other decides whether an epoch snapshot is the one it claims to be. A non-cryptographic stand-in there does not weaken the check, it makes it say something untrue. So agmsg_sha256 ends by FAILING, and the comment says why, because the obvious next edit is to make the two consistent. A FINGERPRINT THAT COULD NOT BE COMPUTED IS WORSE THAN NONE. The five print sites read `echo "Recipient fingerprint: $(_key_fingerprint ...)"`. A command substitution that fails inside a simple command's arguments leaves that command's own status alone, so echo succeeded and printed the label with nothing after it -- and the whole point of the string is that two people compare it. Both would have seen the same blank and agreed. Each site now takes the value into a variable of its own first, where a bare assignment's status IS the substitution's and set -e stops. Not `local x="$(...)"`, which puts the status back on the declaration and undoes it. Preflighted at connect, not only in doctor. Every SHA-256 in the e2ee path happens at or after key generation, which is after registration, so the check is asked before either. Reported by doctor as optional, next to age and for the same reason: a team on cipher "none" never computes one. The preflight probes by RUNNING the digest rather than asking command -v. A tool that is installed and broken answers yes to a presence check and no to the question, which is the direction that hurts -- the preflight passes and the digest fails later, which is the shape of this bug again. Left alone deliberately: remote.sh's fail-closed structure. An empty digest made the checkpoint invalid, setup refused, and connect reported "binding recorded, sync engine not started" without falling back to plaintext. That behaviour was correct; it is the digest that was wrong. UNLIKE fujibee#850, THIS IS VERIFIED HERE. The behaviour is chosen by command -v, so a PATH holding only the tools we pick reproduces each platform's choice on any platform -- no Windows machine required. Each arm is asserted against a literal digest rather than against the other arms, since three arms can agree on a wrong answer. The suite includes its own negative control (that a restricted PATH really does hide what it claims to) and covers the case that motivated the run-it probe: tools present and failing. * test(hash): the no-tool sandbox was hiding the fallback it exists to forbid The mutation that matters most for this change is "add a cksum arm, to make agmsg_sha256 consistent with agmsg_sha1" -- the exact edit the helper's comment warns the next reader against. Applied, the suite stayed green on the one test named for it. Not because the refusal survived. The sandbox PATH held only `awk`, so the mutant's `cksum` was not found either, the pipeline failed anyway, and the assertion passed for a reason that had nothing to do with the decision under test. It was measuring the box, not the behaviour. cksum is present on every machine this runs on, so that is the shape the test has to run in. With it in the box, the same mutation now turns four cases red, including the one whose name is the claim. Same correction applied to the other sandboxes built for "no SHA-256 tool": the message case, the usable probe, and the require preflight. * test: make the new assertions able to fail (fujibee#670 ratchet) CI's enforceable-assertions check counted 642 against a baseline of 638. The four are mine, and they are the shape that check exists for: a non-last `[[ ]]` cannot fail a test on bash 3.2, so it reads as an assertion and behaves as a comment. Three were in the "says which tools were looked for" case, where only the third of three could ever have fired -- the two that name shasum and sha256sum were decoration. The others were the first half of the doctor case and the message half of the connect preflight case, each followed by another assertion. All four are now plain `grep -qF`, which fails the test under bats' errexit. The count is back at the baseline and the suites are green; the mutation rows that rest on these assertions are being re-run, since rewriting an assertion changes the instrument and the earlier matrix was measured with the old one. * docs(key): the no-blank-fingerprint refusal rides on pipefail, so say which line agmsg_sha256 sits in the middle of the fingerprint pipeline and cut and sed are content with the empty input a failed digest leaves them. Without pipefail the pipeline exits 0 with an empty string and the label prints with nothing after it - the exact output the callers were changed to prevent. Measured: replacing set -euo pipefail with set -eu reddens both no-blank -fingerprint cases and nothing else. The comment names that control so the next person shortening line 2 has somewhere to look. * fix(hash): the helper's refusal was the caller's pipefail, and its answer was unchecked Two findings from static review, both one level below where the same shape was already noted. The arms were shasum|awk, so the function's status was awk's, and awk is content with the empty input a failed digest hands it. 'this FAILS' held only because key.sh and remote.sh set pipefail on line 2 - a caller without it got an empty success and carried it into a fingerprint. Each tool now runs as its own substitution with its status checked. And nothing looked at the answer. A tool that exits 0 while printing a warning is not a tool that failed, so 64 lowercase hex is now required, and the usable probe compares against the known digest of its input rather than asking whether anything came back - sixty-four characters that are the wrong sixty-four are exactly what a fingerprint cannot survive. Five cases, and one of them was rewritten after it failed to discriminate: the first no-pipefail case used an empty PATH, where the helper returns 1 outright and pipefail was never involved. A present- but-broken tool is the only path that isolates it. The case pinning the status check separately from the shape check exists because reverting the arms alone changed no result until it was written. * fix(hash): put the known-answer check in the helper, not in a list of entry points Review: the correctness check lived in agmsg_sha256_usable, whose only production callers are remote.sh's doctor and connect preflight. key.sh is its own CLI - generate, show, import and rotate all reach a digest without going near connect - so a tool exiting 0 with a well-formed but wrong digest was accepted by every one of them, and the suite pinned that as expected behaviour. agmsg_sha256 now asks the selected tool a question whose answer is known, once per process, before it will return anything. Keyed on the helper rather than on the callers because the alternative is a list of entry points kept complete by hand, which is what was missed. A key.sh case drives it directly with a lying shim, because a helper -level case cannot fail if a caller stops calling the helper. Also from review: require and doctor said 'none was found' and 'tool on PATH' for a tool that is present and broken. Presence and usability are different questions and these lines answer the second. * fix(hash): a memo that could be preseeded, and that never memoised anything Review, twice over on the same three lines. The self-test's flag was read from the environment, so _AGMSG_SHA256_VERIFIED=1 in an inherited environment meant 'already checked' and skipped it - an undocumented env override of a fail-closed contract, on the hole closed one commit ago. And it saved nothing. Every production call is printf | agmsg_sha256 or x="$(agmsg_sha256 …)", both subshells, so the flag never reached the parent and the self-test ran again on the next digest anyway. Removed rather than made private: a memo whose saving is imaginary is only a surface. The cost is now stated where the code is - one extra digest of a 5-byte input per digest taken, at most three per command. A case pins that no inherited variable can skip it, trying several names, because the property is about the environment and not about one spelling. * docs(hash): the comment kept the contract the code stopped having Three claims left over from the memo, all found in review, none needing code. The block heading still said ONCE PER PROCESS with RUN BEFORE EVERY DIGEST six lines under it - one comment stating two contracts, and a reader has no way to know which one the code has. agmsg_sha256_usable said it costs one subprocess. It calls agmsg_sha256, which self-tests and then digests, so it runs the tool twice. And the worst path was undercounted: key rotate with an accepted rotation to verify takes four digests, not three - the accepted recipient's fingerprint before the three already listed. * test(remote): bind the curl-config path rendering, both fields separately (fujibee#850) The stop condition was that reverting the production change leaves CI green. It does now: reverting the renderer turns two tests red, and reverting either single field turns exactly its own test red. The test drives the real _remote_http_post_json, sourced from remote.sh, with two stubs on PATH. What makes the stub worth anything is that it does not just record the config -- it OPENS WHAT THE CONFIG NAMES: cygpath -m yields forward slashes, -w yields backslashes. It must really produce both, or the test cannot tell the two apart, and telling them apart is the whole point of the fix naming -m. curl resolves each embedded path the way a native Windows binary resolves a Windows path back to the same file, and EXITS NON-ZERO when it cannot. A stub that accepts any string as a path would pass on a rendering no curl could open -- which is the defect itself. Four cases: no cygpath both fields byte-for-byte, so macOS and Linux are demonstrably unchanged rather than assumed unchanged cygpath, data rendered mixed cygpath, dump-header rendered mixed either way no backslash anywhere in the config The two field cases were one test until the matrix showed why they cannot be. Reverting the header field and reverting the data field both reddened the same combined assertion, which says 'something is untranslated' and points at neither. They are two effects of one line and regress apart: translate the body and not the header and curl opens the body, fails on the header, and the caller reports 000 -- which reads as a header problem. A fifth case is the control on the stub: replace _remote_curl_path after sourcing so it emits a path nothing can open, and the helper must return 000 -- the code the user actually saw. Without it, every assertion above could be passing on a string that no curl would accept. M0 no mutation 0 red M1 renderer back to passthrough 2 -- DATA and DUMP-HEADER M2 cygpath -m becomes -w 3 -- both fields and the slash assertion M3 header field left untranslated 1 -- DUMP-HEADER M4 data field left untranslated 1 -- DATA Production is untouched. Windows: still unverified here -- the cygpath branch is driven by a stub, and a stub cannot tell you what MSYS does. * test(remote): make the stubs disagree the way the two platforms do (fujibee#850) Review found two holes, both in my instrument rather than in the fix. P1-a THE NEGATIVE ARM WAS NOT NEGATIVE. 'without cygpath' prepended a directory holding only a curl symlink and left the host PATH behind it, so production's 'command -v cygpath' would still have found a real one. It passed here because macOS has no cygpath -- the test inherited an absence instead of creating it, and on the one machine this fix is for it would have run the wrong arm. Now an allowlist sandbox, the shape test_helper already uses for path_without_python3. A subtraction cannot express this: on Git Bash cygpath lives in the same directory as mktemp. The first test asserts the sandbox produces BOTH answers, so the capability every other arm rests on is measured rather than assumed. P1-b THE STUB ACCEPTED THE BROKEN PATH. Its resolve() took any absolute POSIX path whatever the platform, so it could not fail on the untranslated /tmp/... that broke the user -- while the file header claimed it opened what curl would open and the commit message claimed it failed where curl would fail. M1 went red on string assertions alone. The claim and the stub now say the same thing. The stub is told which consumer it plays. native-windows opens only Windows paths; posix opens only POSIX ones. Neither accepts both, which is the actual shape of the defect: the same bytes are openable or not depending on who reads them. Both directions have a case, so 'refuses a POSIX path' cannot be read as 'refuses whatever the test needs'. The outcome assertion moved out of the two field tests, and this was forced by the matrix rather than chosen. With the stub now failing on either untranslated field, asserting 200 inside both field tests made both go red for either mutation -- destroying the per-field attribution that splitting them was for. The 200 has its own test, whose job is to fail for any of them. M0 no mutation 0 red M1 renderer back to passthrough 3 -- DATA, DUMP-HEADER, completes M2 cygpath -m becomes -w 4 -- those three and the slash assertion M3 header field untranslated 2 -- DUMP-HEADER and completes M4 data field untranslated 2 -- DATA and completes 7 ok / 0 not ok. Production untouched; the diff is this one test file. Windows: still unverified by me. The cygpath arm is driven by stubs, and a stub cannot tell you what MSYS does. What this closes is 'revert production and CI stays green', not 'it works on Windows'. * test(remote): bind the header sink, and narrow the comment about the probe (fujibee#850) Two changes. The tests are the stop condition; the one production line is the ARTIFACT finding from the same review, and it is a comment. TESTS. tests/test_remote_header_sink.bats, 6 ok / 0 not ok, driving the real _remote_http_post_json under an allowlist sandbox PATH. Absences are the whole subject here -- no fifo, no copier -- and an absence is only observable if the instrument can record a presence. mkfifo and python3 are replaced by wrappers that log and then exec the real thing, so nothing about the run changes, and the first test proves the wrappers record at all. A log that was never wired up looks exactly like a call that never happened. no fifo is made marker present no bounded copier is started marker present the caller's header file survives marker present fifo AND copier still used marker absent -- the platforms that are supposed to be unchanged, asserted the fifo is removed, the header file is not marker absent The third is the trap this fix had to avoid: on the marker path header_fifo IS header_file, so a cleanup that removes the fifo deletes the headers the caller asked for, after a request that succeeded. M0 no mutation 0 red M1 marker ignored, always fifo 2 -- no fifo, no copier M2 cleanup removes header_fifo 1 -- header file survives M3 marker branch builds a fifo 1 -- no fifo M4 marker branch starts a copier 1 -- no copier M3 and M4 are why the two absences are two tests. They are separate machinery and are left behind separately: a fifo nobody drains, or a reader blocked on a file that never fills. One test asserting both reddens for either and names neither. COMMENT. The gate said "what decides is whether a real fifo can be made, and that is what the probe asks". It does not ask that. It runs a command -v on cygpath, which is a capability MARKER for an environment where MSYS fifos and a native curl coexist -- nothing here tests whether a real fifo can be made. As written it would have told the next reader that a machine passing the probe had been checked for the property that matters. Behaviour is unchanged: the diff outside tests/ is comment lines. This branch does not carry fujibee#851, so paths stay POSIX here and the curl stub is a POSIX consumer. On a real Windows machine this fix alone is not enough -- both are needed, and they compose only on the combined branch. Windows remains unverified by me. * feat(doctor): name the registry locks, and which directory to remove (fujibee#865) A lock left behind by a killed process is never broken by anything: nothing expires, nothing sweeps, and acquire waits out its budget and reports 'timed out acquiring registry lock', which describes contention. The operator goes looking for the process holding it; there is none, and no message anywhere names the directory to remove. A machine in that state cannot get itself out. doctor now lists each team's lock with what it records, whether that process is running, and the removal as a line to paste. Three answers, not two. 'held' and 'the holder is gone' are what an operator acts on; 'no holder recorded' is neither - a lock this cannot ask about, written either by a library version that recorded nothing or by a process killed between creating the lock and writing its record. Calling that gone would be a guess, and the guess that costs is the one that says a live lock is dead. It removes nothing, and it does not touch the exit code. A team being locked is not a failed prerequisite - reporting it as one would fail doctor every time somebody is joining - and sweeping automatically is a separate decision with a worse failure mode. Reported for the named team when doctor is given one, matching what its header already says. * fix(doctor): an unusable pid was never asked about, so it is not stale (fujibee#865 review) Three findings, all from static review. _agmsg_pid_alive_local returns false for a value it never put to the process table - non-numeric, leading zero, past the POSIX ceiling - and folding that into 'not running' turned 'pid not-a-pid' into a stale verdict with a rm -r beside it. The number is validated with the same ceiling the helper uses before it is asked about, and an unusable one joins 'no holder recorded' under cannot-tell. The sweep used *, which does not match a leading dot, and team names may begin with one - the validator rejects empty, . , .. , a leading -, / , \ and control characters, and nothing else. A named team is now looked at directly, and the sweep adds the two dot patterns. All checks passed. printed under a stale lock and its removal command reads as withdrawing them. Narrowed to All prerequisite checks passed.; the exit code deliberately does not move, so the wording is what carries it. Five existing assertions updated with it. Reporting one lock is its own function now. Both callers share it, so neither has to join paths into a string and read them back - which would mean an unquoted heredoc, running command substitution on a team name, or unquoted word splitting, globbing one. The first attempt at the sweep did build such a string and fed a while-read loop that had no redirection at all, so it read stdin and reported nothing. * fix(doctor): validate the named team before it becomes a path (fujibee#865 review) cmd_doctor takes an optional team and, until the direct lookup landed, only ever put it in a header sentence - so nothing had ever validated it. Building TEAMS_DIR/$team/.config.lock from it turned 'doctor ../outside' into a read of a directory outside the store, reported with its records and a rm -r to paste. The sweep it replaced never reached one only because no team was named that. Validated with agmsg_validate_team_name, the same one every other team-taking path uses, and a rejection ends the command rather than falling through to a prerequisites verdict for a question nobody answered. Three cases: a traversal argument reads nothing (with a sentinel .config .lock outside the store to prove the refusal is about reach), a slash is refused, and an ordinary and a dot-leading name still resolve - a refusal that took the legitimate names with it would be the cheapest way to pass the first two. * test(doctor): follow the summary line the lock report narrowed fujibee#862 asserted on "All checks passed"; fujibee#865's doctor narrowed the summary to "All prerequisite checks passed." so it cannot be read as cancelling a stale lock and the removal command printed above it. The two landed on different lines, so the merge was clean and the assertion was left describing a sentence the code no longer emits. * test(remote): assert the status line, not the stream a killed copier writes into (fujibee#850) * test(remote): name the issue the loosened assertion is recording (fujibee#869)
* release: 1.2.1 * release: map the fujibee#841 squash subject so the changelog carries it * release: keep the changelog's scrubbed history, add only the new section
) * docs(contributing): say that main takes squash commits only Three PRs landed on main as merge commits because nothing said otherwise and nothing stopped it: the repository allowed merge, rebase, and squash, and the only mention of squash anywhere was one command line in RELEASING.md for the release PR. Merge and rebase merges are now disabled on the repository, so the rule is enforced rather than remembered. This records it where a contributor reading before their first PR will see it, in both languages. * docs(contributing): do not hard-wrap the added item The surrounding numbered items are one line each regardless of length; the new item was wrapped at a column width the file does not use.
…#857) * fix(watch): fold sentinel '-' into the empty session-id path (fujibee#855) Callers still pass "${GROK_SESSION_ID:--}" from fujibee#477, but watch.sh stopped folding the literal dash, so monitor re-eval wrote watch.-.pid. Restore the one-line fold and the bats case that locked it. Co-authored-by: Cursor <cursoragent@cursor.com> * test(watch): cite the original fujibee#477 contract on the sentinel fold case The restored bats locks the fujibee#477 fold, not the later regression ticket. Co-authored-by: Cursor <cursoragent@cursor.com> * test(watch): use refute for sentinel absence checks (fujibee#855) Non-last `! cmd` is silent on bash 3.2, so the new fold case raised the enforced-assertions baseline. refute fails the test on every platform. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(resolve): map grok-build to grok in the pid walk (fujibee#859) Without this arm, grok-build falls through to claude/codex/gemini, so agmsg_agent_pid never sees a live grok and watchers are not liveness-gated. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(resolve): note why pid names omit the shared basename agent (fujibee#856) Homebrew grok-build and the Cursor CLI installer both ship `agent`. Listing it on either type would attach the wrong pid (#93). Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
fujibee#771) `ps -o comm=` prints the full executable path on macOS. Piping it through `xargs basename` word-splits that path, so `basename` receives the part before the first space as NAME and the rest as SUFFIX. The Claude Code desktop app always installs its CLI under `~/Library/Application Support/Claude/claude-code/<ver>/...`, so `compat_get_comm` returned `Application` there. `agmsg_pid_is_agent` then never matched, `agmsg_agent_pid` walked past the agent up to launchd and failed, the instance id degraded to a bare session id, `cc-instance.<pid>` was never written, and every actas lock was GC'd as stale on the next `session-start.sh` — silently, while the watcher stayed alive and kept delivering. Take the basename of the whole string instead. Paths without spaces behave as before; paths that `xargs` would have choked on (quotes) now work too, and an unknown pid returns non-zero rather than an empty string. Scope: the POSIX branch only. The msys branch has the same `xargs basename` shape at compat.sh:96, which `C:\Program Files\...` would hit, but that is untested here. Co-authored-by: Naohide Takahashi <n.takahashi@austa.co.jp> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…as TPL_TYPE (fujibee#846) (fujibee#880) Before: --update re-detected an existing install's type by grepping its shared SKILL.md for only antigravity/gemini/grok-build, a narrower and separately-hardcoded copy of the list TPL_TYPE selection uses. opencode, hermes, and cursor installs fell through to the codex default at re-detection, then had their SKILL.md overwritten with the codex template on the very next --update -- the installer clobbering what it had itself written. Fix: a single AGMSG_SHARED_SKILL_TPL_TYPES list, read by all three call sites (fresh install's TPL_TYPE case, --update's TPL_TYPE case, and --update's re-detection loop) that previously carried three independent, already-drifted copies of the same set.
Ref #89. Syncs 18 upstream commits (release/v1.2.1 and follow-ups) since this fork's main diverged at 3c537a5. 38 files, +4119/-71. Two conflicts, both resolved by keeping this fork's side unchanged (scripts/watch.sh, tests/test_watch.bats): this fork's watch.sh already carries a more complete fix for the same problem class upstream's fujibee#857 addresses (a shifted/dropped session-id argument). Upstream moved to ${2:?Missing project_path} (a hard bash failure that writes to stderr); this fork deliberately validates with a stdout-visible error instead, because the Monitor tool that consumes this script's output does not see stderr -- that rationale is already documented inline just below the argument parsing. This fork's test_watch.bats also already has the broader shifted-argument-guard and herdr-placement-aware despawn test coverage that upstream's version of the same test file does not have at this commit. Nothing upstream added here was lost; upstream's narrower fix is superseded by this fork's existing one. Everything else came in as upstream wrote it.
Ref #89. upstream commit fa826b7 (test_compat_posix.bats, merged in the prior commit) added a non-last `[[ ]]` assertion, the one pattern this repo's check-enforced-assertions.sh baseline exists to catch: on macOS bash 3.2, a non-last `[[ ]]`/`(( ))`/`! cmd` cannot fail the test. Converted to `grep -q ... <<< "\$raw"`, matching the existing house idiom used throughout this fork. check-enforced-assertions.sh: back to 718, matching baseline (was 719 after the merge alone).
This was referenced Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #89.
概要
upstream（
fujibee/agmsg）のmainを18コミット分（release/v1.2.1を含む）取り込みます。分岐点3c537a5以降、38ファイル・+4119/-71行の差分です。マージコミットとして取り込み、upstream側のauthorship/履歴を保持しています（squashしていません）。コンフリクトとその解決
scripts/watch.sh・tests/test_watch.batsの2ファイルでコンフリクトが発生し、いずれもこのforkの既存版（HEAD側）をそのまま維持する形で解決しました。このforkのwatch.shは、upstream fujibee#857（
fold sentinel '-' into the empty session-id path）と同じ問題クラス（session-id引数の欠落・シフト）に対して、既により完成度の高い修正を持っています。upstreamは${2:?Missing project_path}という素のbash機能（stderrにのみ出力される強制失敗）へ寄せましたが、このforkは意図的にstdoutへ出力する検証に倒しています（Monitorツールがこのスクリプトの出力を消費するがstderrは見ないため — その理由はコード内に既にコメントとして残っています）。test_watch.batsについても、このforkは対応する「引数シフトの検知」「herdr placementを考慮したdespawn」まわりのテストを既により広く持っており、upstream側で失われるものはありません。追加修正: check-enforced-assertions
マージ単体では
check-enforced-assertions.shが718→719になりました（upstream commitfa826b7が追加した新規ファイルtests/test_compat_posix.batsの1箇所が、非最後の[[ ]]という、このリポジトリのbaselineが検出対象とするパターンに該当していたため）。grep -q ... <<< "$raw"へ変換し、718（baseline一致）へ戻しました。詳細は当該commitに記載しています。テスト
bats tests/*.batsをフル実行: 1801件全pass（exit code 0）。check-enforced-assertions.sh: 718、baseline一致。bash -n scripts/watch.sh: 構文OK。git merge-treeによる事前確認では実質conflict無し（実際のmergeでは上記2ファイルのみ本物のconflictがあり、手動解決）。レビュー状況
直近の他PRと同じ、ユーザー承認済みのsolo-Claude運用（
docs/decisions/2026-08-17T060000_codex不在期間のClaude単独運用とmerge方針.md）です。upstream由来の広範な差分を取り込むPRのため、通常より念入りにローカルの全件テストで確認しました。補足: Issue #88との関連
upstream commit
f7de22a fix(roster): bound the wait on the local child without spawning anything to do it (#821)は、「ローカル子プロセスのハングでロックが永久に保持される」「macOSのbash 3.2ではread -tのタイムアウト判定が機能しない」という趣旨の修正で、現在追跡中のIssue #88（macos runnerでのsync startテスト群の断続的ハング）と症状が酷似しています。本PRのマージ後、PR #87のCI再試行でIssue #88が解消するかを確認する予定です（Issue #88の根本調査自体はユーザー決定によりCodex復帰待ちのままです）。