Many hyperlinks are disabled.
Use anonymous login
to enable hyperlinks.
| Comment: | A run whose program exits without reading its run gets its ended, and the watcher answers nil only for a lock another holds
Intent: protocol v1's promise that a run that started gets an ended, exited with its code, budget, stopped or lost (docs/protocol-v1.md, sections 4 to 7 and 10), never silence. An adversarial review of 9118789d1a (scratchpad review-headroom.md, finding 1) saw the gate fail once on a clean tree: spec/ocx/program_spec.cr:341, an unconfined echo run, hit watch(...).not_nil! "Nil assertion failed", and an empty /tmp/*ocx-run directory was left, which only Watcher.prepared makes. Watcher.watch ended in a rescue of IO::Error around everything it did, meant for the two lock takings, so an IO::Error after the run started was answered nil: no ended after the started, and the scratch directory left. The review forced the same path: an unconfined run given a 200 KB objective, whose program exits without reading it. watch returned nil, the directory was left, and no ended followed the started; with a 10-byte objective it returned 3 reports. The cause: the run's first line went to Process.new as an IO::Memory, which Crystal copies into a pipe in a fiber of its own, and Process#wait raises what that copy met, here EPIPE, before it gives the status. A session each of whose deliveries starts a process was given its lines the same way and was lost the same way. A session's relay and a terminal write their own pipe or pseudo-terminal in a fiber that rescues IO::Error: the same program and objective, as a session and at a terminal, ended exited 3 before this change. Which IO::Error failed the gate that once is not known: a short line given to a program that exits at once did not raise in 500 tries on a quiet host, with Process.new or with this change (scratchpad wl/race-probe-short.txt). With this change, whether the program read its line before it ended changes nothing. Expression: src/ocx/sandbox.cr: Sandbox.start, the one call that starts a run's process under precept I7 (Single Process Site), now takes its input by type. An IO::FileDescriptor the caller holds, a terminal's or a relay's, is passed on as before. A String, the text given, is written through a pipe by a fiber of its own, which then closes it and rescues IO::Error: a process that ends without reading all it was given is no error, and its status says how it ended. This process's copy of the read end is closed once the process has started, as Process.new closes its own pipes' ends, so the fiber's write fails at once when the process ends. Nothing gives Process.new an IO::Memory as input any more, so Process#wait never raises a copy's error. src/ocx/launch.cr: a task's empty input is "". src/ocx/watcher.cr: launched passes relay.try(&.input) || input.to_s, a descriptor or a String, which the compiler sends to the overload for each; its input stays an IO::Memory, whose lines a terminal types. watch no longer rescues IO::Error: it returns nil only when Node.take_run or Node.take answers that another holds the lock, as before. Any other failure raises, and the node's check answers lost a started run left with no ended (docs/protocol-v1-fossil.md, section 5; failure case 2). Before, such an error was answered nil, which wrote no ended either. prepared removes its scratch directory, the files made for the run's sandbox, in an ensure, whatever ends it: a refusal, a decision not to start, the run's end, or an error such as a write that fails. That replaces its three removals, and it now runs after the ended is written. The comments on watch and watched say that each lock is let go as its File.open block closes its file, an error too. src/ocx/care.cr: Node.take_run and Node.take answer whether they took the lock, false while another holds it, and took? rescues only the non-blocking try of flock, so no other error is read as a lock another holds. docs/protocol-v1-failures.md gains case 30, the program never reads its run. docs/crystal.md says that Process#wait raises what the copy of an input met. src/ocx/watcher.cr is 400 lines, within precept C1 (File Boundedness). Review: Gemma (gemma4:31b) read the change seven times. Held: none. Misread, each a Clarity defect, changed until a later reading read it right: first, that closing the read end right after the start lets the child see an end or an error before it reads. The child holds its own copy from its fork on; the comment now says so, and that Process.new closes its own ends the same way, and the second reading did not raise it. Second, that an error escaping watch leaves the run with no ended for ever. watch's comment said nothing of other failures. It now says the node's check answers lost a started run left with no ended, and the third to fifth readings did not raise it. Reworded as "left to the node's check", the sixth raised it again; reworded to say what the check does, the seventh did not. Third and fourth, that the run's lock, then the user's, stays held when decided raises. Each is opened by File.open with a block, which closes the file and so lets the lock go. The comments on watch and watched now say so, and the next reading read each right. Sixth, that the scratch directory leaks when watched fails before prepared. Only prepared makes it, and its ensure covers it from there; the ensure's comment now names the scratch made above, and the seventh reading did not raise it. Wrong: the first reading's finding that the pipe's writer leaks if the fiber never reaches its ensure. The fiber's ensure closes it whenever the fiber runs. The fifth reading's runtime TypeError for a relay whose input is no descriptor: Relay#input is declared IO::FileDescriptor, and Crystal picks an overload when it compiles. An input of any other type does not compile, as this change's first build showed: "expected argument #2 to 'OCX::Sandbox.start' to be IO::FileDescriptor or IO::Memory, not IO". The sixth reading's claim that watch runs inside Node.locked in the node's check loop: the hook's watch mode calls it in a process of its own, holding no lock of the node's. The seventh reading's NameError when the scratch is not yet assigned: scratch is assigned as prepared starts, before any begin. Crystal types a body's local as nilable in its ensure, which `if scratch` handles, and an unknown name does not compile. Demonstrates: spec/ocx/watcher_spec.cr, "ends a run whose program exits without reading its run, alone, as a session, at a terminal, or each of whose deliveries starts a process, as it ended (case 30), leaving no scratch directory, whatever ends the run". The run is unconfined, with a 200 000-byte objective, sh -c 'exit 3' and a wall of 1s. On a node that runs only sandboxed processes it is refused. On one that allows unconfined runs it ends exited 3 alone, as a session and at a terminal. As a session each of whose deliveries starts a process, given no delivery, it ends budget, past its wall. A run whose started's index fails (StandInFossil failing test-crosslink) raises Fossil::Failed from watch. After each, no scratch directory of this process is left. Against the old code it failed at the first exited run, watch answering nil (scratchpad wl/before-1.txt). Run first, the each-delivery session failed the same way (wl/before-each.txt). Run first, the failed write left its scratch directory (wl/before-failed-write.txt). spec/ocx/watcher_spec.cr, "ends a running session's program on TERM: stopped for a stop that counts, lost with none; a second watcher leaves the first's lock as it is", and "keeps a second run of the same user waiting while the first is in care, and hands it on after (case 26)": with the run's lock held by another watcher, and the user's by another holder, watch still answers nil. spec/ocx/care_spec.cr, "takes a run's lock only once a try of it under way is over, so the try never makes the taking fail", now reads take's answer rather than rescuing its error. spec/ocx/sandbox_spec.cr, "runs no task unconfined": a program not installed is refused through the String overload too. spec/ocx/program_spec.cr:341 is unchanged: the echo run's ended no longer depends on whether echo exits before its line is written. crystal run ci.cr green: 449 examples, 0 failures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVpmCsBwVF3d4zhMHHDAxs |
|---|---|
| Downloads: | Tarball | ZIP archive |
| Timelines: | family | ancestors | descendants | both | agent-tools |
| Files: | files | file ages | folders |
| SHA3-256: |
35b2b0febb45eb09d136744c57d0b7fd |
| User & Date: | opus-ocx2 2026-10-21 12:07:15.542 |
|
2026-10-21
| ||
| 13:24 |
A run ends however its watcher fails after started: a terminal its program left full is typed into without spinning, a kept log whose place the program took is not kept, and any other failure of the watcher's own ends the run lost, naming it
Intent: protocol v1's promise that a run that started gets an ended, exited with its code, budget, stopped or lost (docs/protocol-v1.md, sections 4 to 7 and 10), never silence. An adversarial review of 35b2b0febb (scratchpad review-watcher-lost.md) found two ways a run's own program could keep its ended from being written. First, a terminal session's watcher hung at 100% CPU and ignored TERM, holding the run's lock for ever, so no check could answer the run lost. When a pseudo-terminal's input holds a whole line no program has read, Linux holds what more is typed, up to 64 KiB, and then a write to the near end gets EAGAIN. Once the far end hangs up, Crystal 1.18's polling event loop takes the descriptor as ready for ever (Waiters#add), so its own write tries again without ever yielding, and neither the SIGCHLD reaper nor the TERM trap runs. The review's reproduction: a session with terminal true, command sh -c 'sleep 1; exit 3', wall 6s, given one 100 KB post while it sleeps, killed at 40 s. Second, a run's program made a directory where its kept log goes (mkdir "$HOME/stderr.log"); the rename of the node's new file onto it raised File::Error, the watcher crashed, since the hook rescues only Fossil::Failed and Refused, and a temp file was left in the workspace. And beyond those two, any unexpected failure of the watcher's after started left no ended until the node answered the run lost: at its check, as it starts or when run by hand, or when a stop for the run arrived. Expression: src/ocx/terminal.cr: Terminal::Typed, an IO, writes what is typed at the near end through LibC.write itself: it advances past what Linux took, and on EAGAIN asks poll(2), at once, whether every holder of the far end has closed it (POLLHUP, HUNG_UP), raising IO::Error if so; otherwise it parks the fiber through Crystal::EventLoop.current.wait_writable until Linux takes more, the far end hangs up, or close wakes it, and it raises once the near end is closed. So nothing spins, and the wait has a cause, the kernel's word, not a timer: the review's proposed loop slept 10 ms on EAGAIN, but precept causation's guard (spec/guards/causation_spec.cr) holds src/ to no sleep but the listed ones, and listing this one would loosen it. Every write to the near end goes through Typed: Terminal#type's lines and end of input, and a joined person's keys (Terminal.copy(person, Typed.new(@near)) in serve). The lib C binding gains poll and its struct. src/ocx/workspaces.cr: Workspaces.written, moved whole with its comment from src/ocx/watcher.cr, which was at its 400 lines: a kept log that cannot be kept in the workspace is not kept. The new file is made with File.tempfile (a File::Error there, as for a workspace removed, makes nothing), written unbuffered, and renamed into place; an IO::Error writing or renaming it (File::Error is an IO::Error), as for a directory at that name or a full disk, removes the new file, which an ensure closes. The logs are still posted where the run is, and the ended says exited with the code, which table 12 gives no note. src/ocx/watcher.cr: in prepared, everything between the started and the ended (the process launched and watched, its kept logs, shares, verdict, the ending decided, the workspace cleared) is in a begin whose rescue takes any failure. It ends the program, if it still runs, as a stop does (Stopping#ask), writes the run's ended itself under the node's lock, lost, its note "its watcher failed: CLASS: " and the message's first line, any password in a URL masked (Fossil.masked) and the run's secrets redacted (Values.redact), then raises the failure again. If that write fails too, as with Fossil down, its own error is raised instead, and the node's check answers the run lost (case 2), as the comment says. A failure of the ended's own write is outside the rescue, so no second ended is written. Launch.secrets_of (src/ocx/launch.cr) is the watcher's secrets_of, moved whole with its comment to make room; watcher.cr is 393 lines, terminal.cr 400, within precept C1 (File Boundedness). HISTORY.md decision 95: this is no sweep under decision 39, but the watcher reporting its own run's end, as it reports budget and stopped. docs/protocol-v1.md: table 12's lost and section 10's row include a failure of the node's own while the run is in its care, its process ended first, its note naming the failure; section 8.2 says a kept log whose place the process took is not written. docs/protocol-v1-failures.md gains cases 31 (the program leaves its terminal full), 32 (the program takes a kept log's place) and 33 (the watcher fails after started), under a new heading for the last two. docs/protocol-v1-fossil.md, section 5, GLOSSARY.md (Watcher), docs/design/modules.json and the modules.md generated from it say the same, and docs/crystal.md says how Crystal 1.18's write behaves once a terminal's far end has hung up. Review: Gemma (gemma4:31b) read the change five times. Held: the fourth reading's, that the new file is left when writing it fails inside File.tempfile's block, before fresh is assigned. Tried, it was worse: such a write raises IO::Error (EFBIG under a file-size limit, SIGXFSZ ignored; ENOSPC on a full disk), which rescue File::Error missed altogether, so the run would have ended lost by the rescue above and the file stayed: a probe raised "IO::Error: write (w/f306qmarstderr.log): File too large" and left that file (scratchpad ns/probe/full-read.txt). Workspaces.written was rewritten as above, and the same probe returns, leaving nothing (ns/probe/full-after.txt). Misread, each a Clarity defect changed until later readings read it right: the first reading, that slice += taken does not advance a Bytes, taken for a String. Bytes is a Slice(UInt8), and Slice#+ advances it, as the 100 KB typed showed; it is now slice = slice[taken..], and the second to fifth readings did not raise it. The second reading, that the near end could be closed between the closed check and the write, and that wait_writable on a closed descriptor could crash or loop: this program has one thread, so no fiber runs between the check and the write, and Crystal's close wakes a fiber waiting on the descriptor, which then finds it closed. Comments at the check and the wait now say so, and the third to fifth readings did not raise it. The second reading also took the method's ensure to call Workspaces.cleared: the diff it was given, at three lines of context, cut the ensure's body, which removes the scratch directory; given at Fossil's default context, the third to fifth readings did not raise it. The third reading, that File.tempfile with a block gives back the block's value: it gives back the file, closed (Crystal's file/tempfile.cr); a comment said so, and the fourth reading did not raise it, but found the held defect above. Wrong: the first reading's write returning 0 without EAGAIN: a terminal's write of bytes takes at least one, or says EAGAIN on a descriptor that does not block (n_tty_write), as a comment now says. The second reading's finding that a run is left with no ended, and the first failure not raised, when the lost ended cannot be written: when Fossil is down nothing reaches the repository; the error that kept the ended out is the one raised, which the hook answers as it answers every Fossil failure in a watch, and the node's check answers the run lost, as the task asked. The fifth reading's, that deleting the new file while it is open fails on Windows: OCX's node runs on Linux alone (setsid, bubblewrap, /proc, pseudo-terminals), where unlinking an open file removes its name at once, and the ensure closes it. Demonstrates: spec/ocx/terminal_spec.cr, "ends a session that reads none of what is typed, past what its terminal holds, as its program exits or on TERM, as a stop says (case 31)": an unconfined session at a terminal, wall 6s, its run's line typed and not read, given a post of 100 000 bytes as it launches; sh -c 'sleep 1; exit 3' ends exited 3 within its wall. The same session with a program that says when it has slept 1 s and then sleeps 30, sent TERM then, ends stopped with a stop posted and lost (Watcher::UNSTOPPED) without one. Against the old src, the first part ran at 97% CPU for 45 s until killed at 60 s (scratchpad ns/before-31.txt), and the TERM part alone the same (ns/before-31-term.txt). spec/ocx/watcher_spec.cr, "ends a run whose program made a directory where a kept log goes, that log kept there no more but posted (case 32)": an unconfined run whose only step is its directory, sh -c 'mkdir stderr.log; echo oops >&2; exit 3', ends exited 3; its workspace holds only the empty directory stderr.log and stdout.log, and the posted logs hold oops. Against the old src, watch raised File::Error "Error renaming file ... Is a directory" (ns/before-32-33.txt). spec/ocx/watcher_spec.cr, "ends a run lost, its note naming the failure, masked, when its watcher fails after its started, ending its program first (case 33)": a Stopping whose launched raises an IO::Error after the program is launched, standing in for the write naming it in the run's lock on a full disk, its message naming https://builder:second@example.invalid and a second line. watch raises it; the thread holds one ended, lost, "its watcher failed: IO::Error: No space left on device, near https://builder:***@example.invalid/fixture"; the program, sleep 30, was ended by TERM; and the node's check then answers nothing. Against the old src there was no ended (ns/before-33.txt). spec/ocx/watcher_spec.cr, "leaves its ended stored but not indexed when a writer outside OCX holds the repository, ...": the ended's own write failing still writes no other. spec/ocx/terminal_spec.cr, "types nothing into a terminal let go, raises what a write met other than a full terminal, and is only written": Typed on a closed descriptor raises "the terminal was let go", on /dev/full "typing into the terminal: No space left on device", and its read raises; the first gate run of this change failed on the coverage ratchet alone, src/ocx/terminal.cr missing 3 statements (scratchpad ns/gate-1-coverage.txt); with this example, a coverage run of the terminal, attach and watcher specs alone runs all 120 of that file's statements. spec/ocx/terminal_spec.cr's other examples and spec/ocx/attach_spec.cr, "joins a person's terminal to the run's at its window, ...": lines, an end of input and a person's keys are typed through Typed. crystal run ci.cr green: 453 examples, 0 failures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVpmCsBwVF3d4zhMHHDAxs check-in: e9b388af16 user: opus-ocx2 tags: agent-tools | |
| 12:07 |
A run whose program exits without reading its run gets its ended, and the watcher answers nil only for a lock another holds
Intent: protocol v1's promise that a run that started gets an ended, exited with its code, budget, stopped or lost (docs/protocol-v1.md, sections 4 to 7 and 10), never silence. An adversarial review of 9118789d1a (scratchpad review-headroom.md, finding 1) saw the gate fail once on a clean tree: spec/ocx/program_spec.cr:341, an unconfined echo run, hit watch(...).not_nil! "Nil assertion failed", and an empty /tmp/*ocx-run directory was left, which only Watcher.prepared makes. Watcher.watch ended in a rescue of IO::Error around everything it did, meant for the two lock takings, so an IO::Error after the run started was answered nil: no ended after the started, and the scratch directory left. The review forced the same path: an unconfined run given a 200 KB objective, whose program exits without reading it. watch returned nil, the directory was left, and no ended followed the started; with a 10-byte objective it returned 3 reports. The cause: the run's first line went to Process.new as an IO::Memory, which Crystal copies into a pipe in a fiber of its own, and Process#wait raises what that copy met, here EPIPE, before it gives the status. A session each of whose deliveries starts a process was given its lines the same way and was lost the same way. A session's relay and a terminal write their own pipe or pseudo-terminal in a fiber that rescues IO::Error: the same program and objective, as a session and at a terminal, ended exited 3 before this change. Which IO::Error failed the gate that once is not known: a short line given to a program that exits at once did not raise in 500 tries on a quiet host, with Process.new or with this change (scratchpad wl/race-probe-short.txt). With this change, whether the program read its line before it ended changes nothing. Expression: src/ocx/sandbox.cr: Sandbox.start, the one call that starts a run's process under precept I7 (Single Process Site), now takes its input by type. An IO::FileDescriptor the caller holds, a terminal's or a relay's, is passed on as before. A String, the text given, is written through a pipe by a fiber of its own, which then closes it and rescues IO::Error: a process that ends without reading all it was given is no error, and its status says how it ended. This process's copy of the read end is closed once the process has started, as Process.new closes its own pipes' ends, so the fiber's write fails at once when the process ends. Nothing gives Process.new an IO::Memory as input any more, so Process#wait never raises a copy's error. src/ocx/launch.cr: a task's empty input is "". src/ocx/watcher.cr: launched passes relay.try(&.input) || input.to_s, a descriptor or a String, which the compiler sends to the overload for each; its input stays an IO::Memory, whose lines a terminal types. watch no longer rescues IO::Error: it returns nil only when Node.take_run or Node.take answers that another holds the lock, as before. Any other failure raises, and the node's check answers lost a started run left with no ended (docs/protocol-v1-fossil.md, section 5; failure case 2). Before, such an error was answered nil, which wrote no ended either. prepared removes its scratch directory, the files made for the run's sandbox, in an ensure, whatever ends it: a refusal, a decision not to start, the run's end, or an error such as a write that fails. That replaces its three removals, and it now runs after the ended is written. The comments on watch and watched say that each lock is let go as its File.open block closes its file, an error too. src/ocx/care.cr: Node.take_run and Node.take answer whether they took the lock, false while another holds it, and took? rescues only the non-blocking try of flock, so no other error is read as a lock another holds. docs/protocol-v1-failures.md gains case 30, the program never reads its run. docs/crystal.md says that Process#wait raises what the copy of an input met. src/ocx/watcher.cr is 400 lines, within precept C1 (File Boundedness). Review: Gemma (gemma4:31b) read the change seven times. Held: none. Misread, each a Clarity defect, changed until a later reading read it right: first, that closing the read end right after the start lets the child see an end or an error before it reads. The child holds its own copy from its fork on; the comment now says so, and that Process.new closes its own ends the same way, and the second reading did not raise it. Second, that an error escaping watch leaves the run with no ended for ever. watch's comment said nothing of other failures. It now says the node's check answers lost a started run left with no ended, and the third to fifth readings did not raise it. Reworded as "left to the node's check", the sixth raised it again; reworded to say what the check does, the seventh did not. Third and fourth, that the run's lock, then the user's, stays held when decided raises. Each is opened by File.open with a block, which closes the file and so lets the lock go. The comments on watch and watched now say so, and the next reading read each right. Sixth, that the scratch directory leaks when watched fails before prepared. Only prepared makes it, and its ensure covers it from there; the ensure's comment now names the scratch made above, and the seventh reading did not raise it. Wrong: the first reading's finding that the pipe's writer leaks if the fiber never reaches its ensure. The fiber's ensure closes it whenever the fiber runs. The fifth reading's runtime TypeError for a relay whose input is no descriptor: Relay#input is declared IO::FileDescriptor, and Crystal picks an overload when it compiles. An input of any other type does not compile, as this change's first build showed: "expected argument #2 to 'OCX::Sandbox.start' to be IO::FileDescriptor or IO::Memory, not IO". The sixth reading's claim that watch runs inside Node.locked in the node's check loop: the hook's watch mode calls it in a process of its own, holding no lock of the node's. The seventh reading's NameError when the scratch is not yet assigned: scratch is assigned as prepared starts, before any begin. Crystal types a body's local as nilable in its ensure, which `if scratch` handles, and an unknown name does not compile. Demonstrates: spec/ocx/watcher_spec.cr, "ends a run whose program exits without reading its run, alone, as a session, at a terminal, or each of whose deliveries starts a process, as it ended (case 30), leaving no scratch directory, whatever ends the run". The run is unconfined, with a 200 000-byte objective, sh -c 'exit 3' and a wall of 1s. On a node that runs only sandboxed processes it is refused. On one that allows unconfined runs it ends exited 3 alone, as a session and at a terminal. As a session each of whose deliveries starts a process, given no delivery, it ends budget, past its wall. A run whose started's index fails (StandInFossil failing test-crosslink) raises Fossil::Failed from watch. After each, no scratch directory of this process is left. Against the old code it failed at the first exited run, watch answering nil (scratchpad wl/before-1.txt). Run first, the each-delivery session failed the same way (wl/before-each.txt). Run first, the failed write left its scratch directory (wl/before-failed-write.txt). spec/ocx/watcher_spec.cr, "ends a running session's program on TERM: stopped for a stop that counts, lost with none; a second watcher leaves the first's lock as it is", and "keeps a second run of the same user waiting while the first is in care, and hands it on after (case 26)": with the run's lock held by another watcher, and the user's by another holder, watch still answers nil. spec/ocx/care_spec.cr, "takes a run's lock only once a try of it under way is over, so the try never makes the taking fail", now reads take's answer rather than rescuing its error. spec/ocx/sandbox_spec.cr, "runs no task unconfined": a program not installed is refused through the String overload too. spec/ocx/program_spec.cr:341 is unchanged: the echo run's ended no longer depends on whether echo exits before its line is written. crystal run ci.cr green: 449 examples, 0 failures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVpmCsBwVF3d4zhMHHDAxs check-in: 35b2b0febb user: opus-ocx2 tags: agent-tools | |
| 10:55 |
Questions read every post and ticket change the ocx-message tag was ever put on, whatever its value, cancelled or not, so editing a tag after a confirmation changes neither who may confirm the question nor what closed it; the docs say what the reader trusts of a run's started
Intent: the maintainer, 2026-10-21: "Whoever sent the run (you, usually) may also confirm its questions." (HISTORY.md decision 93), and the standing rule decision 94 quotes, "please don't go down any path with a possible race condition". An adversarial review of 2ea82d7e36 (scratchpad review-qnr.md) found that a confirmation changes after it was made: "any user holding `i` can reopen a question the run's sender closed", since said_tagged_asked read only artifacts whose ocx-message tag still stood with one of the kinds it wanted. It reproduced this on a copy of the server: run-zz cancelled the tag on the run R1 and pushed one artifact, and the question read open, with stone's confirmation counted as a plain response; giving R1's started the tag value "stop R1" did the same. Yet a tag "only chooses what is read" (docs/protocol-v1-fossil.md, section 2), and src/ocx/fossil/message_tags.cr says the tag "only says where to read: what an artifact holds is read from it". The same review found three more things. The docs claimed more than the reader checks: "src/ocx/commands/questions.cr:287 accepts a started from whatever login the run names as its node. A sender can name itself, so no real node has to start the run." src/ocx/fossil/forum.cr still said "a sync meanwhile sends it", though the maintainer, 2026-10-21, wrote: "agents are mistakenly trying to use `sync`, when they should just use a regular `fossil push`." (decision 92). And SCENARIOS.md, docs/parity-v1.md and USAGE.md still showed the question command without --run, which the patterns now write (decision 94). Expression: src/ocx/fossil/message_tags.cr: MESSAGE_TAGS_EVER is every row of the tag, whatever its value, a cancelled one included: Fossil's tag cancel keeps the artifact's row and sets its tagtype to 0. MESSAGE_TAGS, the live rows that the rounds and status read, is now MESSAGE_TAGS_EVER with "AND x.tagtype>0"; its SQL is unchanged. said_tagged_asked is still one query: b.rid IN (SELECT x.rid FROM MESSAGE_TAGS_EVER), with no filter on the tag's value or state. The one message each post or ticket change holds (Messages.find) says what it is, as run_of, Question.in and Response.in already read it. A stop or an ended is passed over, and a check-in or a wiki page carrying the tag holds no message, so it is not read. The comment says why no filter is kept: a filter would let anyone holding i hide a question's run, its started or its confirmation. src/ocx/commands/questions.cr: ReadAsked's description now says what it reads. RunStarted's comment says what is trusted: the started, as the word of the login the run names as its node that the run passed every rule. What the run's sender could do when they sent it, and whether that login is a node at all, are not checked. What the reader checks is unchanged. docs/protocol-v1-fossil.md section 2 says what questions reads and why an edit of a tag changes nothing there. It also says that a round and status keep the limit given there: they read by the tags as they stand, so a tag cancelled or changed leaves its message one whose tag is missing or wrong, found only as the end of that paragraph says. docs/protocol-v1-questions.md section 2 now says what the reader checks of a run and its started and what it trusts, the login the run names as its node. It says plainly that one who names their own login as a run's node and writes its started may confirm a question whose sender names that run, while the patterns' --run $$run$$ names only the run whose process asks. The table row on runs taking no password step says a node refuses that step in a run from a user holding R, which this reader does not check. The row on the started names the login the run names as its node. The paragraph on moderation says a run's sender holding neither admin nor setup may have their confirmation held for a moderator. docs/protocol-v1-tools.md (questions), docs/interfaces.md (the ocx-message tag row), docs/design/modules.json and the modules.md generated from it, and HISTORY.md decision 94 say the same. src/ocx/fossil/forum.cr: "a sync meanwhile sends it to be read where it arrives" becomes "and fossil push, or a clone's pull, meanwhile carries it". SCENARIOS.md steps 1 and 2 and docs/parity-v1.md now show the question line with --run $$run$$ and --run RUN, and USAGE.md's synopsis with [--run RUN] inside --for. Timed on copies with a binary built from this tree, median of five (scratchpad qtt/page-times.txt and questions-times.txt). Copy A is scratchpad adv10/r0.fossil: 252 questions, 532 rows of the tag. Copy B is copy A plus an ended for each of its 35 posted runs: 567 rows. On copy A, the list page took 0.687 to 0.710 s for stone, other, ec2-user and run-zz, against 0.686 to 0.710 s with 2ea82d7e36's binary. On copy B it took 0.697 to 0.721 s, against 0.687 to 0.709 s. The questions tool took 0.388 to 0.389 s on copy A and 0.393 to 0.395 s on copy B, against 0.385 to 0.388 s. On a copy of this workspace's clone of the server, which holds 8 rows of the tag, the page took 0.066 to 0.067 s and questions 0.033 to 0.034 s. On copies of the review's edited repositories, rev12/e.fossil (R1's tag cancelled) and rev12/d.fossil (S1 tagged "stop R1"), 2ea82d7e36's binary read Q1 as open with confirmers [run-zz]. This tree's binary reads it closed: post refuses a response, saying "the question is closed already: stone, who sent the run that asked it, confirmed it answered" (scratchpad qtt/repro.txt). Review: Gemma (gemma4:31b) read the change five times, the fifth after the spec below was folded into an existing example. Held: none. Misread, each a Clarity defect changed until the next reading read it right. First, that the query should keep its filter on the tag's value, and that an ended whose text held a question might be taken for one. Each artifact holds one message, the first OCX block, and Question.in and Response.in read only one of their own kind. The comment, which had listed the kinds a reader of the questions needs as if the query chose them, now says the query keeps no filter on value or state and why, and that the message, never the tag, says what each is; the second reading did not raise it. The fix the first reading proposed would also have left the started tagged "stop RUN" hidden. Second, that said_where joins MESSAGE_TAGS and so drops a cancelled row again. said_where filters only on the condition it is given, and its tagged only reports the tag. The comment now says so, and the third reading did not raise it. Third, that a cancelled confirmation "remains hidden because its tag is cancelled". MESSAGE_TAGS_EVER's comment now says Fossil's tag cancel keeps the row, setting its tagtype to 0, so the artifact is still selected, and the fourth reading did not raise it. Wrong: the third and fourth readings' finding that a check-in carrying the tag is selected by the subquery but matched by no post or ticket change, at a cost, the fourth added, as cancelled tags grow. A check-in holds no message (docs/protocol-v1-fossil.md, section 2), so not reading one is the design, which the comment says. The restriction they proposed repeats what said_where's join on forumpost and ticketchng already does, and would not parse, as MESSAGE_TAGS_EVER already ends in its WHERE. The timings above stay under 0.4 s for questions and 0.73 s for the page. Also wrong: the fifth reading's finding that said_where fails on a condition that is a subquery: the condition has the form, b.rid IN (SELECT x.rid FROM ...), it always had. And its finding that thousands of tagged artifacts that are no question would be read: a tag is anyone's to add with any value, so the old filter never bounded what is read, as anyone could tag ten thousand posts "question"; each run now adds one more row, its ended, beside its run and its started, which the old query read already, and 35 endeds cost 7 ms above. Its fix named a column, b.kind, that blob has not. It waited, set aside in a stash, for gate-headroom's check-in, 2a256e2adc, which runs the specs in three parts side by side and brought the gate from 180 to 191 s down to about 92 s, under its 180 s limit. Demonstrates: spec/ocx/questions_spec.cr, "lets whoever sent the run a question names as asking it confirm it answered, ... whatever they, or an edit of a tag, may do since; no one else". Once its questions are closed, it records every question's confirmers and what closed it. Merge was closed by the fixture user, who sent the run mine; rename and again by boss, who sent the run theirs; ship by other, its sender. Then it makes the review's two reproductions and a third: the tag on mine cancelled, the started of theirs that counts tagged "stop THEIRS", and the tag on other's confirmation of ship cancelled. Every question reads the same. Against 2ea82d7e36's query it failed at that assertion: merge, rename and again lost their run's sender and read open, and ship read open (scratchpad qtt/before-folded.txt). This was first a separate example with three fresh runs, which failed the same way (scratchpad qtt/before.txt). It was folded into this one, whose runs and confirmations it reuses, after two gates run with it failed only on the 180 s pace limit, at 188 s and 181 s (scratchpad qtt/gate-1-pace.txt and gate-2-pace.txt), its spec run taking 2:23 and 2:34. spec/ocx/message_tags_spec.cr, "is on a question and a response, ... and questions reads every post it was ever put on, whatever its value, cancelled or not": a question whose tag was cancelled and a started tagged as a stop are read, with the run and the response, each with its tag as it stands. It failed against the old query too (scratchpad qtt/before-both.txt). spec/scenarios/question_spec.cr, "runs the steps, a wiki page's among them": the scenario's question is asked with run, as the patterns' --run $$run$$ asks it, and its block names the run. spec/ocx/create_spec.cr, "names a run stored but not indexed, ...": the error ends "and fossil push, or a clone's pull, meanwhile carries it". crystal run ci.cr green: 448 examples, 0 failures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QVpmCsBwVF3d4zhMHHDAxs check-in: 9118789d1a user: opus-ocx2 tags: agent-tools | |