Browse Source
Outcomes of the 2026-07-19 punkcheck module review: - G-094 [proposed]: single OO record lifecycle - installtrack/installevent becomes the sole implementation with an eager|deferred persistence policy, punkcheck::install rebuilt as an OO consumer in deferred mode, the zero-caller installfile_begin/finished/skipped trio retired to shims, and the o_record_list staleness fixed. The single save chokepoint is G-095's attachment point. - G-095 [proposed]: concurrent-writer safety - atomic temp+rename saves, advisory EXCL lockfile serializing whole installer events per punkcheck root, merge-on-flush for deferred mode, non-interactive-safe duplicate-INSTALLER recovery. SQLite recorded as rejected for now (TDL human readability/debugability overriding; three-part reconsideration bar). - G-027: acceptance reframed - .punkcheck records demoted from sole provenance basis to acceleration layer; safe-overwrite classification survives record loss via a committed baseline record (origin identity + pulled version, pristine content re-derivable from the origin's VCS), degrading conservatively (report/confirm - never silent overwrite, never a wedged pull). Approach classification bullet extended to match. - G-095 notes carry the tiered expendability taxonomy (build trees: recopies only; synced-targets bin/ deploy: non-self-healing skip wedge; derived projects: G-027 baseline) and the cross-tier invariant: absent records degrade to skip/confirm, never to overwrite. - (The G-087 cross-pointer to G-094/G-095 already landed with the stage-3 sources commit.) goals_lint clean (73 active-index goals, 22 archived). Claude-Session: https://claude.ai/code/session_01L7MfuYBfYpUFpVafo5JowC Assisted-by: harness=claude; primary-model=claude-fable-5; api-location=anthropic.commaster
4 changed files with 206 additions and 1 deletions
@ -0,0 +1,88 @@
|
||||
# G-094 punkcheck single record lifecycle: OO installtrack as sole implementation with a persistence policy |
||||
|
||||
Status: proposed |
||||
Scope: src/modules/punkcheck-999999.0a1.0.tm (installtrack/installevent lifecycle unification, installfile_* retirement, punkcheck::install as OO consumer), src/modules/punk/mix/cli-999999.0a1.0.tm + src/make.tcl (consumers verified unchanged), src/tests/modules/punkcheck/testsuites/punkcheck/ (characterization + new equivalence/staleness coverage) |
||||
Goal: The installtrack/installevent OO layer is the only implementation of the .punkcheck record lifecycle (targetset init / add-source / started / end), with an explicit per-event persistence policy - eager per-operation read-modify-write (current OO behaviour) and deferred event-scoped flush (current batch behaviour, QUERY's no-save being the in-object precedent) - punkcheck::install rebuilt as a tree-walking consumer of that layer in deferred mode, the zero-caller legacy pipeline (installfile_begin / installfile_finished_install / installfile_skipped_install) retired to error-with-pointer shims, and every .punkcheck save flowing through a single chokepoint. |
||||
Acceptance: characterization suites install.test and installtrack.test pass (pins flipped only where this goal's contract deliberately changes behaviour, each flip recorded); a new test pins field equivalence between batch-installed and OO-installed records (including -targets_cksums, which the legacy proc pipeline never stored); a new regression test covers o_record_list coherence - two sequential events on one installtrack instance, with targetset writes in the first event, lose nothing at the second start_event; grep of src/modules, src/make.tcl and src/project_layouts finds no live caller of the retired trio outside shims and tests; a full make.tcl build completes with skip/copy decisions unchanged against pre-refactor .punkcheck state (no spurious recopies of unchanged sources). |
||||
|
||||
## Context |
||||
|
||||
2026-07-19 module review found three parallel implementations of the record |
||||
lifecycle, with drift: |
||||
|
||||
- **Legacy proc pipeline** (installfile_begin/started/finished/skipped + |
||||
start_installer_event): frozen at an earlier design - INSTALL-only (no |
||||
MODIFY/DELETE/VIRTUAL/QUERY), no FAILED status, no -note, and |
||||
installfile_finished_install stores no post-install target cksums, so its |
||||
records can never serve `-overwrite synced-targets`. Zero live callers of |
||||
begin/finished/skipped anywhere (punk::mix::cli's calls are commented out |
||||
next to their OO replacements); they are kept alive only by installfile_help |
||||
and the characterization tests. |
||||
- **OO layer** (installtrack/installevent): targetset_started, |
||||
targetset_addsource and targetset_addsource_virtual delegate down to the |
||||
legacy procs, while targetset_init and targetset_end are parallel rewrites |
||||
of installfile_begin and finished/skipped with the extensions above - a |
||||
half-inversion of the desired layering (procs should be the facade, the OO |
||||
lifecycle the substrate). |
||||
- **Batch punkcheck::install**: bypasses both - uses start_installer_event and |
||||
the metadata fetcher but inlines its own begin and finalize so it can defer |
||||
all persistence to one end-of-run save, independently re-growing |
||||
-targets_cksums. Its in-memory recordset handling is the deferred |
||||
persistence mode this goal makes first-class. |
||||
|
||||
Latent single-process clobber: start_event saves the whole constructor-era |
||||
o_record_list, which targetset operations never refresh (they re-load from |
||||
file into locals) - a second start_event on one instance would discard every |
||||
FILEINFO update the first event's targetsets wrote. Unexercised-path evidence: |
||||
installtrack get_recordlist reads a misspelled variable ($o_recordlist) and |
||||
targetset_dict calls a nonexistent punk::records_as_target_dict - both would |
||||
error if ever called. |
||||
|
||||
Performance shape that motivates the policy knob rather than "just save |
||||
eagerly everywhere": eager OO costs three full parse+prettyprint rewrites of a |
||||
MB-scale file per installed target (modules/.punkcheck is ~1.9MB), which is |
||||
why the batch path defers; deferring must be a first-class mode of the one |
||||
lifecycle, not an ad-hoc bypass with its own record code. |
||||
|
||||
## Approach |
||||
|
||||
- Persistence policy selected per event (eager | deferred); in deferred mode |
||||
the installtrack's in-memory recordset is authoritative between flushes, so |
||||
the o_record_list coherence fix is a precondition, not a side quest. |
||||
- installfile_add_source_and_fetch_metadata stays as the shared pure engine - |
||||
it is the one correctly-layered piece (record transform + filesystem |
||||
metadata fetch, no saves), used by all three current paths. |
||||
- start_installer_event's role is subsumed by installtrack construction + |
||||
start_event; the surviving convenience surface for callers is |
||||
punkcheck::install / install_tm_files / install_non_tm_files as thin OO |
||||
consumers. |
||||
- Single save chokepoint: all writes to a .punkcheck file (event start, eager |
||||
targetset ops, deferred flush) route through one proc/method - this is the |
||||
attachment point G-095 locks onto. |
||||
|
||||
## Alternatives considered |
||||
|
||||
- Making the legacy procs true facades that construct transient event objects |
||||
while preserving their dict-passing contract - rejected: zero live callers |
||||
make the reconstitution ceremony pointless; error-with-pointer shims are |
||||
honest and cheaper. |
||||
- Locking/concurrency first, refactor later - rejected: the safety measures |
||||
would have to be implemented and kept in sync across three lifecycle |
||||
implementations; sequencing the unification first means G-095 lands once. |
||||
|
||||
## Notes |
||||
|
||||
- Prerequisite for G-095 (concurrent-writer safety) - that goal's lock, |
||||
atomic-rename and merge-on-flush measures assume this goal's single save |
||||
chokepoint. |
||||
- Version-skew during rollout: bootsupport punkcheck (0.4.0) and the dev |
||||
module can both write the same tree's .punkcheck files (make.tcl runs |
||||
bootsupport's copy; a dev shell runs the built one) - re-vendor bootsupport |
||||
promptly after landing, and sequence this refactor before any on-disk |
||||
protocol change from G-095. |
||||
- G-087 recorded a punkcheck MAX_PATH candidate (deep relative source chains |
||||
exceed Windows path limits, unnormalized) - natural rider on this goal's |
||||
load/save consolidation. |
||||
- The characterization suites were written expressly to lock behaviour "before |
||||
any refactoring" - this is that refactoring. |
||||
- punkcheck::cli (status/paths) is a read-only consumer and stays untouched. |
||||
@ -0,0 +1,96 @@
|
||||
# G-095 punkcheck concurrent-writer safety: atomic saves and advisory event-scoped locking |
||||
|
||||
Status: proposed |
||||
Scope: src/modules/punkcheck-999999.0a1.0.tm (save/load chokepoint: atomic rename, lockfile protocol, merge-on-flush, non-interactive recovery), src/make.tcl + src/modules/punk/mix/cli-999999.0a1.0.tm (event-scoped lock consumers as verified), src/tests/modules/punkcheck/testsuites/punkcheck/ (new concurrency suite) |
||||
Goal: Concurrent writers to the same .punkcheck file cannot corrupt it or silently erase each other's records, and readers never observe torn content - every save is write-temp-then-atomic-rename (bounded retry for Windows sharing violations); an advisory cross-platform lockfile (open {WRONLY CREAT EXCL}, contents naming holder pid/host/installer/timestamp, bounded-retry acquisition, age-based stale-break with warning) serializes whole installer events per punkcheck root; deferred-mode flushes merge own records (INSTALLER by -name, FILEINFO by -targets) into freshly-loaded file state instead of wholesale overwrite; and the duplicate-INSTALLER sanity recovery degrades to a clean actionable error instead of an askuser prompt when stdin is non-interactive. |
||||
Acceptance: a concurrency test drives two child tclsh installer processes against one .punkcheck folder - the file parses cleanly throughout (a polling load_records_from_file reader hits no TDL parse error), both installers' records are present afterwards, and no duplicate INSTALLER records exist; lock behaviour verified - the second writer observably waits, or times out with a message naming the holder from lockfile contents; stale-break verified against a planted orphan lockfile; suite passes on the project's Windows Tcl 8.6 + Tcl 9 test matrix (capability-gated where child-process facilities require it), with a unix run when an environment is available. |
||||
|
||||
## Context |
||||
|
||||
2026-07-19 risk review: every punkcheck writer is an unguarded |
||||
read-modify-write cycle against a shared file, and the batch/deferred path |
||||
holds the entire recordset in memory for the whole tree walk |
||||
(seconds-to-minutes of checksumming) before a truncate-in-place rewrite of a |
||||
MB-scale file. Failure modes ranked by operational pain: |
||||
|
||||
1. Duplicate INSTALLER/FILEINFO records from interleaved insert cycles - the |
||||
next run either aborts (records_as_target_dict errors in targetset_init) |
||||
or demands interactive confirmation to delete the file (installtrack's |
||||
duplicate-INSTALLER sanity check via punk::lib::askuser) - which blocks or |
||||
errors unattended `-confirm 0` builds. |
||||
2. Torn reads - save truncates in place, so a concurrent reader (another |
||||
installer's load, or punkcheck::cli status) can parse a half-written file |
||||
and abort. |
||||
3. Silent lost updates - mildest: .punkcheck is an expendable |
||||
skip-optimization cache, so losses mean redundant recopies next run. |
||||
|
||||
The lock's larger value: two processes racing one .punkcheck are also racing |
||||
`file copy` into the same target tree - serializing the installer event |
||||
serializes the real hazard, not just the metadata. |
||||
|
||||
Current avoidance is convention only: per-target-root file partitioning, |
||||
strictly sequential make.tcl stages that re-load at stage boundaries, and |
||||
single-user build habits. Nothing technical prevents overlap (two builds, or |
||||
user + agent). |
||||
|
||||
## Approach |
||||
|
||||
- Depends on G-094's single save chokepoint - lock, atomic rename and |
||||
merge-on-flush are implemented once there, inherited by every path. |
||||
- Lock scope: the whole installer event (start_event through final |
||||
flush/end; batch install = the whole depth-0 call), sibling lockfile |
||||
`.punkcheck.lock` in the punkcheck folder. |
||||
- Acquisition: `open ... {WRONLY CREAT EXCL}` (atomic create-new on Windows |
||||
and POSIX, Tcl 8.6 and 9, no extra packages); hold the channel open for the |
||||
duration (on Windows an open handle also resists deletion); on contention |
||||
retry with backoff to a timeout, then fail with a message quoting the |
||||
holder identity from the lockfile. |
||||
- Stale locks: age-based break with warning as the portable baseline; |
||||
optional liveness hardening behind capability checks (twapi process_exists |
||||
on Windows, kill -0 probe on unix). |
||||
- Optimistic tripwire independent of the lock: capture a file signature |
||||
(mtime/size) at load, verify at flush; mismatch means a writer bypassed the |
||||
protocol (e.g. an older vendored punkcheck during the transition window) - |
||||
reload-and-merge or warn loudly rather than silently overwrite. |
||||
|
||||
## Alternatives considered |
||||
|
||||
- SQLite storage - rejected for now (user decision 2026-07-19): the TDL text |
||||
format's easy human readability/debugability is an overriding factor. |
||||
Reconsideration requires all three: performance becomes an actual problem; |
||||
sqlite is determined to provide an overwhelming advantage in that |
||||
situation; and good debugging/viewing tools are built alongside the |
||||
migration. (sqlite3 is also only conditionally available across the target |
||||
interps.) |
||||
- mkdir-as-mutex - not implementable in Tcl: `file mkdir` succeeds silently |
||||
on an existing directory, so there is no atomic test-and-set. |
||||
- TclX flock / OS byte-range locks / twapi-exclusive CreateFile - not |
||||
cross-platform across the 8.6+9 win/unix envelope; mandatory locking would |
||||
also punish read-only consumers (punkcheck::cli status). |
||||
|
||||
## Notes |
||||
|
||||
- Expendability is tiered, not absolute (2026-07-19 review): in build trees |
||||
(modules/, lib/) deletion costs only redundant recopies - overwrite modes |
||||
there are source-driven and targets are clobber-by-design; for |
||||
synced-targets consumers (the bin/ deploy) deletion wedges existing targets |
||||
into skip-until-manual-action - fail-safe but stuck, and not self-healing, |
||||
since skipped records never store target cksums; in derived projects the |
||||
G-027 committed-baseline direction keeps safe-overwrite classification |
||||
working after record loss. The invariant to preserve across all tiers: |
||||
absent records degrade to skip/report-confirm, never to overwrite. Within |
||||
those consequence bounds, deletion stays the documented last-resort |
||||
recovery; locking does not make the file precious. |
||||
- The installtrack constructor's deletion prompt ("It is safe to delete |
||||
.punkcheck files...") oversells for synced-targets consumers - add the |
||||
wedge caveat to that message when it is touched under G-094/G-095. |
||||
- Advisory protocol is only effective once all writers carry it - re-vendor |
||||
bootsupport immediately after landing; the signature tripwire covers the |
||||
transition window. |
||||
- Readers (punkcheck::cli status/paths) gain torn-read immunity from atomic |
||||
renames without any reader-side change. |
||||
- Related: G-028 (Restart Manager locker naming for OS-mandatory locks on |
||||
deploy targets) - same "name the blocker" diagnostic principle; this goal's |
||||
timeout message naming the lock holder is the advisory-lock sibling. |
||||
- Related: G-027 - a derived-project pull writing .punkcheck files while |
||||
builds run in that project is a future instance of exactly this scenario. |
||||
Loading…
Reference in new issue