yuris/noctalia-timew
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(timew): hand runAsync argv arrays, delete shQuote #11

Merged
yuris merged 2 commits from upgrade/u1-argv into main 2026-09-01 12:44:18 +00:00
Conversation 2 Commits 2 Files changed 3 +587 -249
yuris commented 2026-09-01 12:07:01 +00:00
Owner
Copy link

U1 of the API-28 upgrade: every noctalia.runAsync call site that does not need a
shell now hands the host an argv array, and shQuote is deleted from both entries.
No plugin_api bump — the argv form works at 3 (processArgsFromLua →
startAsyncProcess → execvp, no /bin/sh on that branch; the string branch is
the only thing that wraps).

grep -c shQuote timew/panel.luau timew/bar.luau → 0 and 0.

Call-site census

Ten noctalia.runAsync calls before, ten after — this changes what each is handed,
not how many there are.

form before after sites
argv array 0 5 todayTotal, tags, rangeTotal, Timew.exec, bar's onClick
sh -c script + positional data 0 2 dayTotals, intervalsForDay
bare shell string 10 3 activeInterval, allIntervals, bar's sync

Five shells remain, down from ten. Two of the five disappeared by restructuring
rather than by being kept (option b), and the tag/date in the two that survive
reaches them as $1, so no script in this plugin contains user data any more
and no quoting helper survives under any name.

Why each remaining shell survives

site shell feature data in it verdict
Timew.cmd.activeInterval (panel), sync (bar) s=$(…), [ -n "$s" ], &&, nested $(date -d …) none (a) keep. A conditional over two command substitutions is not an argv array. Nothing is interpolated, so there is nothing for argv to protect. Restructuring means parsing timew's zoneless local-time stamp in Luau, in both entries, with DST to get wrong — invented risk for no safety gain.
Timew.cmd.dayTotals for over seq, per-day $(…), case the tag → $1 (a) keep, data moved out. 14 per-day timew summary calls with a case that floors prose to 0:00:00. As argv it is 14 subprocesses and 14 async landings to order. The tag is ${1:+"$1"}, which also preserves "no tag adds no argument": with $1 empty the loop that expands it iterates zero times.
Timew.cmd.intervalsForDay while with break, ${l%%T*}, case the date → $1 (d="$1") (a) keep, data moved out. The walk's early break on ids that predate the day is the bound; without it the day view fetches 200 ids.
Timew.cmd.allIntervals while over dom.tracked.$i none (a) keep. Same loop, no data at all.

Restructured away (option b), both because the shell existed only to run an awk:

  • todayTotal: timew summary :day 2>/dev/null \| awk 'NF{l=$NF} END{print l}' → {"timew","summary",":day"}, with the awk as Timew.lastField. 2>/dev/null went too — the callback exposes stderr separately.
  • tags: timew tags 2>/dev/null \| awk 'NR==3{w=length($1); next} NR>3 && NF{print substr($0,1,w)}' → {"timew","tags"}, with the column-width cut inside Timew.parseTags.
  • rangeTotal: same awk as todayTotal; now {"timew","summary",":all"[, tag]}.
  • Timew.exec / bar's onClick: cmd .. " 2>&1" → argv, and the toast reads trim(stderr .. stdout). 2>&1 bought "the message reaches the user whichever stream carried it"; the concatenation buys the same thing with no shell. Both directions are asserted.

Proof

Both gates:

$ nix shell nixpkgs#luau -c luau test.luau
viewModel: 14 states, 1485 field comparisons
ok
$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings

The two Luau replacements are byte-identical to the awk they replace, run
against this machine's live timew:

lastField(`timew summary :day`)  = "17:47:41"   old pipeline = "17:47:41"
parseTags(`timew tags`)          = Learn C++|Test|Timer plugin|client-x|deep work|
                                   email|focus|hello|heyy|review|standup|test
                                   == old awk output, trimmed, exactly

The two surviving scripts produce identical output to the strings they replace,
old builder (from HEAD~1) versus new script + $1, against live data:

dayTotals, tag='deep work' -> IDENTICAL (14 lines)
dayTotals, no tag          -> IDENTICAL (14 lines)
intervalsForDay 2026-08-31 -> IDENTICAL (33 lines)

The hostile-tag assert

local HOSTILE = [==[a b'c"d $HOME `id` ;rm -rf / #|& \ ]==] .. "\ntail"
argvIs(T.cmd.rangeTotal(HOSTILE), { "timew", "summary", ":all", HOSTILE },
	"a hostile tag is one argv element, byte for byte, on an argv command")
argvIs(T.cmd.dayTotals(nil, HOSTILE), { "/bin/sh", "-c", DAY_SCRIPT, "sh", HOSTILE },
	"and one positional parameter, byte for byte, on the shell one")
for _, argv in ipairs({ T.cmd.dayTotals(nil, HOSTILE), T.cmd.intervalsForDay(HOSTILE) }) do
	assert(not string.find(argv[3], HOSTILE, 1, true), "no data reaches a script's text: " .. shown(argv[3]))
	assert(argv[4] == "sh", "the element after the script is $0, which is what puts the data on $1")
end

The same tag also runs through bar's onClick loop, and DAY_SCRIPT is asserted
to be the same constant with a tag and without one — which is the assert that
makes the surviving sh -c sites safe rather than merely quoted.

Driven through the real /bin/sh (n=0; for a in ${1:+"$1"}; do n=$((n+1)); printf "arg%s=[%s]\n" "$n" "$a"; done; printf "count=%s\n" "$n"):

tag = "a b'c\"d $HOME `id` ;rm -rf / #|& \\ \ntail"
  arg1=[a b'c"d $HOME `id` ;rm -rf / #|& \
  tail]
  count=1
tag = ""            -> count=0          (no tag still adds no argument)
tag = "deep work"   -> arg1=[deep work] count=1

And the real dayTotals script with that tag: 14 normal lines of zeros, empty
stderr, / still has its 18 entries. $HOME unexpanded, `id` not executed,
;rm -rf / not a command, the newline not a separator.

What a reviewer should challenge

  1. ${1:+"$1"} vs the deleted tagArg. Both mean "no tag, no argument", but by different mechanisms. count=0 above is the evidence; the alternative was an unconditional "$1" passing an empty argument to timew summary.
  2. stderr .. stdout replacing 2>&1. Ordering differs from a live merge when both streams carry text; for a failing timew only one does. If you would rather branch (stderr if non-empty else stdout), say so — it is two lines in each entry. Note the panel now reads result.stderr unguarded, so a host that ever omitted the field would error; U0 verified the field is present on both branches.
  3. parseTags counts records to find the rule on line 3, exactly as the awk did, so it iterates blank lines too (string.gmatch(stdout .. "\n", "([^\n]*)\n")). If timew tags ever grows or drops a leading blank line, the width lands on the wrong line — same fragility as the awk, now visible in Luau and covered by a generated fixture (tagsOutput) instead of a frozen post-awk string.
  4. parseTotal still returns the last field verbatim when timew prints prose. timew summary with no data yields e.g. "found.", and it did before this change too. Preserved deliberately — behaviour parity, not an endorsement. The case in dayTotals is what floors the chart.
  5. Numeric bounds are still interpolated (seq 0 13, -le 60, -le 200). They are module constants reached through arithmetic or a literal, not user data, so they stayed text; moving them to $2 would have grown the diff without closing anything.
  6. test.luau's takeJob now takes a table needle (exact argv match) or a string needle (substring of the command joined with spaces). Day-walk landings that used to match on d='2026-08-31' now use T.cmd.intervalsForDay(date) — a stronger match than the old substring.
  7. Not smoke-tested through the live panel, because the registered plugin runs from the main checkout and this branch must not enable or disable anything. The argv contract is U0's live probe; the shell scripts and both parsers were run against real timew from this worktree, as quoted above.
U1 of the API-28 upgrade: every `noctalia.runAsync` call site that does not need a shell now hands the host an argv array, and `shQuote` is deleted from both entries. No `plugin_api` bump — the argv form works at 3 (`processArgsFromLua` → `startAsyncProcess` → `execvp`, no `/bin/sh` on that branch; the string branch is the only thing that wraps). `grep -c shQuote timew/panel.luau timew/bar.luau` → `0` and `0`. ## Call-site census Ten `noctalia.runAsync` calls before, ten after — this changes what each is handed, not how many there are. | form | before | after | sites | |---|---|---|---| | argv array | 0 | 5 | `todayTotal`, `tags`, `rangeTotal`, `Timew.exec`, bar's `onClick` | | `sh -c` script + positional data | 0 | 2 | `dayTotals`, `intervalsForDay` | | bare shell string | 10 | 3 | `activeInterval`, `allIntervals`, bar's `sync` | Five shells remain, down from ten. Two of the five disappeared by restructuring rather than by being kept (option b), and the tag/date in the two that survive reaches them as `$1`, so **no script in this plugin contains user data any more** and no quoting helper survives under any name. ## Why each remaining shell survives | site | shell feature | data in it | verdict | |---|---|---|---| | `Timew.cmd.activeInterval` (panel), `sync` (bar) | `s=$(…)`, `[ -n "$s" ]`, `&&`, nested `$(date -d …)` | none | **(a) keep.** A conditional over two command substitutions is not an argv array. Nothing is interpolated, so there is nothing for argv to protect. Restructuring means parsing timew's zoneless local-time stamp in Luau, in both entries, with DST to get wrong — invented risk for no safety gain. | | `Timew.cmd.dayTotals` | `for` over `seq`, per-day `$(…)`, `case` | the tag → `$1` | **(a) keep, data moved out.** 14 per-day `timew summary` calls with a `case` that floors prose to `0:00:00`. As argv it is 14 subprocesses and 14 async landings to order. The tag is `${1:+"$1"}`, which also preserves "no tag adds no argument": with `$1` empty the loop that expands it iterates zero times. | | `Timew.cmd.intervalsForDay` | `while` with `break`, `${l%%T*}`, `case` | the date → `$1` (`d="$1"`) | **(a) keep, data moved out.** The walk's early `break` on ids that predate the day is the bound; without it the day view fetches 200 ids. | | `Timew.cmd.allIntervals` | `while` over `dom.tracked.$i` | none | **(a) keep.** Same loop, no data at all. | Restructured away (option b), both because the shell existed only to run an `awk`: - `todayTotal`: `timew summary :day 2>/dev/null \| awk 'NF{l=$NF} END{print l}'` → `{"timew","summary",":day"}`, with the awk as `Timew.lastField`. `2>/dev/null` went too — the callback exposes `stderr` separately. - `tags`: `timew tags 2>/dev/null \| awk 'NR==3{w=length($1); next} NR>3 && NF{print substr($0,1,w)}'` → `{"timew","tags"}`, with the column-width cut inside `Timew.parseTags`. - `rangeTotal`: same awk as `todayTotal`; now `{"timew","summary",":all"[, tag]}`. - `Timew.exec` / bar's `onClick`: `cmd .. " 2>&1"` → argv, and the toast reads `trim(stderr .. stdout)`. `2>&1` bought "the message reaches the user whichever stream carried it"; the concatenation buys the same thing with no shell. Both directions are asserted. ## Proof Both gates: ``` $ nix shell nixpkgs#luau -c luau test.luau viewModel: 14 states, 1485 field comparisons ok $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings ``` **The two Luau replacements are byte-identical to the awk they replace**, run against this machine's live timew: ``` lastField(`timew summary :day`) = "17:47:41" old pipeline = "17:47:41" parseTags(`timew tags`) = Learn C++|Test|Timer plugin|client-x|deep work| email|focus|hello|heyy|review|standup|test == old awk output, trimmed, exactly ``` **The two surviving scripts produce identical output to the strings they replace**, old builder (from `HEAD~1`) versus new script + `$1`, against live data: ``` dayTotals, tag='deep work' -> IDENTICAL (14 lines) dayTotals, no tag -> IDENTICAL (14 lines) intervalsForDay 2026-08-31 -> IDENTICAL (33 lines) ``` ### The hostile-tag assert ```lua local HOSTILE = [==[a b'c"d $HOME `id` ;rm -rf / #|& \ ]==] .. "\ntail" argvIs(T.cmd.rangeTotal(HOSTILE), { "timew", "summary", ":all", HOSTILE }, "a hostile tag is one argv element, byte for byte, on an argv command") argvIs(T.cmd.dayTotals(nil, HOSTILE), { "/bin/sh", "-c", DAY_SCRIPT, "sh", HOSTILE }, "and one positional parameter, byte for byte, on the shell one") for _, argv in ipairs({ T.cmd.dayTotals(nil, HOSTILE), T.cmd.intervalsForDay(HOSTILE) }) do assert(not string.find(argv[3], HOSTILE, 1, true), "no data reaches a script's text: " .. shown(argv[3])) assert(argv[4] == "sh", "the element after the script is $0, which is what puts the data on $1") end ``` The same tag also runs through bar's `onClick` loop, and `DAY_SCRIPT` is asserted to be the *same constant* with a tag and without one — which is the assert that makes the surviving `sh -c` sites safe rather than merely quoted. Driven through the real `/bin/sh` (`n=0; for a in ${1:+"$1"}; do n=$((n+1)); printf "arg%s=[%s]\n" "$n" "$a"; done; printf "count=%s\n" "$n"`): ``` tag = "a b'c\"d $HOME `id` ;rm -rf / #|& \\ \ntail" arg1=[a b'c"d $HOME `id` ;rm -rf / #|& \ tail] count=1 tag = "" -> count=0 (no tag still adds no argument) tag = "deep work" -> arg1=[deep work] count=1 ``` And the real `dayTotals` script with that tag: 14 normal lines of zeros, empty stderr, `/` still has its 18 entries. `$HOME` unexpanded, `` `id` `` not executed, `;rm -rf /` not a command, the newline not a separator. ## What a reviewer should challenge 1. **`${1:+"$1"}` vs the deleted `tagArg`.** Both mean "no tag, no argument", but by different mechanisms. `count=0` above is the evidence; the alternative was an unconditional `"$1"` passing an empty argument to `timew summary`. 2. **`stderr .. stdout` replacing `2>&1`.** Ordering differs from a live merge when both streams carry text; for a failing `timew` only one does. If you would rather branch (`stderr` if non-empty else `stdout`), say so — it is two lines in each entry. Note the panel now reads `result.stderr` unguarded, so a host that ever omitted the field would error; U0 verified the field is present on both branches. 3. **`parseTags` counts records to find the rule on line 3**, exactly as the awk did, so it iterates blank lines too (`string.gmatch(stdout .. "\n", "([^\n]*)\n")`). If `timew tags` ever grows or drops a leading blank line, the width lands on the wrong line — same fragility as the awk, now visible in Luau and covered by a generated fixture (`tagsOutput`) instead of a frozen post-awk string. 4. **`parseTotal` still returns the last field verbatim when timew prints prose.** `timew summary` with no data yields e.g. `"found."`, and it did before this change too. Preserved deliberately — behaviour parity, not an endorsement. The `case` in `dayTotals` is what floors the chart. 5. **Numeric bounds are still interpolated** (`seq 0 13`, `-le 60`, `-le 200`). They are module constants reached through arithmetic or a literal, not user data, so they stayed text; moving them to `$2` would have grown the diff without closing anything. 6. **`test.luau`'s `takeJob` now takes a table needle** (exact argv match) or a string needle (substring of the command joined with spaces). Day-walk landings that used to match on `d='2026-08-31'` now use `T.cmd.intervalsForDay(date)` — a stronger match than the old substring. 7. **Not smoke-tested through the live panel**, because the registered plugin runs from the main checkout and this branch must not enable or disable anything. The argv contract is U0's live probe; the shell scripts and both parsers were run against real timew from this worktree, as quoted above. 
noctalia.runAsync accepts an argv table and passes it straight to execvp with no
shell anywhere on that branch (processArgsFromLua -> startAsyncProcess ->
execvp), so the commands that never needed a shell now hand it a list and a tag
travels as one element. shQuote is deleted from both entries; nothing replaced
it, because there is nothing left to quote.

Four commands are genuine shell programs and stay scripts: the active-interval
conditional, the two DOM walks, and the 14-day summary loop. The two of those
that carry data take it as a positional parameter -- {"/bin/sh", "-c", script,
"sh", value} -- so every script in the plugin is now a constant string that no
interpolation touches.

Two shells went away instead of being kept: the awk behind `timew summary`'s
pipe is Timew.lastField, and the awk that cut the `timew tags` column is
Timew.parseTags. Both were checked byte-for-byte against the pipelines they
replace on live timew data. The mutating path's `2>&1` is gone too -- the
callback delivers stderr separately, so the toast reads `stderr .. stdout`.

test.luau pins argv element by element rather than freezing a command string,
and a hostile tag (space, both quote characters, $HOME, backticks, a semicolon
and a newline) is asserted to arrive as exactly one element and to appear
nowhere in any script's text.
yuris reviewed 2026-09-01 12:25:26 +00:00
yuris left a comment
Copy link

Verdict: REQUEST_CHANGES

The conversion itself is right and I could not break it. ${1:+"$1"} is byte-for-byte
the old tagArg under both /bin/sh shells I ran it through, the two Luau parsers agree
with the awk they replaced on every input timew can produce, the hostile-tag assert is
load-bearing rather than decorative, and both gates are green from a clean run. What
blocks is on the test side: two of the five surviving mutations land on lines this PR
wrote, and one of them contradicts a comment this PR also wrote.

18 mutations, 5 survivors. Full table at the bottom.

Blocking

1. test.luau:476-483 claims a drift check it does not perform

The comment says the two files' spellings of timew start <tag> "are the one pair
nothing shares. This is what catches them drifting." It does not. On main the same
assert read

lastCmd == panel.Timew.cmd.exec("timew start" .. panel.Timew.tagArg(tag))

which compared bar's command against panel's own builders, so panel drifting moved the
expected value. The replacement compares bar's argv against a literal written in the test,
and panel's side is asserted nowhere at all. startTag (panel.luau:891) is the one call
site in the panel that puts user data into an argv array, and no test reaches it:

SURVIVED  M12  startTag: drop the tag from the argv     -> run({ "timew", "start" })
SURVIVED  M12b startTag drifts from bar                 -> run({ "timew", "start", "--tag", tag })
SURVIVED  M5   panel onStop / onIpc("stop")             -> run({ "timew", "stop", "--dry" })

M5 survives because takeJob("timew stop") (test.luau:1338) is a string needle and
table.concat({"timew","stop","--dry"}, " ") contains "timew stop". That weakness
predates the PR, but the PR introduces the exact-argv needle that closes it and converts
every sibling to it, so this one is a missed conversion rather than an inherited limit.

Fix, at test.luau:1338:

local stopJob = takeJob("timew stop")
argvIs(stopJob.cmd, { "timew", "stop" }, "panel's stop is the same argv bar sends")
stopJob.cb({ exitCode = 0, stdout = "" })

and drive one start through the panel so startTag is reached at all — onIpc("start", HOSTILE)
then argvIs(takeJob("timew start").cmd, { "timew", "start", HOSTILE }). That restores the
cross-file pin the comment promises and kills M5, M12 and M12b together.

2. panel.luau:709 — the new result.stderr .. result.stdout has no test

Timew.exec is the only path from a failed timew start / timew stop to a toast, and
its body is new in this PR. Nothing exercises it:

SURVIVED  M6  Timew.exec: Parse.trim(result.stderr .. result.stdout) -> Parse.trim(result.stdout)
KILLED    M7  bar.luau:221, the identical drop                        -> test.luau:498

Two reasons it survives. test.luau:589 stubs the panel's notifyError as
function() end, so the argument is discarded, and no panel job is ever landed with a
non-zero exitCode — test.luau:1338 lands { exitCode = 0, stdout = "" }.

This matters more than a coverage gap because I confirmed against timew 1.9.1 that a real
failure puts its message on stderr only:

$ timew stop            # nothing tracked
  rc=255  stdout=[]  stderr=[The end of a date range must be after the start.]
$ timew bogusverb
  rc=255  stdout=[]  stderr=['bogusverb' is not a timew command. See 'timew help'.]

So if that concatenation ever regresses to result.stdout, every panel error toast goes
blank and the suite stays green. The line is correct today; it just is not defended.

Fix — capture the argument in the panel stub and land one failure, mirroring test.luau:494-503:

	notifyError = function(_, message)
		panelError = message
	end,

Advisory

3. panel.luau:709, bar.luau:221 — a missing timew now toasts an empty string

Patch-introduced, and the only user-visible behaviour change I found. On main the
command went through a shell, so the shell's own diagnostic was folded into stdout by the
2>&1 that this PR deletes:

$ env -i PATH=/usr/bin:/bin /bin/sh -c "timew start 'focus' 2>&1"
  rc=127  stdout=[/bin/sh: line 1: timew: command not found]  stderr=[]

With argv there is no shell to say that. U0's live probe of this host has
{"definitely-not-a-binary-xyz"} returning exit=127 with stdout="" and
stderr="" (the host does ::_exit(127) after a failed execvp, process.cpp:410), so
Parse.trim(result.stderr .. result.stdout) is "" and the toast body is empty. The
timeout case (exitCode 143) was already blank on both sides, so that half is not a
regression.

dependencies = ["timewarrior"] covers the install-time case but not a PATH the shell
does not share. A fixed fallback is two lines:

		if not ok then
			local text = Parse.trim(result.stderr .. result.stdout)
			noctalia.notifyError("timew", if text == "" then "timew failed (exit " .. result.exitCode .. ")" else text)
		end

noctalia.commandExists("timew") is also a real binding at this API if a preflight check
reads better than a fallback string.

4. test.luau:664-684 — three string needles left, and one of them hides a real gap

takeJob("seq 0 13") (:828, :1164, :1339, :1534, :1767) finds the day-totals job
by a substring of its script, so it matches whatever tag the job carries:

SURVIVED  M11  fetchDays: Timew.dayTotals(DAYS, state.filter, ...) -> Timew.dayTotals(DAYS, nil, ...)

That mutation removes the behaviour the README documents ("scopes the chart and the totals
to that tag") and the suite does not notice. T.cmd.dayTotals(DAYS, "focus") as a table
needle pins it. Same class as finding 1; separate because this one is inherited rather
than introduced.

Worth noting while you are in takeJob: the table branch compares
cmdText(job.cmd) == cmdText(needle) plus the element count, not element by element.
{"a","b c"} and {"a b","c"} both join to "a b c" with count 2. argvIs already does
the element-wise comparison — reusing it inside takeJob costs nothing and removes the
case entirely.

5. panel.luau:392-397 — "every script in this file is a constant string" is not quite true

Two numbers are still interpolated: (days or DAYS) - 1 at :571 and
(walkMax or DAY_WALK_MAX) / (walkMax or ALL_WALK_MAX) at :597 and :617. The first
is forced numeric by the subtraction; the two walkMax sites are raw concatenation, so a
string argument would be spliced into the script verbatim. Both call sites pass module
locals (loadDay at :832, fetchAll at :847), so nothing is reachable today — but
the comment is the file's central safety claim and a future caller will read it as a
guarantee. Either narrow the sentence to "no user data", or make it true:

		'while [ "$i" -le "$n" ] && [ "$i" -le ' .. math.floor(tonumber(walkMax) or DAY_WALK_MAX) .. ' ]; do',

6. panel.luau:445-448 — parseTotal still returns prose (pre-existing, parity preserved)

Recording it because this PR rewrote the function and added its fixtures. Real timew on an
empty day prints prose on stdout, and the last field of it is a timestamp:

$ timew summary :day
No filtered data found in the range 2026-09-01T00:00:00 - 2026-09-02T00:00:00.
$ ... | awk 'NF{l=$NF} END{print l}'     ->  2026-09-02T00:00:00.
  Timew.lastField(same input)            ->  2026-09-02T00:00:00.

Both spellings agree, so this is not a regression — main showed the same string. But
parseRangeTotal (:521-523) guards with ^%d+:%d%d:%d%d$ and parseTotal does not, and
the new SUMMARY fixture at test.luau:180 is the natural place to add the prose case now
that the reduction is Luau.

Verified

Everything below I ran; nothing is quoted from the PR description.

Gates, from a clean run in .worktrees/u1-argv:

$ nix shell nixpkgs#luau -c luau test.luau
viewModel: 14 states, 1485 field comparisons
ok                                                    EXIT=0
$ noctalia plugins lint timew
yuris/timew  ok      0 errors, 0 warnings             EXIT=0

Counts, derived not taken. shQuote is 0 in timew/panel.luau and 0 in timew/bar.luau;
the 2 hits in test.luau are both prose (:439, :468). No re-implementation under another
name: grepping gsub(.*', string.format("%q", \\" across timew/ and test.luau returns
only shown() at test.luau:276 (a %q formatter for assert messages) and dumpValue at
:927. tagArg and Timew.cmd.exec survive only in comments — clean cutover.
10 noctalia.runAsync sites (8 panel, 2 bar); 5 reach a shell: activeInterval and
allIntervals as strings, bar.sync as a string, dayTotals and intervalsForDay as
/bin/sh -c argv. Matches the claim.

Injection is structural, by inspection of all five. activeInterval, allIntervals and
bar.sync take no arguments at all. dayTotals and intervalsForDay build their script from
literals plus one numeric bound and pass data as argv[5]. No user-controlled value is
concatenated into any script text.

${1:+"$1"} against the deleted tagArg, driven through a real shell with a stub timew
that dumps its own argv. 17 tags, plus the no-tag and unset cases, under bash 5.3.15 as
/bin/sh and under dash. Identical argv in every case, including *, ~, $HOME,
a bare -, -x, --all, a single space, a tab, and the multiline hostile tag:

== <no tag / empty> ==   1=<summary> 2=<D> 3=<-> 4=<E> argc=4
tag "deep work"          1=<summary> 2=<D> 3=<-> 4=<E> 5=<deep work>  argc=5
tag "*"                  ... 5=<*>       argc=5      (no glob expansion)
tag "x'; rm -rf /; #"    ... 5=<x'; rm -rf /; #>      argc=5
hostile, with newline    ... 5=<a b'c"d $HOME `id` ;rm -rf / #|& \ \ntail>  argc=5
argdiff(/bin/sh) fail=0     argdiff(dash) fail=0

argc=4 for both nil and "" is the "no tag adds no argument" rule, preserved exactly.

Parser parity, 26 corpus cases against GNU awk 5.4.1. Old side is the real pipeline —
awk 'NF{l=$NF} END{print l}' then Parse.trim for lastField, and
awk 'NR==3{w=length($1); next} NR>3 && NF{print substr($0,1,w)}' then the old trim-and-drop
loop for parseTags. Corpus includes real timew tags and timew summary output, the empty-range
prose, empty input, blank-only, whitespace-only lines, trailing whitespace, CRLF, tabs, a
missing trailing newline, Learn C++, a tag wider than the rule, a rule with a leading
space, a blank line mid-list, and a header with no rows.

lastField diffs=2   parseTags diffs=0   cases=26
LASTFIELD DIFF formfeed   old=[a\fb]  new=[b]
LASTFIELD DIFF vtab       old=[a\vb]  new=[b]

Only \f and \v mid-line diverge, because Lua's %s counts them and awk's default field
splitting does not. timew emits neither. CRLF does not diverge, because main ran
Parse.trim over awk's output and Lua's %s already ate the \r.

Real timew 1.9.1 shapes, against a scratch TIMEWARRIORDB. timew tags is a blank line, a
header, a rule whose first dash run is the Tag column width, then padded rows — which is what
parseTags assumes. An empty database gives No data found. on stdout (one line, rc=0);
parseTags returns {} for it, as does the awk. Every failure I could construct put its
message on stderr with stdout empty; every success the reverse. I could not produce a
timew failure that writes to both streams, so the stderr .. stdout ordering is not
observable and is not a regression.

Argv element types. Nothing non-string can reach an array: startTag runs
Parse.trim first (:887), setFilter runs Parse.trim (:902), Config.get("tag")
is stringified (test.luau:439), and loadDay returns early when state.days[index] is
absent (:830-834), so intervalsForDay never sees a nil date.

Not checked. I did not run the plugin inside a live noctalia shell — the host contract
(result.stderr always a string, execvp PATH search, ::_exit(127) on a missing binary,
timedOut as a separate boolean) is U0's live probe plus its source citations, not my own.
I did not re-verify the surviving dayTotals awk under mawk or busybox awk, since the PR
does not touch it. I did not look at the panel on screen; every check here is the suite,
the shell, and timew. I did not exercise the 15 s / 10 s / 20 s timeouts.

Mutations

Applied to a copy of the worktree, one at a time, suite re-run and reverted after each.

# Mutation Caught What it proves
M1 dayTotals: splice tag back into the script text (real concatenation, not a literal edit) killed test.luau:311 the frozen DAY_SCRIPT sees it
M1' M1 again with every frozen-literal assert commented out (311-317, 321-330, 350-354) killed test.luau:359 the hostile-tag assert is load-bearing, not decorative — it fires alone
M2 intervalsForDay: splice date back into the script text killed :321, and :359 after the same neutering same, for the day walk
M3 ${1:+"$1"} -> "$1" (an empty tag now adds an empty argument) killed :311 the empty-tag rule is pinned, by the literal
M4 tag or "" -> tag (argv loses its 5th element) killed :283 argvIs' #argv == #expected is real
M5 panel onStop/onIpc -> run({"timew","stop","--dry"}) SURVIVED finding 1
M6 Timew.exec: drop result.stderr from the toast SURVIVED finding 2
M7 bar.onClick: drop result.stderr from the toast killed :498 bar's half is defended
M8 rangeTotal: insert the tag unconditionally killed :283 the empty-tag element rule holds
M9 lastField: first field of each line instead of the last killed :189 the SUMMARY fixture is not decoration
M10 parseTags: read the width off row 2 instead of row 3 killed :215 the generated tagsOutput fixture bites
M11 fetchDays: stop passing state.filter SURVIVED finding 4
M12 startTag: drop the tag from the argv SURVIVED finding 1 — the site is unreached
M12b startTag -> {"timew","start","--tag",tag} SURVIVED finding 1 — "catches them drifting" is false
M13 intervalsForDay: "sh" -> "", shifting the date onto $0 killed :311 the $0 placeholder is pinned
M14 todayTotal: :day -> :week killed :295 argv elements are pinned individually
M15 lastField: drop the if field guard killed :190 the whitespace-only-line assert can fail
M16 loadDay: pass day.date .. " " killed :684 dayJob's table needle pins the date
**Verdict: REQUEST_CHANGES** The conversion itself is right and I could not break it. `${1:+"$1"}` is byte-for-byte the old `tagArg` under both `/bin/sh` shells I ran it through, the two Luau parsers agree with the awk they replaced on every input timew can produce, the hostile-tag assert is load-bearing rather than decorative, and both gates are green from a clean run. What blocks is on the test side: two of the five surviving mutations land on lines this PR wrote, and one of them contradicts a comment this PR also wrote. 18 mutations, 5 survivors. Full table at the bottom. --- ## Blocking ### 1. `test.luau:476-483` claims a drift check it does not perform The comment says the two files' spellings of `timew start <tag>` "are the one pair nothing shares. This is what catches them drifting." It does not. On `main` the same assert read ```lua lastCmd == panel.Timew.cmd.exec("timew start" .. panel.Timew.tagArg(tag)) ``` which compared bar's command against *panel's own builders*, so panel drifting moved the expected value. The replacement compares bar's argv against a literal written in the test, and panel's side is asserted nowhere at all. `startTag` (`panel.luau:891`) is the one call site in the panel that puts user data into an argv array, and no test reaches it: ``` SURVIVED M12 startTag: drop the tag from the argv -> run({ "timew", "start" }) SURVIVED M12b startTag drifts from bar -> run({ "timew", "start", "--tag", tag }) SURVIVED M5 panel onStop / onIpc("stop") -> run({ "timew", "stop", "--dry" }) ``` M5 survives because `takeJob("timew stop")` (`test.luau:1338`) is a string needle and `table.concat({"timew","stop","--dry"}, " ")` contains `"timew stop"`. That weakness predates the PR, but the PR introduces the exact-argv needle that closes it and converts every sibling to it, so this one is a missed conversion rather than an inherited limit. Fix, at `test.luau:1338`: ```suggestion local stopJob = takeJob("timew stop") argvIs(stopJob.cmd, { "timew", "stop" }, "panel's stop is the same argv bar sends") stopJob.cb({ exitCode = 0, stdout = "" }) ``` and drive one start through the panel so `startTag` is reached at all — `onIpc("start", HOSTILE)` then `argvIs(takeJob("timew start").cmd, { "timew", "start", HOSTILE })`. That restores the cross-file pin the comment promises and kills M5, M12 and M12b together. ### 2. `panel.luau:709` — the new `result.stderr .. result.stdout` has no test `Timew.exec` is the only path from a failed `timew start` / `timew stop` to a toast, and its body is new in this PR. Nothing exercises it: ``` SURVIVED M6 Timew.exec: Parse.trim(result.stderr .. result.stdout) -> Parse.trim(result.stdout) KILLED M7 bar.luau:221, the identical drop -> test.luau:498 ``` Two reasons it survives. `test.luau:589` stubs the panel's `notifyError` as `function() end`, so the argument is discarded, and no panel job is ever landed with a non-zero `exitCode` — `test.luau:1338` lands `{ exitCode = 0, stdout = "" }`. This matters more than a coverage gap because I confirmed against timew 1.9.1 that a real failure puts its message on **stderr only**: ``` $ timew stop # nothing tracked rc=255 stdout=[] stderr=[The end of a date range must be after the start.] $ timew bogusverb rc=255 stdout=[] stderr=['bogusverb' is not a timew command. See 'timew help'.] ``` So if that concatenation ever regresses to `result.stdout`, every panel error toast goes blank and the suite stays green. The line is correct today; it just is not defended. Fix — capture the argument in the panel stub and land one failure, mirroring `test.luau:494-503`: ```suggestion notifyError = function(_, message) panelError = message end, ``` ## Advisory ### 3. `panel.luau:709`, `bar.luau:221` — a missing `timew` now toasts an empty string Patch-introduced, and the only user-visible behaviour change I found. On `main` the command went through a shell, so the shell's own diagnostic was folded into stdout by the `2>&1` that this PR deletes: ``` $ env -i PATH=/usr/bin:/bin /bin/sh -c "timew start 'focus' 2>&1" rc=127 stdout=[/bin/sh: line 1: timew: command not found] stderr=[] ``` With argv there is no shell to say that. U0's live probe of this host has `{"definitely-not-a-binary-xyz"}` returning `exit=127` with `stdout=""` **and** `stderr=""` (the host does `::_exit(127)` after a failed `execvp`, `process.cpp:410`), so `Parse.trim(result.stderr .. result.stdout)` is `""` and the toast body is empty. The timeout case (`exitCode 143`) was already blank on both sides, so that half is not a regression. `dependencies = ["timewarrior"]` covers the install-time case but not a PATH the shell does not share. A fixed fallback is two lines: ```suggestion if not ok then local text = Parse.trim(result.stderr .. result.stdout) noctalia.notifyError("timew", if text == "" then "timew failed (exit " .. result.exitCode .. ")" else text) end ``` `noctalia.commandExists("timew")` is also a real binding at this API if a preflight check reads better than a fallback string. ### 4. `test.luau:664-684` — three string needles left, and one of them hides a real gap `takeJob("seq 0 13")` (`:828`, `:1164`, `:1339`, `:1534`, `:1767`) finds the day-totals job by a substring of its script, so it matches whatever tag the job carries: ``` SURVIVED M11 fetchDays: Timew.dayTotals(DAYS, state.filter, ...) -> Timew.dayTotals(DAYS, nil, ...) ``` That mutation removes the behaviour the README documents ("scopes the chart and the totals to that tag") and the suite does not notice. `T.cmd.dayTotals(DAYS, "focus")` as a table needle pins it. Same class as finding 1; separate because this one is inherited rather than introduced. Worth noting while you are in `takeJob`: the table branch compares `cmdText(job.cmd) == cmdText(needle)` plus the element count, not element by element. `{"a","b c"}` and `{"a b","c"}` both join to `"a b c"` with count 2. `argvIs` already does the element-wise comparison — reusing it inside `takeJob` costs nothing and removes the case entirely. ### 5. `panel.luau:392-397` — "every script in this file is a constant string" is not quite true Two numbers are still interpolated: `(days or DAYS) - 1` at `:571` and `(walkMax or DAY_WALK_MAX)` / `(walkMax or ALL_WALK_MAX)` at `:597` and `:617`. The first is forced numeric by the subtraction; the two `walkMax` sites are raw concatenation, so a string argument would be spliced into the script verbatim. Both call sites pass module locals (`loadDay` at `:832`, `fetchAll` at `:847`), so nothing is reachable today — but the comment is the file's central safety claim and a future caller will read it as a guarantee. Either narrow the sentence to "no *user* data", or make it true: ```suggestion 'while [ "$i" -le "$n" ] && [ "$i" -le ' .. math.floor(tonumber(walkMax) or DAY_WALK_MAX) .. ' ]; do', ``` ### 6. `panel.luau:445-448` — `parseTotal` still returns prose (pre-existing, parity preserved) Recording it because this PR rewrote the function and added its fixtures. Real timew on an empty day prints prose on stdout, and the last field of it is a timestamp: ``` $ timew summary :day No filtered data found in the range 2026-09-01T00:00:00 - 2026-09-02T00:00:00. $ ... | awk 'NF{l=$NF} END{print l}' -> 2026-09-02T00:00:00. Timew.lastField(same input) -> 2026-09-02T00:00:00. ``` Both spellings agree, so this is not a regression — `main` showed the same string. But `parseRangeTotal` (`:521-523`) guards with `^%d+:%d%d:%d%d$` and `parseTotal` does not, and the new `SUMMARY` fixture at `test.luau:180` is the natural place to add the prose case now that the reduction is Luau. --- ## Verified Everything below I ran; nothing is quoted from the PR description. **Gates**, from a clean run in `.worktrees/u1-argv`: ``` $ nix shell nixpkgs#luau -c luau test.luau viewModel: 14 states, 1485 field comparisons ok EXIT=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings EXIT=0 ``` **Counts, derived not taken.** `shQuote` is 0 in `timew/panel.luau` and 0 in `timew/bar.luau`; the 2 hits in `test.luau` are both prose (`:439`, `:468`). No re-implementation under another name: grepping `gsub(.*'`, `string.format("%q"`, `\\"` across `timew/` and `test.luau` returns only `shown()` at `test.luau:276` (a `%q` formatter for assert messages) and `dumpValue` at `:927`. `tagArg` and `Timew.cmd.exec` survive only in comments — clean cutover. 10 `noctalia.runAsync` sites (8 panel, 2 bar); 5 reach a shell: `activeInterval` and `allIntervals` as strings, `bar.sync` as a string, `dayTotals` and `intervalsForDay` as `/bin/sh -c` argv. Matches the claim. **Injection is structural, by inspection of all five.** `activeInterval`, `allIntervals` and `bar.sync` take no arguments at all. `dayTotals` and `intervalsForDay` build their script from literals plus one numeric bound and pass data as `argv[5]`. No user-controlled value is concatenated into any script text. **`${1:+"$1"}` against the deleted `tagArg`**, driven through a real shell with a stub `timew` that dumps its own argv. 17 tags, plus the no-tag and unset cases, under **bash 5.3.15 as `/bin/sh`** and under **dash**. Identical argv in every case, including `*`, `~`, `$HOME`, a bare `-`, `-x`, `--all`, a single space, a tab, and the multiline hostile tag: ``` == <no tag / empty> == 1=<summary> 2=<D> 3=<-> 4=<E> argc=4 tag "deep work" 1=<summary> 2=<D> 3=<-> 4=<E> 5=<deep work> argc=5 tag "*" ... 5=<*> argc=5 (no glob expansion) tag "x'; rm -rf /; #" ... 5=<x'; rm -rf /; #> argc=5 hostile, with newline ... 5=<a b'c"d $HOME `id` ;rm -rf / #|& \ \ntail> argc=5 argdiff(/bin/sh) fail=0 argdiff(dash) fail=0 ``` `argc=4` for both nil and `""` is the "no tag adds no argument" rule, preserved exactly. **Parser parity, 26 corpus cases against GNU awk 5.4.1.** Old side is the real pipeline — `awk 'NF{l=$NF} END{print l}'` then `Parse.trim` for `lastField`, and `awk 'NR==3{w=length($1); next} NR>3 && NF{print substr($0,1,w)}'` then the old trim-and-drop loop for `parseTags`. Corpus includes real `timew tags` and `timew summary` output, the empty-range prose, empty input, blank-only, whitespace-only lines, trailing whitespace, CRLF, tabs, a missing trailing newline, `Learn C++`, a tag wider than the rule, a rule with a leading space, a blank line mid-list, and a header with no rows. ``` lastField diffs=2 parseTags diffs=0 cases=26 LASTFIELD DIFF formfeed old=[a\fb] new=[b] LASTFIELD DIFF vtab old=[a\vb] new=[b] ``` Only `\f` and `\v` mid-line diverge, because Lua's `%s` counts them and awk's default field splitting does not. timew emits neither. **CRLF does not diverge**, because `main` ran `Parse.trim` over awk's output and Lua's `%s` already ate the `\r`. **Real timew 1.9.1 shapes**, against a scratch `TIMEWARRIORDB`. `timew tags` is a blank line, a header, a rule whose first dash run is the Tag column width, then padded rows — which is what `parseTags` assumes. An empty database gives `No data found.` on stdout (one line, rc=0); `parseTags` returns `{}` for it, as does the awk. Every failure I could construct put its message on stderr with stdout empty; every success the reverse. I could not produce a timew failure that writes to both streams, so the `stderr .. stdout` ordering is not observable and is not a regression. **Argv element types.** Nothing non-string can reach an array: `startTag` runs `Parse.trim` first (`:887`), `setFilter` runs `Parse.trim` (`:902`), `Config.get("tag")` is stringified (`test.luau:439`), and `loadDay` returns early when `state.days[index]` is absent (`:830-834`), so `intervalsForDay` never sees a nil date. **Not checked.** I did not run the plugin inside a live noctalia shell — the host contract (`result.stderr` always a string, `execvp` PATH search, `::_exit(127)` on a missing binary, `timedOut` as a separate boolean) is U0's live probe plus its source citations, not my own. I did not re-verify the surviving `dayTotals` awk under mawk or busybox awk, since the PR does not touch it. I did not look at the panel on screen; every check here is the suite, the shell, and timew. I did not exercise the 15 s / 10 s / 20 s timeouts. ## Mutations Applied to a copy of the worktree, one at a time, suite re-run and reverted after each. | # | Mutation | Caught | What it proves | |---|---|---|---| | M1 | `dayTotals`: splice `tag` back into the script text (real concatenation, not a literal edit) | killed `test.luau:311` | the frozen `DAY_SCRIPT` sees it | | M1' | M1 again with **every** frozen-literal assert commented out (`311-317`, `321-330`, `350-354`) | killed `test.luau:359` | **the hostile-tag assert is load-bearing, not decorative** — it fires alone | | M2 | `intervalsForDay`: splice `date` back into the script text | killed `:321`, and `:359` after the same neutering | same, for the day walk | | M3 | `${1:+"$1"}` -> `"$1"` (an empty tag now adds an empty argument) | killed `:311` | the empty-tag rule is pinned, by the literal | | M4 | `tag or ""` -> `tag` (argv loses its 5th element) | killed `:283` | `argvIs`' `#argv == #expected` is real | | M5 | panel `onStop`/`onIpc` -> `run({"timew","stop","--dry"})` | **SURVIVED** | finding 1 | | M6 | `Timew.exec`: drop `result.stderr` from the toast | **SURVIVED** | finding 2 | | M7 | `bar.onClick`: drop `result.stderr` from the toast | killed `:498` | bar's half is defended | | M8 | `rangeTotal`: insert the tag unconditionally | killed `:283` | the empty-tag element rule holds | | M9 | `lastField`: first field of each line instead of the last | killed `:189` | the `SUMMARY` fixture is not decoration | | M10 | `parseTags`: read the width off row 2 instead of row 3 | killed `:215` | the generated `tagsOutput` fixture bites | | M11 | `fetchDays`: stop passing `state.filter` | **SURVIVED** | finding 4 | | M12 | `startTag`: drop the tag from the argv | **SURVIVED** | finding 1 — the site is unreached | | M12b | `startTag` -> `{"timew","start","--tag",tag}` | **SURVIVED** | finding 1 — "catches them drifting" is false | | M13 | `intervalsForDay`: `"sh"` -> `""`, shifting the date onto `$0` | killed `:311` | the `$0` placeholder is pinned | | M14 | `todayTotal`: `:day` -> `:week` | killed `:295` | argv elements are pinned individually | | M15 | `lastField`: drop the `if field` guard | killed `:190` | the whitespace-only-line assert can fail | | M16 | `loadDay`: pass `day.date .. " "` | killed `:684` | `dayJob`'s table needle pins the date | 
Review follow-up on #11. Three things the reviewer's mutants walked through.

The drift comment claimed to catch bar and panel diverging on
`timew start <tag>` and did not: it compared bar's argv against a literal
written in the test, so panel's `startTag` was asserted nowhere. bar's arrays
are kept in `START_TAGS`/`barStart` now and the panel's are driven against them
at the end of the file, off the queue positionally so neither side is compared
to a literal. Stop is pinned the same way.

`Timew.exec`'s toast had no test: the panel's `notifyError` stub discarded its
argument and no panel job ever landed non-zero. The stub captures, and the
failure path is driven for every result shape.

`stderr .. stdout` can be empty where `2>&1` never was -- execvp cannot answer
for a binary that is not there, so a missing timew is exit 127 with both streams
empty and the toast body was blank. `Parse.toast` (and bar's copy) falls back to
a body naming exit 127 by cause.

Every `takeJob` string needle is gone. They were substrings: `"timew stop"`
matched `timew stop --dry`, and `"seq 0 13"` matched the day-totals script
whatever tag it carried, so `fetchDays` dropping `state.filter` went unnoticed.
Needles are exact -- `argvIs` element by element for argv, whole-string for the
two shell scripts -- with an `ANY` sentinel for the one slot the day-walk
landings genuinely do not care about.

Also: `parseTotal`'s prose behaviour is now asserted rather than merely
inherited, and the "every script is a constant string" comment is narrowed to
the claim that is true (no *user* data; three numeric bounds are still spliced).
yuris commented 2026-09-01 12:44:10 +00:00
Author
Owner
Copy link

Addressed in 58a0b0b. All four blocking/advisory findings taken, plus 5 and 6. One
partial pushback on 5 and one place where I could not produce the evidence you would
want, both below.

The five mutants you reported as survivors survive on the reviewed tree and are killed
on this one. Re-run against git show HEAD~1 for all three files, so the "before"
column is the tree you actually reviewed:

                                        reviewed tree   58a0b0b
HEAD baseline                           SURVIVED (ok)   -
M5    `timew stop --dry`                SURVIVED        KILLED
M6    Timew.exec drops result.stderr    SURVIVED        KILLED
M11   fetchDays drops state.filter      SURVIVED        KILLED
M12   startTag drops the tag            SURVIVED        KILLED
M12b  startTag -> --tag <tag>           SURVIVED        KILLED

1 — the drift check the comment promised

You were right that the comment described a check the assert did not perform, and right
about the mechanism: a literal cannot drift, so it cannot catch the panel drifting.

Both mutating commands are pinned across the two entries now. bar's arrays are kept as
they are built (START_TAGS / barStart, test.luau:502) and the panel's are driven
against them at the end of the file, which is the first point panelJobs exists.

I did not take the takeJob("timew start") half of your suggestion, and the reason is
the finding itself. A needle spelling out {"timew","start",tag} is another literal —
it would be satisfied by bar and the panel drifting together, and a takeJob that
misses raises "no queued command matching", which is a failure with a message that
names the needle rather than the disagreement. So the panel's array is taken off the
queue positionally instead, #panelJobs == 1 and then argvIs(panelJobs[1].cmd, barStart[tag]). Neither side is compared to anything written in the test:

for _, tag in ipairs(START_TAGS) do
	panelJobs = {}
	panelEnv.onIpc("start", tag)
	assert(#panelJobs == 1, "the panel's start is one command, not " .. #panelJobs .. ", for " .. shown(tag))
	argvIs(panelJobs[1].cmd, barStart[tag], "panel's start is the argv bar sends, for " .. shown(tag))
end

That runs over all six tags including the hostile one. Stop is the same shape, with bar
driven to tracking first so its own argv is the comparand.

KILLED  M12   startTag drops the tag        -> panel's start is the argv bar sends, for "focus": 2 elements, expected 3
KILLED  M12b  startTag -> --tag <tag>       -> panel's start is the argv bar sends, for "focus": 4 elements, expected 3
KILLED  M5    stop -> {"timew","stop","--dry"}  -> no queued command matching timew stop

2 — Timew.exec's toast

Taken as written: the panel's notifyError stub captures into panelError, and the
failure path is driven for eight result shapes (stderr-only, stdout-only, both, blank,
127 blank, 127 with text, 143 blank) through onIpc("start", …), because run reports
through Timew.exec and refetches either way. Plus the negative: exit 0 toasts nothing,
or the loop above would pass on a panel that notified on everything.

KILLED  MT6  Timew.exec -> Parse.trim(result.stdout)     -> the panel toasts through Parse.toast on exit 1: ""
KILLED  MT7  Timew.exec toasts on success too            -> exit 0 is not a failure: "ok"

3 — the blank toast for a missing timew

Fallback string, not a commandExists preflight, and not both. The reasoning:

  • The bug is "the toast body is empty", not "timew is missing". Exit 127 is the cause
    you found, but a preflight only closes that one; any other silent failure still
    toasts nothing. The fallback closes the class at the point where the body is chosen.
  • A preflight is an extra host round-trip on every click, on a path that already has
    one, to answer a question the failing call answers for free.
  • commandExists cannot see the case it is supposed to guard: the point of your finding
    is a PATH the shell does not share, and a preflight reads the same PATH execvp
    will.

It is Parse.toast(result) in the panel and toast(result) in bar, line for line
identical apart from the trim being a field on one side and a local on the other:

function Parse.toast(result)
	local text = Parse.trim(result.stderr .. result.stdout)
	if text ~= "" then
		return text
	end
	if result.exitCode == 127 then
		return "timew is not on PATH"
	end
	return "timew failed (exit " .. tostring(result.exitCode) .. ")"
end

127 is named because it is the one a user can act on — "timew is not on PATH" is the
command not found line 2>&1 used to carry, said by the plugin instead of the shell.
Anything else silent carries its exit code, which is strictly better than the blank both
main and the reviewed tree produced for a timeout. Text on either stream still wins, so
nothing about the normal failure path changed.

Untranslated, deliberately: Parse reaches no host, which is what makes it assertable
without a stub, and the timew text it stands in for was never translated either.

Because it is pure and duplicated, it is pinned both ways — the two copies against each
other over every shape, and then against what they actually say, since two identical
copies of a regression agree just as well:

KILLED  MT1  panel toast drops result.stderr           -> the two toast copies agree on exit 1: "timew failed (exit 1)"
KILLED  MT2  panel toast returns "" for a blank trim   -> the two toast copies agree on exit 1: ""
KILLED  MT3  panel toast: 127 loses its name           -> the two toast copies agree on exit 127: "timew failed"
KILLED  MT4  bar's copy drops result.stderr            -> the two toast copies agree on exit 1: "timew: boom"
KILLED  MT5  bar onClick bypasses toast (your M7)      -> bar's onClick toasts through `toast`: ""

4 — the string needles

All of them, not just the one you named. takeJob takes an exact needle now: a table is
matched by argvIs element by element, a string — the two commands that really are
shell scripts — is matched whole. cmdText survives only to build the failure message,
which now dumps the queue.

Converted: "seq 0 13" ×5, "summary :day" ×2, "summary :all" ×2, "dom.active.start",
"timew stop", WALK, ANY_DAY. grep 'takeJob("' returns only prose.

Day totals go through daysJob() (T.cmd.dayTotals(nil, panel.state.filter)) where the
tag is incidental, and through the literal T.cmd.dayTotals(nil, "focus") at the site
that documents "the chart follows the tag". One conversion turned out to be landing the
wrong job: at :915 the needle used to take the oldest queued day-totals job, issued
under "focus" several filters earlier, not the one row 2 had just asked for.

ANY_DAY is T.cmd.intervalsForDay(ANY) — an ANY sentinel in the date slot only, so
the script, the sh that puts the date on $1 and the element count all stay pinned.
Three mutations to loadDay confirm it did not become "match anything":

KILLED  M11  fetchDays drops state.filter            -> no queued command matching /bin/sh -c for i in $(seq 0 13); …
KILLED  MA3  loadDay asks for the wrong date         -> no queued command matching …
KILLED  MA4  loadDay asks with walkMax 30, not the default
KILLED  MA5  loadDay walks every interval instead of the day
KILLED  MN1  fetchAll asks for a one-day walk        (WALK, exact string)
KILLED  MN2  fetchAll walks 60 ids, not ALL_WALK_MAX
KILLED  MN3  fetchActive asks for the day total      (activeInterval, exact string)
KILLED  MN4  todayTotal asks :all instead of :day
KILLED  MN5  fetchAll's range total drops the tag

Where I owe you a mutation and do not have one. You also noted the table branch
compared joined text plus element count, so {"a","b c"} and {"a b","c"} matched. It
now reuses argvIs, as you suggested. I could not produce a mutation that distinguishes
the two comparisons, and I think the class is unreachable rather than merely hard: every
table needle in the suite is built by the same Timew.cmd builder as the job it matches,
so a builder change moves needle and job together. Demonstrated by neutering the frozen
WALK_SCRIPT asserts and then breaking the builder:

SURVIVED  MA1'  intervalsForDay: "sh" -> "bash", every frozen walk assert neutered
SURVIVED  MA2'  intervalsForDay: drop the walkMax bound, same neutering

Both are caught immediately with those asserts left in place. So the division of labour
is: the frozen literals pin the builders, the needles pin the call sites, and neither
substitutes for the other — which is your finding 1 from the other direction. I took the
change because it costs nothing and removes a latent trap, not because I can show it
firing.

5 — "every script in this file is a constant string"

Narrowed rather than made true, taking the first of the two options you offered:

so no user data reaches the text of any script here. What is still spliced into a
script is a numeric bound -- days at Timew.cmd.dayTotals and walkMax at the two
walks -- each of them a module local reached through arithmetic or a literal, never an
argument a caller outside this file can supply.

I did not add the math.floor(tonumber(…)) coercion. tonumber(walkMax) or DAY_WALK_MAX
also changes behaviour in the case it is meant to guard: a non-numeric string would
silently become the default instead of failing, which is a worse answer than the
type error a caller deserves. No caller outside the file can reach these — loadDay and
fetchAll pass module locals — so the coercion would be defence against a caller that
cannot exist, and the comment is now accurate about which values are text. Verified by
grep: the only concatenations into script text are the three bounds at :596, :622
and :642.

6 — parseTotal returns prose

Taken. The prose case is asserted beside the SUMMARY fixture, with your exact input,
as behaviour rather than endorsement — including the reason it is harmless, which is that
Parse.clock reads it as 0 downstream:

local PROSE = "No filtered data found in the range 2026-09-01T00:00:00 - 2026-09-02T00:00:00.\n"
assert(T.parseTotal(PROSE) == "2026-09-02T00:00:00.", "prose comes back verbatim, exactly as the awk left it")
assert(T.lastField(PROSE) == "2026-09-02T00:00:00.", "and lastField is the half that does it")
assert(Parse.clock(T.parseTotal(PROSE)) == 0, "so the arithmetic downstream reads it as zero rather than erroring")

So a future change that "fixes" it by adding parseRangeTotal's guard now has to fail
this line first and say so:

KILLED  MP1  parseTotal guards prose like parseRangeTotal does  -> prose comes back verbatim, exactly as the awk left it

Mutations

20 mutations, 20 killed, 0 survivors. Each applied to a throwaway copy so the branch is
never edited; the three files' sha256 are byte-identical before and after the whole run,
and the suite is green in the branch afterwards. The two labelled ' are the neutered
runs above, reported as survivors on purpose.

# Mutation Result
MT1 panel Parse.toast drops result.stderr KILLED
MT2 panel Parse.toast returns "" instead of the fallback KILLED
MT3 panel Parse.toast: exit 127 loses its name KILLED
MT4 bar's toast copy drops result.stderr KILLED
MT5 bar onClick bypasses toast (your M7) KILLED
MT6 panel Timew.exec bypasses Parse.toast (your M6) KILLED
MT7 panel Timew.exec toasts on success too KILLED
M12 startTag drops the tag (your M12) KILLED
M12b startTag drifts to --tag <tag> (your M12b) KILLED
M5 panel stop becomes timew stop --dry (your M5) KILLED
M11 fetchDays stops passing state.filter (your M11) KILLED
MP1 parseTotal floors prose the way parseRangeTotal does KILLED
MA3 loadDay asks for the wrong date KILLED
MA4 loadDay asks with a non-default walkMax KILLED
MA5 loadDay walks every interval instead of the day KILLED
MN1 fetchAll asks for a one-day walk KILLED
MN2 fetchAll walks 60 ids instead of ALL_WALK_MAX KILLED
MN3 fetchActive asks for the day total KILLED
MN4 todayTotal asks :all instead of :day KILLED
MN5 fetchAll's range total drops the tag KILLED
MA1' intervalsForDay: "sh" -> "bash", frozen walk asserts neutered SURVIVED (expected)
MA2' intervalsForDay: drop the walkMax bound, same neutering SURVIVED (expected)

Gates

$ nix shell nixpkgs#luau -c luau test.luau
viewModel: 14 states, 1485 field comparisons
ok
SUITE_EXIT=0

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
LINT_EXIT=0

git status --porcelain is empty in the worktree.

Diff

50 insertions and 3 deletions in the two entries; the rest is the suite.

 test.luau        | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 timew/bar.luau   |  20 ++++--
 timew/panel.luau |  32 ++++++++--

Nothing about the argv conversion itself changed. The Parse.toast / toast pair is the
only behavioural difference from the reviewed tree, and it only ever fires where the
toast was previously blank.

Addressed in 58a0b0b. All four blocking/advisory findings taken, plus 5 and 6. One partial pushback on 5 and one place where I could not produce the evidence you would want, both below. The five mutants you reported as survivors survive on the reviewed tree and are killed on this one. Re-run against `git show HEAD~1` for all three files, so the "before" column is the tree you actually reviewed: ``` reviewed tree 58a0b0b HEAD baseline SURVIVED (ok) - M5 `timew stop --dry` SURVIVED KILLED M6 Timew.exec drops result.stderr SURVIVED KILLED M11 fetchDays drops state.filter SURVIVED KILLED M12 startTag drops the tag SURVIVED KILLED M12b startTag -> --tag <tag> SURVIVED KILLED ``` ## 1 — the drift check the comment promised You were right that the comment described a check the assert did not perform, and right about the mechanism: a literal cannot drift, so it cannot catch the panel drifting. Both mutating commands are pinned across the two entries now. bar's arrays are kept as they are built (`START_TAGS` / `barStart`, `test.luau:502`) and the panel's are driven against them at the end of the file, which is the first point `panelJobs` exists. I did not take the `takeJob("timew start")` half of your suggestion, and the reason is the finding itself. A needle spelling out `{"timew","start",tag}` is another literal — it would be satisfied by bar and the panel drifting *together*, and a `takeJob` that misses raises "no queued command matching", which is a failure with a message that names the needle rather than the disagreement. So the panel's array is taken off the queue positionally instead, `#panelJobs == 1` and then `argvIs(panelJobs[1].cmd, barStart[tag])`. Neither side is compared to anything written in the test: ```lua for _, tag in ipairs(START_TAGS) do panelJobs = {} panelEnv.onIpc("start", tag) assert(#panelJobs == 1, "the panel's start is one command, not " .. #panelJobs .. ", for " .. shown(tag)) argvIs(panelJobs[1].cmd, barStart[tag], "panel's start is the argv bar sends, for " .. shown(tag)) end ``` That runs over all six tags including the hostile one. Stop is the same shape, with bar driven to tracking first so its own argv is the comparand. ``` KILLED M12 startTag drops the tag -> panel's start is the argv bar sends, for "focus": 2 elements, expected 3 KILLED M12b startTag -> --tag <tag> -> panel's start is the argv bar sends, for "focus": 4 elements, expected 3 KILLED M5 stop -> {"timew","stop","--dry"} -> no queued command matching timew stop ``` ## 2 — `Timew.exec`'s toast Taken as written: the panel's `notifyError` stub captures into `panelError`, and the failure path is driven for eight result shapes (stderr-only, stdout-only, both, blank, 127 blank, 127 with text, 143 blank) through `onIpc("start", …)`, because `run` reports through `Timew.exec` and refetches either way. Plus the negative: exit 0 toasts nothing, or the loop above would pass on a panel that notified on everything. ``` KILLED MT6 Timew.exec -> Parse.trim(result.stdout) -> the panel toasts through Parse.toast on exit 1: "" KILLED MT7 Timew.exec toasts on success too -> exit 0 is not a failure: "ok" ``` ## 3 — the blank toast for a missing timew Fallback string, not a `commandExists` preflight, and not both. The reasoning: - The bug is "the toast body is empty", not "timew is missing". Exit 127 is the cause you found, but a preflight only closes that one; any other silent failure still toasts nothing. The fallback closes the class at the point where the body is chosen. - A preflight is an extra host round-trip on every click, on a path that already has one, to answer a question the failing call answers for free. - `commandExists` cannot see the case it is supposed to guard: the point of your finding is a PATH the shell does not share, and a preflight reads the same PATH `execvp` will. It is `Parse.toast(result)` in the panel and `toast(result)` in bar, line for line identical apart from the trim being a field on one side and a local on the other: ```lua function Parse.toast(result) local text = Parse.trim(result.stderr .. result.stdout) if text ~= "" then return text end if result.exitCode == 127 then return "timew is not on PATH" end return "timew failed (exit " .. tostring(result.exitCode) .. ")" end ``` 127 is named because it is the one a user can act on — "timew is not on PATH" is the `command not found` line `2>&1` used to carry, said by the plugin instead of the shell. Anything else silent carries its exit code, which is strictly better than the blank both `main` and the reviewed tree produced for a timeout. Text on either stream still wins, so nothing about the normal failure path changed. Untranslated, deliberately: `Parse` reaches no host, which is what makes it assertable without a stub, and the timew text it stands in for was never translated either. Because it is pure and duplicated, it is pinned both ways — the two copies against each other over every shape, and then against what they actually say, since two identical copies of a regression agree just as well: ``` KILLED MT1 panel toast drops result.stderr -> the two toast copies agree on exit 1: "timew failed (exit 1)" KILLED MT2 panel toast returns "" for a blank trim -> the two toast copies agree on exit 1: "" KILLED MT3 panel toast: 127 loses its name -> the two toast copies agree on exit 127: "timew failed" KILLED MT4 bar's copy drops result.stderr -> the two toast copies agree on exit 1: "timew: boom" KILLED MT5 bar onClick bypasses toast (your M7) -> bar's onClick toasts through `toast`: "" ``` ## 4 — the string needles All of them, not just the one you named. `takeJob` takes an exact needle now: a table is matched by `argvIs` element by element, a string — the two commands that really are shell scripts — is matched whole. `cmdText` survives only to build the failure message, which now dumps the queue. Converted: `"seq 0 13"` ×5, `"summary :day"` ×2, `"summary :all"` ×2, `"dom.active.start"`, `"timew stop"`, `WALK`, `ANY_DAY`. `grep 'takeJob("'` returns only prose. Day totals go through `daysJob()` (`T.cmd.dayTotals(nil, panel.state.filter)`) where the tag is incidental, and through the literal `T.cmd.dayTotals(nil, "focus")` at the site that documents "the chart follows the tag". One conversion turned out to be landing the wrong job: at `:915` the needle used to take the oldest queued day-totals job, issued under `"focus"` several filters earlier, not the one row 2 had just asked for. `ANY_DAY` is `T.cmd.intervalsForDay(ANY)` — an `ANY` sentinel in the date slot only, so the script, the `sh` that puts the date on `$1` and the element count all stay pinned. Three mutations to `loadDay` confirm it did not become "match anything": ``` KILLED M11 fetchDays drops state.filter -> no queued command matching /bin/sh -c for i in $(seq 0 13); … KILLED MA3 loadDay asks for the wrong date -> no queued command matching … KILLED MA4 loadDay asks with walkMax 30, not the default KILLED MA5 loadDay walks every interval instead of the day KILLED MN1 fetchAll asks for a one-day walk (WALK, exact string) KILLED MN2 fetchAll walks 60 ids, not ALL_WALK_MAX KILLED MN3 fetchActive asks for the day total (activeInterval, exact string) KILLED MN4 todayTotal asks :all instead of :day KILLED MN5 fetchAll's range total drops the tag ``` **Where I owe you a mutation and do not have one.** You also noted the table branch compared joined text plus element count, so `{"a","b c"}` and `{"a b","c"}` matched. It now reuses `argvIs`, as you suggested. I could not produce a mutation that distinguishes the two comparisons, and I think the class is unreachable rather than merely hard: every table needle in the suite is built by the same `Timew.cmd` builder as the job it matches, so a builder change moves needle and job together. Demonstrated by neutering the frozen `WALK_SCRIPT` asserts and then breaking the builder: ``` SURVIVED MA1' intervalsForDay: "sh" -> "bash", every frozen walk assert neutered SURVIVED MA2' intervalsForDay: drop the walkMax bound, same neutering ``` Both are caught immediately with those asserts left in place. So the division of labour is: the frozen literals pin the builders, the needles pin the call sites, and neither substitutes for the other — which is your finding 1 from the other direction. I took the change because it costs nothing and removes a latent trap, not because I can show it firing. ## 5 — "every script in this file is a constant string" Narrowed rather than made true, taking the first of the two options you offered: > so no user data reaches the text of any script here. What is still spliced into a > script is a numeric bound -- `days` at Timew.cmd.dayTotals and `walkMax` at the two > walks -- each of them a module local reached through arithmetic or a literal, never an > argument a caller outside this file can supply. I did not add the `math.floor(tonumber(…))` coercion. `tonumber(walkMax) or DAY_WALK_MAX` also changes behaviour in the case it is meant to guard: a non-numeric string would silently become the default instead of failing, which is a worse answer than the type error a caller deserves. No caller outside the file can reach these — `loadDay` and `fetchAll` pass module locals — so the coercion would be defence against a caller that cannot exist, and the comment is now accurate about which values are text. Verified by grep: the only concatenations into script text are the three bounds at `:596`, `:622` and `:642`. ## 6 — `parseTotal` returns prose Taken. The prose case is asserted beside the `SUMMARY` fixture, with your exact input, as behaviour rather than endorsement — including the reason it is harmless, which is that `Parse.clock` reads it as 0 downstream: ```lua local PROSE = "No filtered data found in the range 2026-09-01T00:00:00 - 2026-09-02T00:00:00.\n" assert(T.parseTotal(PROSE) == "2026-09-02T00:00:00.", "prose comes back verbatim, exactly as the awk left it") assert(T.lastField(PROSE) == "2026-09-02T00:00:00.", "and lastField is the half that does it") assert(Parse.clock(T.parseTotal(PROSE)) == 0, "so the arithmetic downstream reads it as zero rather than erroring") ``` So a future change that "fixes" it by adding `parseRangeTotal`'s guard now has to fail this line first and say so: ``` KILLED MP1 parseTotal guards prose like parseRangeTotal does -> prose comes back verbatim, exactly as the awk left it ``` ## Mutations 20 mutations, 20 killed, 0 survivors. Each applied to a throwaway copy so the branch is never edited; the three files' sha256 are byte-identical before and after the whole run, and the suite is green in the branch afterwards. The two labelled `'` are the neutered runs above, reported as survivors on purpose. | # | Mutation | Result | |---|---|---| | MT1 | panel `Parse.toast` drops `result.stderr` | KILLED | | MT2 | panel `Parse.toast` returns `""` instead of the fallback | KILLED | | MT3 | panel `Parse.toast`: exit 127 loses its name | KILLED | | MT4 | bar's `toast` copy drops `result.stderr` | KILLED | | MT5 | bar `onClick` bypasses `toast` (your M7) | KILLED | | MT6 | panel `Timew.exec` bypasses `Parse.toast` (your M6) | KILLED | | MT7 | panel `Timew.exec` toasts on success too | KILLED | | M12 | `startTag` drops the tag (your M12) | KILLED | | M12b | `startTag` drifts to `--tag <tag>` (your M12b) | KILLED | | M5 | panel stop becomes `timew stop --dry` (your M5) | KILLED | | M11 | `fetchDays` stops passing `state.filter` (your M11) | KILLED | | MP1 | `parseTotal` floors prose the way `parseRangeTotal` does | KILLED | | MA3 | `loadDay` asks for the wrong date | KILLED | | MA4 | `loadDay` asks with a non-default `walkMax` | KILLED | | MA5 | `loadDay` walks every interval instead of the day | KILLED | | MN1 | `fetchAll` asks for a one-day walk | KILLED | | MN2 | `fetchAll` walks 60 ids instead of `ALL_WALK_MAX` | KILLED | | MN3 | `fetchActive` asks for the day total | KILLED | | MN4 | `todayTotal` asks `:all` instead of `:day` | KILLED | | MN5 | `fetchAll`'s range total drops the tag | KILLED | | MA1' | `intervalsForDay`: `"sh"` -> `"bash"`, frozen walk asserts neutered | SURVIVED (expected) | | MA2' | `intervalsForDay`: drop the `walkMax` bound, same neutering | SURVIVED (expected) | ## Gates ``` $ nix shell nixpkgs#luau -c luau test.luau viewModel: 14 states, 1485 field comparisons ok SUITE_EXIT=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings LINT_EXIT=0 ``` `git status --porcelain` is empty in the worktree. ## Diff 50 insertions and 3 deletions in the two entries; the rest is the suite. ``` test.luau | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++----- timew/bar.luau | 20 ++++-- timew/panel.luau | 32 ++++++++-- ``` Nothing about the argv conversion itself changed. The `Parse.toast` / `toast` pair is the only behavioural difference from the reviewed tree, and it only ever fires where the toast was previously blank.
yuris merged commit 4deaaf2dbd into main 2026-09-01 12:44:18 +00:00
yuris deleted branch upgrade/u1-argv 2026-09-01 12:44:18 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yuris/noctalia-timew!11
No description provided.