From ea5971a1fa82354c17541252bccde6fa62dc1fff Mon Sep 17 00:00:00 2001 From: Julian Noble Date: Mon, 3 Aug 2026 17:30:19 +1000 Subject: [PATCH] goals: draft G-160 commandstack hygiene pass + G-161 tcltestrun multiline-banner parsing Two user-approved proposed goals from the 2026-08-03 commandstack characterisation arc (git 145e785c): - G-160 commandstack hygiene pass: fix the suite-pinned defects (tokenid stuck at 1 with duplicate-token dispatch bypass, unreachable get_IMPLEMENTOR builtin branch, debug arg validation, stray bare puts, Delete_stack live-rename recursion, packagepreference rename/renamer guard key) and land the ergonomics items (did_rename record key appended, -renamer placement errors, O(1) token dispatch, delegation helper, Rename_stack/get_stack parking retrievability, known_renamers defaults reconciliation). Acceptance keyed to the _GAP_ test flips; overlap survey surfaced only G-035 (sibling surface on packagepreference, related-noted). - G-161 tcltestrun failure parsing robust to multi-line test descriptions: the parser currently misses multi-line opening banners, misreads the closing banner as an opener and swallows all later events including the summary line; goal makes it tolerant so the src/tests/AGENTS.md single-line-description contract can stand down to style guidance. Overlap survey: no intersecting live goals. goals_lint: clean (84 active-index goals, 77 archived). Assisted-by: harness=claude; primary-model=claude-fable-5; api-location=anthropic.com --- GOALS.md | 8 ++ goals/G-160-commandstack-hygiene.md | 88 ++++++++++++++++++++++ goals/G-161-tcltestrun-multiline-banner.md | 46 +++++++++++ 3 files changed, 142 insertions(+) create mode 100644 goals/G-160-commandstack-hygiene.md create mode 100644 goals/G-161-tcltestrun-multiline-banner.md diff --git a/GOALS.md b/GOALS.md index c9306bb6..a55d3cf1 100644 --- a/GOALS.md +++ b/GOALS.md @@ -406,3 +406,11 @@ Detail: goals/G-158-punk86-kit-parity.md Scope: GOALS.md (goals-system v4 bump + maintenance/authoring rule updates); GOALS-archive.md (archive-edit exception wording); goals/AGENTS.md (Follow-on line grammar, sanctioned disposition edits, archive-sweep update); goals/archive/ (one-time retrofit sweep); scriptlib/developer/goals_lint.tcl (grammar validation); scriptlib/developer/goals_followons.tcl (new advisory report) Detail: goals/G-159-followon-disposition-tracking.md +### G-160 [proposed] commandstack hygiene pass: pinned-defect fixes and API ergonomics +Scope: src/modules/commandstack-999999.0a1.0.tm (token integrity, get_IMPLEMENTOR, debug validation, channel discipline, did_rename record key, -renamer placement, O(1) dispatch, Rename_stack/Delete_stack semantics, delegation helper, known_renamers defaults, PUNKARGS/help updates); src/modules/punk/packagepreference-999999.0a1.0.tm (install re-install guard record-key fix only); src/tests/modules/commandstack/testsuites/commandstack/commandstack.test (GAP flips + new pins); src/tests/modules/punk/packagepreference/testsuites/packagepreference/ (new double-install pin); src/vendormodules/packagetrace-*.tm + src/vendormodules/packagesuppress-*.tm (verification context only - not edited) +Detail: goals/G-160-commandstack-hygiene.md + +### G-161 [proposed] tcltestrun failure parsing robust to multi-line test descriptions +Scope: src/modules/punk/tcltestrun-999999.0a1.0.tm (parse_testrun failure-banner state machine); src/tests/runner/ (parser characterization suite - synthetic captured-output + end-to-end failing-fixture coverage); src/tests/AGENTS.md (single-line-description contract note relaxed to style guidance once tolerated) +Detail: goals/G-161-tcltestrun-multiline-banner.md + diff --git a/goals/G-160-commandstack-hygiene.md b/goals/G-160-commandstack-hygiene.md new file mode 100644 index 00000000..49537b17 --- /dev/null +++ b/goals/G-160-commandstack-hygiene.md @@ -0,0 +1,88 @@ +# G-160 commandstack hygiene pass: pinned-defect fixes and API ergonomics + +Status: proposed +Scope: src/modules/commandstack-999999.0a1.0.tm (token integrity, get_IMPLEMENTOR, debug validation, channel discipline, did_rename record key, -renamer placement, O(1) dispatch, Rename_stack/Delete_stack semantics, delegation helper, known_renamers defaults, PUNKARGS/help updates); src/modules/punk/packagepreference-999999.0a1.0.tm (install re-install guard record-key fix only); src/tests/modules/commandstack/testsuites/commandstack/commandstack.test (GAP flips + new pins); src/tests/modules/punk/packagepreference/testsuites/packagepreference/ (new double-install pin); src/vendormodules/packagetrace-*.tm + src/vendormodules/packagesuppress-*.tm (verification context only - not edited) +Goal: the defects pinned by the 2026-08-03 commandstack characterisation suite are fixed and the API's sharp edges removed, so a developer can stack, re-stack and unstack overrides without foot-guns: per-(renamer,command) tokenids are unique and monotonic (same-renamer re-renames dispatch through the full chain and are individually removable by token), get_IMPLEMENTOR actually classifies builtins where cmdtype exists, debug validates its argument, informational output is debug-gated on defined channels, the rename record carries an explicit did_rename verdict without breaking the lsearch key-order contract, a misplaced -renamer errors instead of silently misparsing, per-call delegation resolution no longer scans the stack list, parked stacks are retrievable, and the packagepreference double-install guard reads the record key that exists. +Acceptance: commandstack minted at >= 0.6.0 with all of: (1) the four _GAP_ tests in the commandstack suite flipped to fixed-behaviour pins - same-renamer re-rename gets a distinct tokenid, delegates to that renamer's previous override (not the original) and is removable by exact token; a third same-renamer rename succeeds; get_IMPLEMENTOR returns builtin for a native command where tcl::info::cmdtype exists (undetermined on 8.6); Delete_stack with live renames errors (or an otherwise-pinned safe behaviour) instead of recursing; (2) new pins proving - debug rejects a non-boolean argument with a clean error naming the argument; with debug off a full rename/re-rename/remove cycle emits nothing on stdout or stderr (puts-shim assertion); records carry a trailing did_rename 0|1 with token still the first and renamer still the second dict entry; rename_command errors when -renamer appears anywhere but the leading position; get_stack retrieves a Rename_stack-parked stack; Rename_stack no longer returns the whole stacks dict; a delegation helper (working name commandstack::next) is PUNKARGS-documented, named in help, and demonstrated delegating correctly by a pin; the known_renamers defaults are reconciled with the vendored packages' actual renamer strings (decision recorded in this file) and the defaults pin updated; (3) get_next_command resolves tokens via dict lookup rather than a stack-list scan, with the mechanism and a timing or source-level verification note recorded under Progress; (4) punk::packagepreference::install reads the renamer key and a new double-install test proves a second install returns 0 silently; (5) the suite is green on tclsh90 and punk86, with all pre-existing non-GAP pins unchanged - the consumer patterns (packagepreference, punk::nav::fs, punk auto_execok, packagetrace, packagesuppress) keep working against the fixed module. + +## Context + +The 2026-08-03 characterisation arc (commandstack 0.5.0 + first suite, git 145e785c / +fossil 9ed984f5) pinned four defects as _GAP_ tests and surfaced several ergonomic +hazards. The sharpest defect: rename_command increments renamer_command_tokens inside +its apply lambda, where the name is apply-local - the namespace variable never changes, +tokenid is always 1, so a same-renamer re-rename stacks a DUPLICATE token. Because +get_next_command takes the first lsearch match, the newer override then delegates +straight to the oldest record, silently bypassing every intermediate layer (including +other renamers' overrides), and a third same-renamer rename dies on a parked-name +collision. This breaks the reload-my-package-during-development case and makes the +documented exact-token removal of non-topmost same-renamer entries unusable. + +Secondary defects: the get_IMPLEMENTOR builtin branch is unreachable on every Tcl +version (unqualified `info commands tcl::info::cmdtype` pattern inside +commandstack::util - info commands pattern namespaces resolve relative-only, unlike +command invocation); debug validates the current variable rather than its argument; a +stray bare `puts stderr` prints the literal word stderr to stdout; Delete_stack under a +live rename leaves the override delegating to itself (recursion-limit error on next +call); punk::packagepreference::install's already-installed guard reads record key +`rename` where the key is `renamer`, so a second install throws instead of silently +returning (latent - kits install once). + +Ergonomic hazards from the same review: consumers must know that implementation eq "" +means no-rename (implicit contract); every consumer hand-writes +`uplevel 1 [list $COMMANDSTACKNEXT {*}$args]`; -renamer is recognised only as the +first argument and is otherwise silently consumed as the command name; get_next_command +does an lsearch on every call of the renamed command (hot for ::package - packagepreference +already hardcodes around it); Rename_stack leaks the whole stacks dict as its return +value and parked stacks are invisible to get_stack (namespace which resolution); +the known_renamers defaults (::packagetrace ::packageSuppress) do not match what the +vendored packages actually register (packagetrace / packagesuppress, plain words). + +## Approach + +- tokenid: compute the increment against the real namespace variable (in the proc frame + before the apply, or namespace-qualified access from inside it); tokens become unique + and monotonic per (renamer, command). +- get_IMPLEMENTOR: fully qualify ::tcl::info::cmdtype in both the guard and the call. +- Channel discipline: route the informational warnings and the code-diff dump through + the debug flag; fix the stray bare puts; errors stay errors. +- did_rename: APPEND the key - the record key ORDER (token first for lsearch -index 1, + renamer second for lsearch -index 3) is a load-bearing contract pinned by the suite. +- Delete_stack: refuse (error) while the stack has live renames, or an equivalent + guarded behaviour settled in the work; Rename_stack returns something deliberate. +- Delegation helper: settled in the work (helper resolving the caller's token, or an + injected per-override convenience); the COMMANDSTACKNEXT variables remain, as + consumers depend on them. +- packagepreference: two-character key fix plus the missing double-install test. +- Module stays dependency-free (no punk::args require; PUNKARGS docs stay inert/lazy). + +## Alternatives considered + +- Searching the stack from the top (lsearch last-match) to tolerate duplicate tokens - + rejected: unique tokens are the actual fix; last-match papers over removal semantics. +- Restructuring the record dict (explicit status field first / dict-of-dicts) - rejected + for this pass: breaks the lsearch key-order contract consumers and the suite pin rely + on; appended keys deliver the same information compatibly. + +## Notes + +- Related: G-035 (sibling surface on src/modules/punk/packagepreference-999999.0a1.0.tm - + that goal characterises mixed .tm/pkgIndex provision with packagepreference active and + fixes it only for outright bugs in the provision machinery; this goal's + packagepreference change is an outright bug in the commandstack-guard path, unrelated + to provision semantics, and lands here). +- Overlap survey 2026-08-03: goals_xref paths over the Scope surfaced only G-035 (via + packagepreference); no live goal touches commandstack itself or its test tree. +- Baseline: characterisation suite (29 tests) green on tclsh90 9.0.3 + punk86 8.6 at + commandstack 0.5.0 (git 145e785c); the GAP tests to flip are + commandstack_GAP_samerenamer_new_body_duplicate_token_bypass, + commandstack_GAP_third_samerenamer_rename_name_collision, + commandstack_GAP_rerename_after_other_renamer_bypasses_chain (subsumed by the tokenid + fix), commandstack_GAP_delete_stack_live_rename_recursion, + commandstack_GAP_get_implementor_builtin_branch_unreachable. +- Compatibility floors: packagesuppress does `package require commandstack 0.3` (minimum + bound - satisfied); bootsupport carries 0.3/0.4 snapshots and _vfscommon 0.4.x/0.5.0 + until the next promotion cycle picks up the fixed module. +- Version: minor bump (0.6.0) expected - additive keys and fixes; the Delete_stack guard + changes an unexported maintenance proc's behaviour (changelog-noted). Revisit as major + only if the settled semantics break an exported surface. diff --git a/goals/G-161-tcltestrun-multiline-banner.md b/goals/G-161-tcltestrun-multiline-banner.md new file mode 100644 index 00000000..a2f1c06e --- /dev/null +++ b/goals/G-161-tcltestrun-multiline-banner.md @@ -0,0 +1,46 @@ +# G-161 tcltestrun failure parsing robust to multi-line test descriptions + +Status: proposed +Scope: src/modules/punk/tcltestrun-999999.0a1.0.tm (parse_testrun failure-banner state machine); src/tests/runner/ (parser characterization suite - synthetic captured-output + end-to-end failing-fixture coverage); src/tests/AGENTS.md (single-line-description contract note relaxed to style guidance once tolerated) +Goal: a failing tcltest whose description contains embedded newlines is reported with full fidelity - failure event with name and description, result_was/result_expected, and every subsequent event including the summary line - instead of the parser missing the multi-line opening banner, misreading the closing banner as an opener, and wedging in failure-capture state so the file degrades to warn/missing-cleanupTests with no failure detail. +Acceptance: a committed runner suite covers both a synthetic captured-output fixture (multi-line-description failure followed by passes and a summary line, parsed to the correct event set) and an end-to-end fixture .test file whose deliberately failing multi-line-description test is reported by runtests.tcl as status=FAILED with actual/expected populated and a trusted summary (summaryline_detected=1, correct totals) in BOTH single-process and multi-process modes; single-line-description failure reports keep their existing shape, verified by scriptlib/developer/runtests_parity.tcl comparing -report json output of an identical full-suite run before and after the change; the src/tests/AGENTS.md single-line-description bullet is updated to style guidance reflecting the new tolerance. + +## Context + +Discovered 2026-08-03 while authoring the commandstack characterisation suite. tcltest +prints a failing test's opening banner as ONE puts of "==== FAILED" +and only trims the description's ENDS, so a description with embedded newlines makes the +banner span lines. punk::tcltestrun's per-line scanner recognises an opening banner only +as a line both starting "==== " and ending " FAILED": the multi-line opener never +matches, the later single-line closing banner "==== FAILED" is then treated as an +OPENER, and the parser waits forever for a "==== Contents of test case:" line that +already went past - swallowing every subsequent event, including the pass events of +later tests and the cleanupTests summary line. The file then reports +warn/missing-cleanupTests with observed passes stopping at the failure and no failure +detail at all. The trap is recorded as a hard contract in src/tests/AGENTS.md +(descriptions must be single content line); this goal makes the parser tolerant so the +contract can stand down to a style preference. + +## Approach + +Two candidate mechanisms, settled in the work: +- Banner buffering: a line starting "==== " that does not end " FAILED" opens a bounded + lookahead buffer joined until a line ending " FAILED" completes the banner (or the + buffer is abandoned as ordinary output). +- Start-event anchoring: the runner always passes -verbose start, so + "---- start" precedes every test; carrying the current test name makes the + closing-banner match exact and the opening banner recognisable by prefix alone. +Either way the existing state machine's confounding-output heuristics (emulated banner +lines inside test output) must keep their current behaviour for single-line cases. + +## Notes + +- Overlap survey 2026-08-03: goals_xref paths over + src/modules/punk/tcltestrun-999999.0a1.0.tm, src/tests/runner and + src/tests/runtests.tcl found no intersecting live goals. +- Reproduction record: the commandstack suite's pre-rewrite failure mode (24-of-29 + observed passes, no failure event, no summary) in both runner modes, 2026-08-03; + zipper.test's single-line-description failure reported correctly under the same + conditions - the discriminating experiment. +- punk::tcltestrun is also consumed by kit-baked copies; a mint (and later promotion) + carries the fix to kits per the normal channels.