jbr870/devwork-skills
1
0
Fork 0
Code Issues 64 Pull requests Projects Releases Packages Wiki Activity Actions

local-fs comment integrity — portable mtime pinning + atomic post_comment #26

New issue
Closed
opened 2026-08-13 07:34:30 +00:00 by jbr870 · 30 comments
jbr870 commented 2026-08-13 07:34:30 +00:00
Owner
Copy link

PREQ: local-fs comment integrity — portable mtime pinning + atomic post_comment

Created: 2026-08-13

Problem

Projects using the local-fs forge adapter cannot trust the integrity of their comment record.
The adapter pins each comment file's mtime to a millisecond timestamp — that pin is what makes
created_at meaningful and what makes a later edit detectable (mtime > created_ms). The pin
fails two coupled ways:

  1. Portability: on a box whose coreutils are uutils (Rust rewrite), date +%s%3N ignores the
    %3N width and emits nine nanosecond digits; the pin then dies (touch: Unable to parse date).
    All four forge flow tests fail at pristine HEAD on such a box. Worse, the readiness preflight
    passes there — its probe checks charset/length loosely, so the guard declares a box ready on
    which the adapter cannot pin a single mtime, converting one up-front message into mid-phase
    failures. (Observed: finding F-PO-21-3-1 during the #21 run; uutils coreutils 0.8.0.)

  2. Atomicity: posting a comment is several steps — pick the next number, write the file, pin
    the mtime. A failure or crash between the steps leaves a visible partial comment: a record
    with a malformed created_ms that consumers ordering on created_ms cannot read, and that can
    raise a false edit signal on a record nobody ever edited. (Observed: an orphaned partial
    test-plan:v1 in the axana dogfood run — whose trigger was itself an mtime-pin failure, i.e.
    failure mode 1 feeding failure mode 2.)

Users:

  • Primary: operators running the SDLC suite on a local-fs project (no remote forge) — their
    issue record is these files.
  • Secondary: suite maintainers — local-fs is the adapter the forge contract is developed
    against, so a corrupt record here poisons contract development and dogfood runs.

Current state: on GNU boxes everything works; on uutils boxes runs die mid-phase after a
green readiness check; any mid-write failure leaves corrupt records that must be found and
removed by hand.

Proposed Solution

Make local-fs comment writes crash-safe and portable-or-loudly-unsupported:

  1. Harden the millisecond-timestamp construction so a toolchain that emits extra fractional
    precision (uutils-style) still yields a correct millisecond pin.
  2. Make the local-fs readiness probe verify capability by doing: it performs a real
    scratch-file mtime pin and reads it back — the same operation the adapter depends on — rather
    than shape-checking tool output. A box that cannot perform the pin fails preflight loudly,
    never mid-run; a probe that cannot execute at all (e.g. no writable temp location) reports
    not-ready, never ready.
  3. Give comment posting a single commit point: a comment becomes visible only after it is fully
    written and pinned; an interruption at any earlier step leaves no comment visible through the
    adapter's read path, and never silently overwrites an existing comment. Prove it with a crash
    test that injects faults at each step boundary.

Scope: Standard — both fix directions from #23 plus the atomicity work from #13, because the
two defects are one mechanism (the pin) and its blast radius (the partial write). Reaffirmed at
review: the observed corruption was pin-triggered, but any mid-write failure corrupts the record
the same way, and local-fs is the adapter the contract is developed against — fix the class.

User Stories

  • As an operator on a box with non-GNU coreutils, I want the suite to either work correctly or
    tell me at readiness time that it cannot, so that I never lose a run to mid-phase pin failures
    after a green check.
  • As an operator, I want a posted comment to either exist completely or not at all, so that my
    issue record never contains partial, unreadable entries.
  • As a consumer of the record (any skill ordering on created_ms or checking edit state), I want
    every visible comment correctly pinned, so that edit detection never fires on a record nobody
    edited.

Acceptance Criteria

Where an AC says a comment is or is not "visible", visibility means through the adapter's
comment-read path (the forge contract's read operations). Temp/staging residue at the raw
filesystem level is permitted, provided it never affects subsequent operations (see the
clean-retry criterion).

  • Given a box whose date emits more than millisecond fractional precision for %s%3N
    (uutils-style), when a comment is posted via local-fs, then the comment is created with a
    correct millisecond pin (created_at readable, edit detection clean) and the forge flow
    tests pass at pristine HEAD on that box. Tolerance rule: any tool output carrying
    at-least-millisecond information normalizes to a correct pin; output carrying less
    fails preflight per the readiness criterion below.
  • Given a box whose tools cannot perform millisecond mtime pinning at all, when the readiness
    guard runs with forge.adapter: local-fs declared, then the guard reports the box not
    ready    , naming the failing tool with its observed vs. expected behaviour — it never reports
    ready on a box where the adapter cannot pin. If the probe itself cannot execute (e.g. no
    writable temp location), the guard likewise reports not-ready — never ready.
  • Given the local-fs readiness probe runs, then it verifies capability by performing a real
    scratch-file mtime pin and reading it back — the same operation the adapter performs — so a
    probe pass demonstrates the pin operation itself succeeding on that box.
  • Given comment posting is interrupted at any step boundary before its commit point
    (fault-injected at minimum: after number allocation, after a partial content write, before
    the pin), when the issue's comments are subsequently read, then no partial comment is
    visible — the record shows either the complete, pinned comment or nothing.
  • Given a prior posting attempt was interrupted, when the next comment is posted on the same
    issue, then it succeeds cleanly — no residue-induced numbering anomaly, no blocked or
    corrupted post.
  • Given a comment already exists, when any posting attempt (concurrent or retried) would land
    on the same comment identity, then the existing comment is never silently overwritten —
    concurrent posts either land as distinct comments or the loser fails loudly.
  • Given a posting attempt fails at or before its commit point (e.g. permission denied, disk
    full), when the failure occurs, then the caller receives an explicit error — it never
    proceeds believing the comment exists.
  • Given a comment that was posted successfully and never edited, when its edit state is
    checked, then it reports unedited (no false edit signal).
  • Given a GNU-coreutils box, when the forge flow tests run at pristine HEAD, then they still
    pass (no regression from the hardening).

Out of Scope

  • Supporting boxes that genuinely cannot pin millisecond mtimes: capable date/touch remains a
    declared local-fs prerequisite, where capability is defined behaviorally — passing the
    readiness probe's real pin-and-read-back — not by provenance ("GNU" vs otherwise). uutils
    counts as capable once the hardening lands; a toolchain that cannot express or apply
    millisecond mtimes does not, and the fix for such a box is honest detection, not dropping the
    prerequisite.
  • Detecting or cleaning up pre-existing partial comments (e.g. the axana orphan): one-off
    manual cleanup; this feature prevents new corruption, it does not remediate old records.
  • Changing the forge contract's created_ms / edit-detection semantics — the mechanism stays;
    only its reliability changes.
  • Other adapters (tea-cli, glab-cli, gh-cli) — they do not pin mtimes.
  • Atomicity for local-fs write paths other than comment posting (issue creation, body edits) —
    known follow-up if the same pattern is observed there; this feature fixes the path with the
    observed corruption.
  • Full lock-based serialization of concurrent writers — the guarantee is no-silent-overwrite
    (loud failure is acceptable for the losing writer), not universal concurrent success.

Dependencies

  • None on other features. No external systems of record — the adapter's substrate is the local
    filesystem itself.
  • Must stay within the helper-tier portability baseline (bash ≥ 3.2, jq, git; probe-verified
    date/touch capability for local-fs only) — the fix must not add new tool prerequisites.
  • The existing forge flow tests (smoke.sh, disposition-flow.sh, promotion-flow.sh,
    release-flow.sh) are the regression harness; the crash test joins them. The uutils behaviour
    is reproducible in the harness via a PATH shim that emulates date +%s%3N emitting full
    nanosecond digits — no real uutils box required for regression coverage.

Timeline

  • Created: 2026-08-13

Notes

  • Fan-in consolidation of #23 (bug, spawned from F-PO-21-3-1 on #21) and #13 (deferred backlog,
    dogfood retrospective 2026-08-07 F5). #23 is the trigger; #13 is the blast radius — #13's
    observed failure was an mtime-pin failure.
  • Reviewed Tier 3 (fan-in): all five lenses; every concern answered interactively by the operator
    (probe-by-doing, atomicity retained, no-silent-overwrite, pre-existing partials out of scope,
    contract-level visibility + clean-retry, loud post failure, probe-error = not ready, behavioral
    capability definition, AC1/AC2 precision).
  • First comment on a fresh issue (no comment store yet) is exercised by the existing flow tests;
    no separate criterion.
# PREQ: local-fs comment integrity — portable mtime pinning + atomic post_comment **Created:** 2026-08-13 ## Problem Projects using the `local-fs` forge adapter cannot trust the integrity of their comment record. The adapter pins each comment file's mtime to a millisecond timestamp — that pin is what makes `created_at` meaningful and what makes a later *edit* detectable (`mtime > created_ms`). The pin fails two coupled ways: 1. **Portability:** on a box whose coreutils are uutils (Rust rewrite), `date +%s%3N` ignores the `%3N` width and emits nine nanosecond digits; the pin then dies (`touch: Unable to parse date`). All four forge flow tests fail at pristine HEAD on such a box. Worse, the readiness preflight *passes* there — its probe checks charset/length loosely, so the guard declares a box ready on which the adapter cannot pin a single mtime, converting one up-front message into mid-phase failures. (Observed: finding `F-PO-21-3-1` during the #21 run; uutils coreutils 0.8.0.) 2. **Atomicity:** posting a comment is several steps — pick the next number, write the file, pin the mtime. A failure or crash between the steps leaves a *visible partial comment*: a record with a malformed `created_ms` that consumers ordering on `created_ms` cannot read, and that can raise a **false edit signal** on a record nobody ever edited. (Observed: an orphaned partial `test-plan:v1` in the axana dogfood run — whose trigger was itself an mtime-pin failure, i.e. failure mode 1 feeding failure mode 2.) **Users:** - **Primary:** operators running the SDLC suite on a `local-fs` project (no remote forge) — their issue record *is* these files. - **Secondary:** suite maintainers — `local-fs` is the adapter the forge contract is developed against, so a corrupt record here poisons contract development and dogfood runs. **Current state:** on GNU boxes everything works; on uutils boxes runs die mid-phase after a green readiness check; any mid-write failure leaves corrupt records that must be found and removed by hand. ## Proposed Solution Make `local-fs` comment writes **crash-safe and portable-or-loudly-unsupported**: 1. Harden the millisecond-timestamp construction so a toolchain that emits extra fractional precision (uutils-style) still yields a correct millisecond pin. 2. Make the `local-fs` readiness probe verify capability **by doing**: it performs a real scratch-file mtime pin and reads it back — the same operation the adapter depends on — rather than shape-checking tool output. A box that cannot perform the pin fails preflight loudly, never mid-run; a probe that cannot execute at all (e.g. no writable temp location) reports not-ready, never ready. 3. Give comment posting a single commit point: a comment becomes visible only after it is fully written *and* pinned; an interruption at any earlier step leaves no comment visible through the adapter's read path, and never silently overwrites an existing comment. Prove it with a crash test that injects faults at each step boundary. **Scope:** Standard — both fix directions from #23 plus the atomicity work from #13, because the two defects are one mechanism (the pin) and its blast radius (the partial write). Reaffirmed at review: the observed corruption was pin-triggered, but any mid-write failure corrupts the record the same way, and `local-fs` is the adapter the contract is developed against — fix the class. ## User Stories - As an operator on a box with non-GNU coreutils, I want the suite to either work correctly or tell me at readiness time that it cannot, so that I never lose a run to mid-phase pin failures after a green check. - As an operator, I want a posted comment to either exist completely or not at all, so that my issue record never contains partial, unreadable entries. - As a consumer of the record (any skill ordering on `created_ms` or checking edit state), I want every visible comment correctly pinned, so that edit detection never fires on a record nobody edited. ## Acceptance Criteria Where an AC says a comment is or is not "visible", visibility means **through the adapter's comment-read path** (the forge contract's read operations). Temp/staging residue at the raw filesystem level is permitted, provided it never affects subsequent operations (see the clean-retry criterion). - [ ] Given a box whose `date` emits more than millisecond fractional precision for `%s%3N` (uutils-style), when a comment is posted via `local-fs`, then the comment is created with a correct millisecond pin (`created_at` readable, edit detection clean) and the forge flow tests pass at pristine HEAD on that box. Tolerance rule: any tool output carrying **at-least-millisecond** information normalizes to a correct pin; output carrying less fails preflight per the readiness criterion below. - [ ] Given a box whose tools cannot perform millisecond mtime pinning at all, when the readiness guard runs with `forge.adapter: local-fs` declared, then the guard reports the box **not ready**, naming the failing tool with its observed vs. expected behaviour — it never reports ready on a box where the adapter cannot pin. If the probe itself cannot execute (e.g. no writable temp location), the guard likewise reports not-ready — never ready. - [ ] Given the `local-fs` readiness probe runs, then it verifies capability by performing a real scratch-file mtime pin and reading it back — the same operation the adapter performs — so a probe pass demonstrates the pin operation itself succeeding on that box. - [ ] Given comment posting is interrupted at any step boundary before its commit point (fault-injected at minimum: after number allocation, after a partial content write, before the pin), when the issue's comments are subsequently read, then no partial comment is visible — the record shows either the complete, pinned comment or nothing. - [ ] Given a prior posting attempt was interrupted, when the next comment is posted on the same issue, then it succeeds cleanly — no residue-induced numbering anomaly, no blocked or corrupted post. - [ ] Given a comment already exists, when any posting attempt (concurrent or retried) would land on the same comment identity, then the existing comment is never silently overwritten — concurrent posts either land as distinct comments or the loser fails loudly. - [ ] Given a posting attempt fails at or before its commit point (e.g. permission denied, disk full), when the failure occurs, then the caller receives an explicit error — it never proceeds believing the comment exists. - [ ] Given a comment that was posted successfully and never edited, when its edit state is checked, then it reports unedited (no false edit signal). - [ ] Given a GNU-coreutils box, when the forge flow tests run at pristine HEAD, then they still pass (no regression from the hardening). ## Out of Scope - Supporting boxes that genuinely cannot pin millisecond mtimes: capable `date`/`touch` remains a declared `local-fs` prerequisite, where **capability is defined behaviorally** — passing the readiness probe's real pin-and-read-back — not by provenance ("GNU" vs otherwise). uutils counts as capable once the hardening lands; a toolchain that cannot express or apply millisecond mtimes does not, and the fix for such a box is honest detection, not dropping the prerequisite. - Detecting or cleaning up **pre-existing** partial comments (e.g. the axana orphan): one-off manual cleanup; this feature prevents new corruption, it does not remediate old records. - Changing the forge contract's `created_ms` / edit-detection semantics — the mechanism stays; only its reliability changes. - Other adapters (`tea-cli`, `glab-cli`, `gh-cli`) — they do not pin mtimes. - Atomicity for `local-fs` write paths other than comment posting (issue creation, body edits) — known follow-up if the same pattern is observed there; this feature fixes the path with the observed corruption. - Full lock-based serialization of concurrent writers — the guarantee is no-silent-overwrite (loud failure is acceptable for the losing writer), not universal concurrent success. ## Dependencies - None on other features. No external systems of record — the adapter's substrate is the local filesystem itself. - Must stay within the helper-tier portability baseline (bash ≥ 3.2, `jq`, `git`; probe-verified `date`/`touch` capability for `local-fs` only) — the fix must not add new tool prerequisites. - The existing forge flow tests (`smoke.sh`, `disposition-flow.sh`, `promotion-flow.sh`, `release-flow.sh`) are the regression harness; the crash test joins them. The uutils behaviour is reproducible in the harness via a PATH shim that emulates `date +%s%3N` emitting full nanosecond digits — no real uutils box required for regression coverage. ## Timeline - Created: 2026-08-13 ## Notes - Fan-in consolidation of #23 (bug, spawned from `F-PO-21-3-1` on #21) and #13 (deferred backlog, dogfood retrospective 2026-08-07 F5). #23 is the trigger; #13 is the blast radius — #13's observed failure *was* an mtime-pin failure. - Reviewed Tier 3 (fan-in): all five lenses; every concern answered interactively by the operator (probe-by-doing, atomicity retained, no-silent-overwrite, pre-existing partials out of scope, contract-level visibility + clean-retry, loud post failure, probe-error = not ready, behavioral capability definition, AC1/AC2 precision). - First comment on a fresh issue (no comment store yet) is exercised by the existing flow tests; no separate criterion.
jbr870 commented 2026-08-13 07:35:01 +00:00
Author
Owner
Copy link

Linked: this issue is parent #23 (recorded by the devwork pipeline).

Linked: this issue is **parent** #23 (recorded by the devwork pipeline).
jbr870 commented 2026-08-13 07:35:14 +00:00
Author
Owner
Copy link

Linked: this issue is parent #13 (recorded by the devwork pipeline).

Linked: this issue is **parent** #13 (recorded by the devwork pipeline).
jbr870 commented 2026-08-13 07:46:28 +00:00
Author
Owner
Copy link

Test Plan: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment

Prerequisites

[State the scenarios need — environment specifics (paths, shim contents) are verified at
execution time, not guessed here.]

  • A scratch project workspace declaring forge.adapter: local-fs, with at least one issue
    that already carries at least one comment (needed for the no-overwrite scenario).
  • A box (or environment) with standard GNU coreutils — the regression baseline.
  • A way to present the suite with an altered toolchain (e.g. a PATH shim), able to emulate:
    (a) a date that ignores fractional-width specifiers and emits full nanosecond digits
    (uutils-style); (b) tools that cannot express or apply millisecond mtimes at all;
    (c) a toolchain whose output looks well-formed but whose mtime pin does not actually
    take effect.
  • A way to interrupt or fail a comment-posting operation partway through (fault injection at
    step boundaries), and a way to induce a write failure (e.g. an unwritable target).

Required Test Data

  • One issue with ≥1 existing, correctly pinned comment (Scenario 6).
  • One fresh issue with no comments yet (Scenario 4/5 starting state).

Test Scenarios

Scenario 1: Comment posting succeeds on a uutils-style toolchain

Acceptance criterion: "Given a box whose date emits more than millisecond fractional
precision for %s%3N (uutils-style), when a comment is posted via local-fs, then the comment
is created with a correct millisecond pin … and the forge flow tests pass at pristine HEAD on
that box."

  1. Arrange the toolchain so date emits full nanosecond digits where millisecond precision was
    requested (uutils emulation).
  2. Post a comment on an issue through the adapter.
  3. Verify: the comment is visible through the adapter's read path, with a readable creation
    timestamp of millisecond precision.
  4. Verify: the comment's edit state reports unedited.
  5. Run the four forge flow tests (smoke, disposition-flow, promotion-flow, release-flow)
    at pristine HEAD in this environment.
  6. Verify: all four pass.

Expected outcome: the quirky-but-capable toolchain is tolerated — posting works, timestamps
are correct, nothing downstream fails.

Scenario 2: Incapable box fails readiness loudly, naming the tool

Acceptance criterion: "Given a box whose tools cannot perform millisecond mtime pinning at
all, when the readiness guard runs with forge.adapter: local-fs declared, then the guard
reports the box not ready, naming the failing tool with its observed vs. expected behaviour…"

  1. Arrange the toolchain so millisecond mtime pinning is impossible (e.g. no fractional-second
    support at all).
  2. Run the readiness guard on a project declaring forge.adapter: local-fs.
  3. Verify: the guard reports not ready — never ready.
  4. Verify: the report names the failing tool and shows what was observed versus what was
    expected.

Expected outcome: one loud up-front message instead of mid-phase failures.

Scenario 2b (edge): Probe unable to execute also reports not-ready

Acceptance criterion: same criterion, probe-error clause: "If the probe itself cannot
execute (e.g. no writable temp location), the guard likewise reports not-ready — never ready."

  1. Arrange the environment so the probe cannot perform its check at all (e.g. no writable
    location for a scratch file).
  2. Run the readiness guard with forge.adapter: local-fs declared.
  3. Verify: the guard reports not-ready (or an explicit error) — it does not report ready.

Expected outcome: an inconclusive probe is never treated as a pass.

Scenario 3: Probe verifies by doing — output shape alone cannot fool it

Acceptance criterion: "Given the local-fs readiness probe runs, then it verifies capability
by performing a real scratch-file mtime pin and reading it back — the same operation the adapter
performs…"

  1. Arrange a toolchain whose command output is well-formed (would satisfy a shape check) but
    whose mtime pin does not actually take effect on the file.
  2. Run the readiness guard with forge.adapter: local-fs declared.
  3. Verify: the guard reports not ready — the probe caught the failure by performing the real
    operation, not by inspecting output shape.
  4. On a healthy box, run the guard and verify it reports ready.

Expected outcome: probe-pass demonstrates the pin operation itself succeeding on that box.

Scenario 4: Interrupted post leaves no visible partial comment

Acceptance criterion: "Given comment posting is interrupted at any step boundary before its
commit point (fault-injected at minimum: after number allocation, after a partial content write,
before the pin), when the issue's comments are subsequently read, then no partial comment is
visible…"

  1. On a fresh issue, begin posting a comment and interrupt it after number allocation.
  2. Read the issue's comments through the adapter's read path.
  3. Verify: no partial comment is visible — the record shows nothing from the interrupted attempt.
  4. Repeat with the interruption after a partial content write; verify the same.
  5. Repeat with the interruption after the full write but before the pin; verify the same.

Expected outcome: at every injection point, the record shows either a complete pinned comment
or nothing at all.

Scenario 5: The next post after an interruption succeeds cleanly

Acceptance criterion: "Given a prior posting attempt was interrupted, when the next comment
is posted on the same issue, then it succeeds cleanly — no residue-induced numbering anomaly, no
blocked or corrupted post."

  1. Continue from any interrupted state produced in Scenario 4.
  2. Post a new comment on the same issue.
  3. Verify: the post succeeds; the comment is visible, complete, and correctly pinned.
  4. Verify: reading the issue's comments shows a coherent record — no gaps or duplicates
    attributable to the earlier interruption, and the new comment's ordering is correct.

Expected outcome: an earlier crash never poisons subsequent posts.

Scenario 6: An existing comment is never silently overwritten

Acceptance criterion: "Given a comment already exists, when any posting attempt (concurrent
or retried) would land on the same comment identity, then the existing comment is never silently
overwritten — concurrent posts either land as distinct comments or the loser fails loudly."

  1. On an issue with an existing comment, record that comment's full content and timestamp.
  2. Contrive two posting attempts that would land on the same comment identity (e.g. two writers
    started from the same observed state).
  3. Let both attempts run to completion.
  4. Verify: the pre-existing comment's content and timestamp are unchanged.
  5. Verify: either both new posts are visible as distinct comments, or exactly one is visible and
    the other attempt reported a loud failure — never a silent replacement.

Expected outcome: no write ever silently destroys a record.

Scenario 7: A failed post reports failure to its caller

Acceptance criterion: "Given a posting attempt fails at or before its commit point (e.g.
permission denied, disk full), when the failure occurs, then the caller receives an explicit
error — it never proceeds believing the comment exists."

  1. Arrange the environment so the posting operation must fail (e.g. the target is unwritable).
  2. Attempt to post a comment and capture the operation's result as its caller sees it.
  3. Verify: the operation signals failure explicitly (non-success result with an error message).
  4. Verify: no comment from the failed attempt is visible through the adapter's read path.

Expected outcome: failure is loud and leaves nothing behind — the caller can react instead of
continuing on a phantom record.

Scenario 8: A never-edited comment reports unedited

Acceptance criterion: "Given a comment that was posted successfully and never edited, when
its edit state is checked, then it reports unedited (no false edit signal)."

  1. Post a comment normally and do not touch it afterwards.
  2. Check the comment's edit state through the adapter.
  3. Verify: it reports unedited.
  4. (Cross-check) Edit a different comment and verify that one reports edited — the detector
    distinguishes the two.

Expected outcome: edit detection fires only on real edits.

Scenario 9: No regression on a GNU-coreutils box

Acceptance criterion: "Given a GNU-coreutils box, when the forge flow tests run at pristine
HEAD, then they still pass (no regression from the hardening)."

  1. On the GNU baseline box, run the four forge flow tests at pristine HEAD.
  2. Verify: all four pass.
  3. Post and read back a comment normally; verify correct pin and unedited state.

Expected outcome: the hardening changes nothing for the already-working environment.

Notes

  • "Visible" throughout means through the adapter's comment-read path (the forge contract's read
    operations); temp/staging residue at the raw filesystem level is acceptable provided
    Scenario 5 holds (per the PREQ's visibility definition).
  • Detection/cleanup of partial comments created before this feature is explicitly out of
    scope; no scenario covers it.
  • The uutils behaviour needs no real uutils box — a toolchain shim emulating the output is
    sufficient (PREQ Dependencies).
<!-- test-plan:v1 issue=26 skill=requirements --> # Test Plan: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment ## Prerequisites [State the scenarios need — environment specifics (paths, shim contents) are verified at execution time, not guessed here.] - [ ] A scratch project workspace declaring `forge.adapter: local-fs`, with at least one issue that already carries at least one comment (needed for the no-overwrite scenario). - [ ] A box (or environment) with standard GNU coreutils — the regression baseline. - [ ] A way to present the suite with an altered toolchain (e.g. a `PATH` shim), able to emulate: (a) a `date` that ignores fractional-width specifiers and emits full nanosecond digits (uutils-style); (b) tools that cannot express or apply millisecond mtimes at all; (c) a toolchain whose *output looks well-formed* but whose mtime pin does not actually take effect. - [ ] A way to interrupt or fail a comment-posting operation partway through (fault injection at step boundaries), and a way to induce a write failure (e.g. an unwritable target). ### Required Test Data - [ ] One issue with ≥1 existing, correctly pinned comment (Scenario 6). - [ ] One fresh issue with no comments yet (Scenario 4/5 starting state). ## Test Scenarios ### Scenario 1: Comment posting succeeds on a uutils-style toolchain **Acceptance criterion:** "Given a box whose `date` emits more than millisecond fractional precision for `%s%3N` (uutils-style), when a comment is posted via `local-fs`, then the comment is created with a correct millisecond pin … and the forge flow tests pass at pristine HEAD on that box." 1. Arrange the toolchain so `date` emits full nanosecond digits where millisecond precision was requested (uutils emulation). 2. Post a comment on an issue through the adapter. 3. Verify: the comment is visible through the adapter's read path, with a readable creation timestamp of millisecond precision. 4. Verify: the comment's edit state reports **unedited**. 5. Run the four forge flow tests (`smoke`, `disposition-flow`, `promotion-flow`, `release-flow`) at pristine HEAD in this environment. 6. Verify: all four pass. **Expected outcome:** the quirky-but-capable toolchain is tolerated — posting works, timestamps are correct, nothing downstream fails. ### Scenario 2: Incapable box fails readiness loudly, naming the tool **Acceptance criterion:** "Given a box whose tools cannot perform millisecond mtime pinning at all, when the readiness guard runs with `forge.adapter: local-fs` declared, then the guard reports the box **not ready**, naming the failing tool with its observed vs. expected behaviour…" 1. Arrange the toolchain so millisecond mtime pinning is impossible (e.g. no fractional-second support at all). 2. Run the readiness guard on a project declaring `forge.adapter: local-fs`. 3. Verify: the guard reports **not ready** — never ready. 4. Verify: the report names the failing tool and shows what was observed versus what was expected. **Expected outcome:** one loud up-front message instead of mid-phase failures. ### Scenario 2b (edge): Probe unable to execute also reports not-ready **Acceptance criterion:** same criterion, probe-error clause: "If the probe itself cannot execute (e.g. no writable temp location), the guard likewise reports not-ready — never ready." 1. Arrange the environment so the probe cannot perform its check at all (e.g. no writable location for a scratch file). 2. Run the readiness guard with `forge.adapter: local-fs` declared. 3. Verify: the guard reports not-ready (or an explicit error) — it does not report ready. **Expected outcome:** an inconclusive probe is never treated as a pass. ### Scenario 3: Probe verifies by doing — output shape alone cannot fool it **Acceptance criterion:** "Given the `local-fs` readiness probe runs, then it verifies capability by performing a real scratch-file mtime pin and reading it back — the same operation the adapter performs…" 1. Arrange a toolchain whose command *output* is well-formed (would satisfy a shape check) but whose mtime pin does not actually take effect on the file. 2. Run the readiness guard with `forge.adapter: local-fs` declared. 3. Verify: the guard reports **not ready** — the probe caught the failure by performing the real operation, not by inspecting output shape. 4. On a healthy box, run the guard and verify it reports ready. **Expected outcome:** probe-pass demonstrates the pin operation itself succeeding on that box. ### Scenario 4: Interrupted post leaves no visible partial comment **Acceptance criterion:** "Given comment posting is interrupted at any step boundary before its commit point (fault-injected at minimum: after number allocation, after a partial content write, before the pin), when the issue's comments are subsequently read, then no partial comment is visible…" 1. On a fresh issue, begin posting a comment and interrupt it **after number allocation**. 2. Read the issue's comments through the adapter's read path. 3. Verify: no partial comment is visible — the record shows nothing from the interrupted attempt. 4. Repeat with the interruption **after a partial content write**; verify the same. 5. Repeat with the interruption **after the full write but before the pin**; verify the same. **Expected outcome:** at every injection point, the record shows either a complete pinned comment or nothing at all. ### Scenario 5: The next post after an interruption succeeds cleanly **Acceptance criterion:** "Given a prior posting attempt was interrupted, when the next comment is posted on the same issue, then it succeeds cleanly — no residue-induced numbering anomaly, no blocked or corrupted post." 1. Continue from any interrupted state produced in Scenario 4. 2. Post a new comment on the same issue. 3. Verify: the post succeeds; the comment is visible, complete, and correctly pinned. 4. Verify: reading the issue's comments shows a coherent record — no gaps or duplicates attributable to the earlier interruption, and the new comment's ordering is correct. **Expected outcome:** an earlier crash never poisons subsequent posts. ### Scenario 6: An existing comment is never silently overwritten **Acceptance criterion:** "Given a comment already exists, when any posting attempt (concurrent or retried) would land on the same comment identity, then the existing comment is never silently overwritten — concurrent posts either land as distinct comments or the loser fails loudly." 1. On an issue with an existing comment, record that comment's full content and timestamp. 2. Contrive two posting attempts that would land on the same comment identity (e.g. two writers started from the same observed state). 3. Let both attempts run to completion. 4. Verify: the pre-existing comment's content and timestamp are unchanged. 5. Verify: either both new posts are visible as distinct comments, or exactly one is visible and the other attempt reported a loud failure — never a silent replacement. **Expected outcome:** no write ever silently destroys a record. ### Scenario 7: A failed post reports failure to its caller **Acceptance criterion:** "Given a posting attempt fails at or before its commit point (e.g. permission denied, disk full), when the failure occurs, then the caller receives an explicit error — it never proceeds believing the comment exists." 1. Arrange the environment so the posting operation must fail (e.g. the target is unwritable). 2. Attempt to post a comment and capture the operation's result as its caller sees it. 3. Verify: the operation signals failure explicitly (non-success result with an error message). 4. Verify: no comment from the failed attempt is visible through the adapter's read path. **Expected outcome:** failure is loud and leaves nothing behind — the caller can react instead of continuing on a phantom record. ### Scenario 8: A never-edited comment reports unedited **Acceptance criterion:** "Given a comment that was posted successfully and never edited, when its edit state is checked, then it reports unedited (no false edit signal)." 1. Post a comment normally and do not touch it afterwards. 2. Check the comment's edit state through the adapter. 3. Verify: it reports unedited. 4. (Cross-check) Edit a *different* comment and verify that one reports edited — the detector distinguishes the two. **Expected outcome:** edit detection fires only on real edits. ### Scenario 9: No regression on a GNU-coreutils box **Acceptance criterion:** "Given a GNU-coreutils box, when the forge flow tests run at pristine HEAD, then they still pass (no regression from the hardening)." 1. On the GNU baseline box, run the four forge flow tests at pristine HEAD. 2. Verify: all four pass. 3. Post and read back a comment normally; verify correct pin and unedited state. **Expected outcome:** the hardening changes nothing for the already-working environment. ## Notes - "Visible" throughout means through the adapter's comment-read path (the forge contract's read operations); temp/staging residue at the raw filesystem level is acceptable provided Scenario 5 holds (per the PREQ's visibility definition). - Detection/cleanup of partial comments created *before* this feature is explicitly out of scope; no scenario covers it. - The uutils behaviour needs no real uutils box — a toolchain shim emulating the output is sufficient (PREQ Dependencies).
jbr870 commented 2026-08-13 07:46:59 +00:00
Author
Owner
Copy link
{
  "next_state": "planning",
  "produced": [
    {
      "kind": "preq",
      "ref": "issue-body",
      "summary": "PREQ for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment (fan-in of #23 + #13), Tier 3 reviewed, all concerns answered interactively"
    },
    {
      "kind": "test-plan",
      "ref": "comment:720",
      "summary": "10 validation cases covering 9/9 acceptance criteria (lanes deferred to /technical-plan)"
    },
    {
      "kind": "promotion",
      "ref": "issue-#23",
      "summary": "promoted #23 into this feature"
    },
    {
      "kind": "promotion",
      "ref": "issue-#13",
      "summary": "promoted #13 into this feature"
    }
  ],
  "findings": [
    {
      "category": "out-of-scope",
      "summary": "Boxes that cannot pin millisecond mtimes stay unsupported; capability is now defined behaviorally (probe pin-and-read-back), not by GNU provenance",
      "reasoning": "The local-fs prerequisite stands; the fix is honest detection plus tolerance of quirky-but-capable toolchains, not dropping the requirement",
      "id": "F-PO-26-1-1"
    },
    {
      "category": "out-of-scope",
      "summary": "Detection/cleanup of pre-existing partial comments (e.g. the axana orphan)",
      "reasoning": "One-off manual cleanup; this feature prevents new corruption, it does not remediate old records (operator decision at PREQ review)",
      "id": "F-PO-26-1-2"
    },
    {
      "category": "out-of-scope",
      "summary": "Changing forge-contract created_ms / edit-detection semantics",
      "reasoning": "The mechanism stays; only its reliability changes",
      "id": "F-PO-26-1-3"
    },
    {
      "category": "out-of-scope",
      "summary": "Other adapters (tea-cli, glab-cli, gh-cli)",
      "reasoning": "They do not pin mtimes; the defect class is local-fs-only",
      "id": "F-PO-26-1-4"
    },
    {
      "category": "in-scope-deferrable",
      "summary": "Atomicity for local-fs write paths other than comment posting (issue creation, body edits)",
      "reasoning": "Known follow-up if the same pattern is observed there; this feature fixes the path with the observed corruption",
      "id": "F-PO-26-1-5"
    },
    {
      "category": "out-of-scope",
      "summary": "Full lock-based serialization of concurrent writers",
      "reasoning": "Guarantee chosen at PREQ review is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success",
      "id": "F-PO-26-1-6"
    }
  ],
  "pending_decisions": []
}
<!-- phase-outcome:v1 id=PO-26-1 skill=requirements --> ```json { "next_state": "planning", "produced": [ { "kind": "preq", "ref": "issue-body", "summary": "PREQ for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment (fan-in of #23 + #13), Tier 3 reviewed, all concerns answered interactively" }, { "kind": "test-plan", "ref": "comment:720", "summary": "10 validation cases covering 9/9 acceptance criteria (lanes deferred to /technical-plan)" }, { "kind": "promotion", "ref": "issue-#23", "summary": "promoted #23 into this feature" }, { "kind": "promotion", "ref": "issue-#13", "summary": "promoted #13 into this feature" } ], "findings": [ { "category": "out-of-scope", "summary": "Boxes that cannot pin millisecond mtimes stay unsupported; capability is now defined behaviorally (probe pin-and-read-back), not by GNU provenance", "reasoning": "The local-fs prerequisite stands; the fix is honest detection plus tolerance of quirky-but-capable toolchains, not dropping the requirement", "id": "F-PO-26-1-1" }, { "category": "out-of-scope", "summary": "Detection/cleanup of pre-existing partial comments (e.g. the axana orphan)", "reasoning": "One-off manual cleanup; this feature prevents new corruption, it does not remediate old records (operator decision at PREQ review)", "id": "F-PO-26-1-2" }, { "category": "out-of-scope", "summary": "Changing forge-contract created_ms / edit-detection semantics", "reasoning": "The mechanism stays; only its reliability changes", "id": "F-PO-26-1-3" }, { "category": "out-of-scope", "summary": "Other adapters (tea-cli, glab-cli, gh-cli)", "reasoning": "They do not pin mtimes; the defect class is local-fs-only", "id": "F-PO-26-1-4" }, { "category": "in-scope-deferrable", "summary": "Atomicity for local-fs write paths other than comment posting (issue creation, body edits)", "reasoning": "Known follow-up if the same pattern is observed there; this feature fixes the path with the observed corruption", "id": "F-PO-26-1-5" }, { "category": "out-of-scope", "summary": "Full lock-based serialization of concurrent writers", "reasoning": "Guarantee chosen at PREQ review is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success", "id": "F-PO-26-1-6" } ], "pending_decisions": [] } ```
jbr870 commented 2026-08-13 08:04:20 +00:00
Author
Owner
Copy link

Software Requirements: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment

Context

Projects on the local-fs forge adapter cannot trust their comment record: the millisecond mtime
pin (created_at anchor + edit-detection signal) dies on uutils-style coreutils after a green
readiness check, and comment posting writes directly to the final visible path, so a crash at any
step boundary leaves a visible partial comment and same-identity writers silently overwrite.
Users are operators of local-fs projects (the files ARE their issue record) and suite maintainers
(local-fs is the contract-development adapter). Constraint: helper-tier baseline only — bash ≥ 3.2,
jq, git, POSIX utilities; no new tool prerequisites.

Approaches Considered

Approach A: Harden in place, per-script

Summary: Fix the clock inline in each script, add staged-write logic separately to
post_comment.sh and comment_on_pr.sh, rewrite the readiness probe inline in readiness-check.md.
Pros: No _lib.sh API change; smallest per-file diff.
Cons: Commit protocol duplicated across two scripts (the existing duplication is exactly how
they drift); probe logic re-implemented in guard prose instead of performing the adapter's real
operation — the shape-check-vs-doing mistake can recur; readiness guard and adapter each own a copy
of capability knowledge.
Effort: Medium

Approach B: Shared primitives in _lib.sh + adapter-owned probe helper

Summary: Normalize the ms clock once in _lib.sh (both write and read side), factor one atomic
commit primitive used by both posting scripts, and ship a probe_pin.sh helper that the readiness
guard invokes — the probe performs the adapter's own pin+readback using the same lib functions.
Pros: One mechanism, one home; fixes the existing post/comment_on_pr duplication; readiness
delegates to the adapter (no duplicated capability knowledge); probe-by-doing is literally the
adapter's operation.
Cons: _lib.sh grows two functions; posting scripts change shape (small migration risk,
covered by existing flow tests).
Effort: Medium

Approach C: Staging-dir WAL with commit marker

Summary: Write comments under comments/.staging/ with a journal/commit marker; read path
validates the marker before trusting a file.
Pros: Generalizes to multi-file transactions.
Cons: Changes the read-path contract (every consumer must learn the marker); more moving parts;
no added guarantee over an atomic no-clobber link, which POSIX already provides.
Effort: High

Decision

Selected: Approach B
Rationale: The two defects share one mechanism (the pin) and one blast radius (the partial
write); a shared primitive fixes both posting paths at once and removes the duplication that let
them diverge. Probe-by-doing via the adapter's own helper makes a probe pass attest the exact
operation the adapter performs — the readiness guard keeps zero capability knowledge of its own.

Architecture

Component Overview

readiness-check.md (Step 0, POSIX glue)
    └─ invokes → local-fs/bin/probe_pin.sh ──┐
post_comment.sh    ─┐                        │ sources
comment_on_pr.sh   ─┼─ source ──► local-fs/bin/_lib.sh
scan_comments.sh   ─┘                (clock normalization, pin,
                                      atomic commit primitive)
  • _lib.sh — _lfs_now_ms / _lfs_mtime_ms (normalized clock, both directions),
    _lfs_pin_mtime (unchanged), new _lfs_commit_comment (staged atomic post), _lfs_fault
    (inert fault-injection hooks).
  • post_comment.sh / comment_on_pr.sh — argument parsing + directory resolution only; the
    write protocol lives in the primitive.
  • probe_pin.sh (new) — real scratch pin + readback; the readiness guard's sole local-fs
    capability check.
  • scan_comments.sh — unchanged read path (*.md glob, header on line 1); gains correctness
    from the normalized _lfs_mtime_ms.

Data Flow

Posting (single commit point):

  1. Best-effort GC: remove .stage-* files in the target comments dir older than the staleness
    threshold (see Key Decisions) — bounds kill -9 orphan residue.
  2. Compute ms via normalized clock; stage body to $cdir/.stage-{pid}-{ms} (dot-prefixed →
    invisible to the *.md read glob). Fault hook after-partial-write sits mid-write.
  3. Pin the stage file's mtime to ms. Fault hook before-pin sits before this step.
  4. Allocate seq = max(seqfile value, highest existing NNNN- prefix in $cdir) — the seqfile is
    an advisory cache; existing files are the authority. Fault hook after-alloc sits here.
  5. Commit: ln "$stage" "$cdir/{seq}-{ms}.md" — hard link, fails if target exists (no-clobber).
    On exists-collision: seq+1, bounded retry. On any other failure: explicit error to stderr,
    nonzero exit, stage cleaned. On success: best-effort seqfile update to seq+1 (failure → stderr
    warning), remove stage, emit {comment_id}.
  6. meta.json updated_at refresh is best-effort after commit (failure → stderr warning): the
    comment is already durable; failing the post over a cosmetic timestamp would report "failed"
    about a record that exists and invite a duplicating retry.

Reading: unchanged — created_ms from filename, edited_ms from normalized mtime readback;
edited_ms > created_ms is the edit signal. Ordering stays by created_ms with comment_id
tiebreaker (local-fs SKILL.md already mandates this; filename order is for human inspection only).

Probing (readiness): probe_pin.sh [--dir DIR] creates a scratch file (preferring the devwork
root's filesystem over TMPDIR — tmpfs mtime behavior can diverge from the real fs), computes a
normalized ms, pins, reads back via _lfs_mtime_ms, compares equality. Failure names the failing
tool with observed vs expected output. Inability to execute at all (no writable scratch location)
is nonzero — never ready. The guard relays the message verbatim.

External Data Contracts

None — the 2.2b inventory is empty (external-deps.json = []). The adapter's substrate is the
local filesystem the tests create fresh; no external system of record, no seeded datastore whose
existing contents are load-bearing.

Key Decisions

Decision Choice Rationale
Clock normalization Single date +%s.%N call; split on .; validate all-digits and frac ≥ 3 digits; ms = sec×1000 + first 3 frac digits (truncation) One call avoids a second-boundary race between two date invocations; truncating zero-padded %N yields correct ms on GNU (3 digits) and uutils (9 digits) alike; any output carrying less than ms information fails digit validation → loud die naming observed vs expected
Read-side hardening _lfs_mtime_ms = date -r FILE +%s.%N with the same normalization The uutils quirk infects the read side identically — an unnormalized edited_ms (19 digits) > pinned created_ms (13) forges a false edit signal on every comment. date -r is GNU/BSD/uutils, not POSIX: intended coverage is the probe's readback, which exercises _lfs_mtime_ms itself — a box without -r fails the probe, not a mid-run scan
Per-op guard _lfs_require_gnu_coreutils → behavioral check: run the normalizer once, validate a 13-digit result Shape-checking raw tool output is the defect that let uutils through; the full pin-by-doing stays in the probe (per-op it would be redundant — ops now fail loudly themselves)
Commit point No-clobber hard link (ln) from stage file to final name Atomic on POSIX; fails (loudly) if the target exists, so an exact-identity loser can never silently overwrite; link preserves mtime, so the pin survives commit. Stage-in-same-dir is normative — it guarantees same-filesystem linking, which is load-bearing for atomicity. A filesystem without hard links fails loudly at commit — acceptable (loud, not silent)
Comment identity & duplicate seq Identity = {seq}-{ms} (the filename stem). Concurrent writers that read the same seq get different ms → distinct identities, both land — duplicate seq prefixes are permitted and are "distinct comments" per the PREQ's no-overwrite criterion The alternative (seq-prefix collision checks via glob) is TOCTOU-racy and buys nothing: ordering is normatively by created_ms + comment_id tiebreak, never by filename, and PO-ordinal derivation is a count — neither is disturbed by a duplicated prefix. The no-clobber link still guards the true identity
Seq allocation Derived from existing files (max NNNN- prefix), seqfile is an advisory cache (max of both wins); updated best-effort after commit A crash before commit burns nothing (no gap); a lost seqfile update self-heals on the next allocation because files are the authority — resolves the reviewer-flagged recovery ambiguity
Stale-stage GC At post start, best-effort remove .stage-* with mtime older than 15 minutes in the target comments dir; threshold is normative in SKILL.md kill -9 (and the fault hooks) skip EXIT traps, so orphans accumulate; a legitimate in-flight stage lives milliseconds-to-seconds, so 15 min is orders of magnitude beyond any live writer — GC can never delete a concurrent writer's in-flight temp. Residue younger than the threshold is contract-permitted (PREQ visibility definition)
Fault-injection hooks _lfs_fault <point> in the shipped primitive; inert unless DEVWORK_LFS_FAULT names the point, then kills the process. Points (normative, in SKILL.md): after-alloc, after-partial-write, before-pin The AC mandates deterministic fault injection at named boundaries; env-guarded hooks are the only deterministic way. The contract owns their placement so tests can't drift from the spec
Capability wording "GNU date/touch" → "probe-verified capable date/touch" in local-fs SKILL.md, readiness-check.md, and repo CLAUDE.md tier table The PREQ defines capability behaviorally (passing the probe), not by provenance; uutils qualifies once hardening lands
comment_on_pr.sh included Both posting scripts route through _lfs_commit_comment It IS comment posting (the PR side) and today duplicates the identical broken protocol inline; sharing the primitive is cheaper than divergence and is how the duplication debt gets paid down

Technical Risks

Risk Likelihood Impact Mitigation
Filesystem without hard links (e.g. some FUSE/FAT mounts) breaks the commit Low Med Fails loudly at commit (explicit error), never corrupts; stage-in-same-dir is normative so cross-device links can't arise. Not probed at readiness — accepted as loud-failure territory
date -r unsupported on some exotic non-GNU toolchain Low Low Probe readback exercises _lfs_mtime_ms → box reports not-ready up front, never mid-run
Flow-test churn from the protocol change (tests pre-seed .next-comment-number) Med Low Seqfile stays honored as advisory cache (max with file scan), so pre-seeded harness state keeps working; flow tests are the regression gate (AC-9)
Bounded commit-retry exhaustion under pathological contention Low Low Loud error after N retries — loud failure for a losing writer is contract-acceptable

Expert Review

Reviewers

  • Backend Developer (fable): two blocking concerns — duplicate-seq escape past the no-clobber
    link (collision keyed on full name while ms differs per writer, breaking the claimed seqfile
    self-heal) and kill -9 orphaning stage files past the EXIT trap; flagged date -r
    non-POSIXness and undefined identity/ordering under duplicate seq.
  • Solution Architect (fable): same two gaps independently (seq recovery rule undefined; GC
    criterion undefined and potentially racing live writers) plus: make stage/target co-location
    normative (load-bearing for ln atomicity), spec the fault hooks in SKILL.md, note legacy
    partial comments remain visible. Confirmed dependency direction (guard delegating to the
    adapter's probe) and tier boundaries are right.

Changes Made

  • Comment identity, duplicate-seq acceptability, and ordering are now explicit Key Decisions:
    identity = {seq}-{ms}, ordering by created_ms + tiebreak (already normative), duplicate
    prefixes allowed as distinct comments — raised independently by both reviewers.
  • Seq allocation redefined: existing files are the authority, seqfile an advisory cache updated
    best-effort after commit — replaces the broken "collide and retry" self-heal story.
  • Stale-stage GC made normative with a 15-minute threshold and a can't-race-live-writers rationale.
  • Stage/target co-location (same directory ⇒ same filesystem) promoted to a normative SKILL.md
    requirement.
  • Fault hooks specified in SKILL.md (inert without env var; the three points named by the contract).
  • Probe scratch prefers the devwork root's filesystem over TMPDIR (tmpfs mtime divergence).
  • Best-effort steps (meta.json refresh, seqfile update) emit stderr warnings when they fail.
  • date -r coverage intent stated (probe readback is the intended detection point).

Noted (not actioned)

  • Legacy partial comments written by the old protocol remain visible through the read path
    (Architect). Not actioned by design: the PREQ explicitly rules pre-existing-partial detection/
    cleanup out of scope (one-off manual remediation) — carried as out-of-scope finding F#2 on this
    Phase Outcome rather than new work.

Acceptance Criteria

ID Criterion (from PREQ) Verification approach
AC-1 Toolchain emitting > ms fractional precision (uutils-style) still yields a correct ms pin; forge flow tests pass at pristine HEAD under it; at-least-ms output normalizes, less-than-ms fails preflight Integration: shim-portability suite (local-fs/test/shim-portability.sh, new) runs post→scan roundtrip + the four flow tests under a uutils-emulating date PATH shim
AC-2 Box that cannot pin ms mtimes: readiness reports not-ready, naming the failing tool with observed vs expected; probe unable to execute likewise reports not-ready — never ready Integration: probe scenarios in the shim suite (incapable-date shim; unwritable scratch dir) assert probe_pin.sh nonzero exit + message naming the tool
AC-3 Probe verifies by doing: real scratch-file pin + readback, the adapter's own operation Integration: "liar touch" shim (exit 0, mtime unchanged) — probe must fail via readback; healthy box — probe exits 0. Mechanical: probe sources _lib.sh and calls _lfs_pin_mtime/_lfs_mtime_ms (grep)
AC-4 Interruption at each boundary (after alloc, after partial write, before pin) → no partial comment visible via the adapter read path Integration: crash suite (local-fs/test/crash.sh, new) sets DEVWORK_LFS_FAULT per point, then asserts scan_comments sees nothing
AC-5 Next post after an interruption succeeds cleanly — no residue-induced numbering anomaly, no blocked/corrupted post Integration: crash suite continues from each interrupted state; asserts post succeeds, record coherent (no gaps/duplicates from the interruption, correct ordering)
AC-6 Existing comment never silently overwritten; same-identity concurrent/retried posts land distinct or lose loudly Integration: crash suite pre-stages an identity collision (existing {seq}-{ms}.md) → commit must fail loudly with the original byte-identical; concurrent-writer case → distinct comments, original untouched
AC-7 Post failing at/before commit (perm denied, disk full) → explicit error to caller; nothing visible Integration: crash suite posts into an unwritable comments dir; asserts nonzero exit + stderr message + empty scan
AC-8 Never-edited comment reports unedited (no false edit signal) Integration: existing local-fs/test/smoke.sh asserts created_ms == edited_ms post-roundtrip (also exercised under the uutils shim via AC-1)
AC-9 GNU box: four flow tests pass at pristine HEAD — no regression Integration: existing _shared/procedures/test/{smoke,disposition-flow,promotion-flow,release-flow}.sh on the GNU baseline
AC-10 Every comment-posting call site routes through the shared commit primitive — no direct writes to a final *.md path outside it Mechanical: grep test in the crash suite — post_comment.sh and comment_on_pr.sh contain no redirection into $cdir paths and both call _lfs_commit_comment; only _lib.sh touches stage/commit mechanics

(No Observability & Audit rows: project observability.mode: none — declared with reason in
CLAUDE.md. No temporary scaffolding introduced. AC-10 is the SREQ-added mechanical
route-through-helper row.)

Implementation Scope

Areas

Area Files / directories involved Nature of change
Adapter lib .claude/skills/local-fs/bin/_lib.sh extend — normalized clock (both directions), behavioral per-op guard, _lfs_commit_comment, _lfs_fault
Posting scripts .claude/skills/local-fs/bin/post_comment.sh, comment_on_pr.sh modify — delegate write protocol to the primitive
Probe helper .claude/skills/local-fs/bin/probe_pin.sh new
Readiness guard .claude/skills/_shared/procedures/readiness-check.md (Step 0 local-fs block) modify — replace inline shape checks with probe invocation, verbatim relay
Normative spec .claude/skills/local-fs/SKILL.md (post_comment steps, prerequisites, fault hooks, GC threshold, identity/ordering note) modify
Tier docs repo CLAUDE.md (tier table wording: behavioral capability) modify
Tests .claude/skills/local-fs/test/crash.sh (new), local-fs/test/shim-portability.sh + shim fixtures (new), existing flow tests untouched new / regression

File Boundaries

_lib.sh is the shared dependency — it lands first and alone. The two posting scripts, the probe
helper, and the doc edits are then independent of each other (non-overlapping files). The test
suites touch only local-fs/test/.

Dependencies & Sequencing

  1. _lib.sh primitives (clock + commit + fault hooks) — everything depends on these.
  2. Then in parallel: posting-script migration; probe_pin.sh; SKILL.md/CLAUDE.md/readiness-check
    text.
  3. Tests are authored red-first against the PREQ-derived test plan (TDD) and go green as 1–2 land.

Constraints & Non-Goals

Constraints:

  • Helper-tier baseline only: bash ≥ 3.2, jq, git, POSIX utilities — no new tool prerequisites.
    The date/touch capability prerequisite stays, redefined behaviorally (probe-verified).
  • Skill-emitted glue (the readiness guard's invocation) stays POSIX — it is an invocation +
    exit-code check only; all capability logic lives in the helper tier.
  • Forge-contract created_ms/edit-detection semantics unchanged (reliability only).
  • Read path (scan_comments.sh glob + header rule) unchanged.

Non-goals (do NOT build):

  • Support for boxes that genuinely cannot pin ms mtimes — honest detection, not a dropped
    prerequisite.
  • Detection/cleanup of pre-existing partial comments (e.g. the axana orphan) — one-off manual
    remediation.
  • Forge-contract semantic changes.
  • Changes to other adapters (tea-cli, glab-cli, gh-cli) — they do not pin mtimes.
  • Atomicity for local-fs write paths other than comment posting (issue creation, body edits) —
    known follow-up if the pattern recurs there.
  • Full lock-based serialization of concurrent writers — the guarantee is no-silent-overwrite;
    loud failure for a losing writer is acceptable.
<!-- sreq:v1 issue=26 skill=technical-plan --> # Software Requirements: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment ## Context Projects on the `local-fs` forge adapter cannot trust their comment record: the millisecond mtime pin (`created_at` anchor + edit-detection signal) dies on uutils-style coreutils after a green readiness check, and comment posting writes directly to the final visible path, so a crash at any step boundary leaves a visible partial comment and same-identity writers silently overwrite. Users are operators of local-fs projects (the files ARE their issue record) and suite maintainers (local-fs is the contract-development adapter). Constraint: helper-tier baseline only — bash ≥ 3.2, `jq`, `git`, POSIX utilities; no new tool prerequisites. ## Approaches Considered ### Approach A: Harden in place, per-script **Summary:** Fix the clock inline in each script, add staged-write logic separately to `post_comment.sh` and `comment_on_pr.sh`, rewrite the readiness probe inline in `readiness-check.md`. **Pros:** No `_lib.sh` API change; smallest per-file diff. **Cons:** Commit protocol duplicated across two scripts (the existing duplication is exactly how they drift); probe logic re-implemented in guard prose instead of performing the adapter's real operation — the shape-check-vs-doing mistake can recur; readiness guard and adapter each own a copy of capability knowledge. **Effort:** Medium ### Approach B: Shared primitives in `_lib.sh` + adapter-owned probe helper **Summary:** Normalize the ms clock once in `_lib.sh` (both write and read side), factor one atomic commit primitive used by both posting scripts, and ship a `probe_pin.sh` helper that the readiness guard invokes — the probe performs the adapter's own pin+readback using the same lib functions. **Pros:** One mechanism, one home; fixes the existing post/comment_on_pr duplication; readiness delegates to the adapter (no duplicated capability knowledge); probe-by-doing is literally the adapter's operation. **Cons:** `_lib.sh` grows two functions; posting scripts change shape (small migration risk, covered by existing flow tests). **Effort:** Medium ### Approach C: Staging-dir WAL with commit marker **Summary:** Write comments under `comments/.staging/` with a journal/commit marker; read path validates the marker before trusting a file. **Pros:** Generalizes to multi-file transactions. **Cons:** Changes the read-path contract (every consumer must learn the marker); more moving parts; no added guarantee over an atomic no-clobber link, which POSIX already provides. **Effort:** High ## Decision **Selected:** Approach B **Rationale:** The two defects share one mechanism (the pin) and one blast radius (the partial write); a shared primitive fixes both posting paths at once and removes the duplication that let them diverge. Probe-by-doing via the adapter's own helper makes a probe pass attest the exact operation the adapter performs — the readiness guard keeps zero capability knowledge of its own. ## Architecture ### Component Overview ``` readiness-check.md (Step 0, POSIX glue) └─ invokes → local-fs/bin/probe_pin.sh ──┐ post_comment.sh ─┐ │ sources comment_on_pr.sh ─┼─ source ──► local-fs/bin/_lib.sh scan_comments.sh ─┘ (clock normalization, pin, atomic commit primitive) ``` - **`_lib.sh`** — `_lfs_now_ms` / `_lfs_mtime_ms` (normalized clock, both directions), `_lfs_pin_mtime` (unchanged), new `_lfs_commit_comment` (staged atomic post), `_lfs_fault` (inert fault-injection hooks). - **`post_comment.sh` / `comment_on_pr.sh`** — argument parsing + directory resolution only; the write protocol lives in the primitive. - **`probe_pin.sh`** (new) — real scratch pin + readback; the readiness guard's sole local-fs capability check. - **`scan_comments.sh`** — unchanged read path (`*.md` glob, header on line 1); gains correctness from the normalized `_lfs_mtime_ms`. ### Data Flow **Posting (single commit point):** 1. Best-effort GC: remove `.stage-*` files in the target comments dir older than the staleness threshold (see Key Decisions) — bounds `kill -9` orphan residue. 2. Compute `ms` via normalized clock; stage body to `$cdir/.stage-{pid}-{ms}` (dot-prefixed → invisible to the `*.md` read glob). Fault hook `after-partial-write` sits mid-write. 3. Pin the stage file's mtime to `ms`. Fault hook `before-pin` sits before this step. 4. Allocate `seq` = max(seqfile value, highest existing `NNNN-` prefix in `$cdir`) — the seqfile is an advisory cache; existing files are the authority. Fault hook `after-alloc` sits here. 5. Commit: `ln "$stage" "$cdir/{seq}-{ms}.md"` — hard link, fails if target exists (no-clobber). On exists-collision: seq+1, bounded retry. On any other failure: explicit error to stderr, nonzero exit, stage cleaned. On success: best-effort seqfile update to seq+1 (failure → stderr warning), remove stage, emit `{comment_id}`. 6. `meta.json` `updated_at` refresh is best-effort after commit (failure → stderr warning): the comment is already durable; failing the post over a cosmetic timestamp would report "failed" about a record that exists and invite a duplicating retry. **Reading:** unchanged — `created_ms` from filename, `edited_ms` from normalized mtime readback; `edited_ms > created_ms` is the edit signal. Ordering stays by `created_ms` with `comment_id` tiebreaker (local-fs SKILL.md already mandates this; filename order is for human inspection only). **Probing (readiness):** `probe_pin.sh [--dir DIR]` creates a scratch file (preferring the devwork root's filesystem over `TMPDIR` — tmpfs mtime behavior can diverge from the real fs), computes a normalized ms, pins, reads back via `_lfs_mtime_ms`, compares equality. Failure names the failing tool with observed vs expected output. Inability to execute at all (no writable scratch location) is nonzero — never ready. The guard relays the message verbatim. ### External Data Contracts None — the 2.2b inventory is empty (`external-deps.json` = `[]`). The adapter's substrate is the local filesystem the tests create fresh; no external system of record, no seeded datastore whose existing contents are load-bearing. ### Key Decisions | Decision | Choice | Rationale | | --- | --- | --- | | Clock normalization | Single `date +%s.%N` call; split on `.`; validate all-digits and frac ≥ 3 digits; ms = sec×1000 + first 3 frac digits (truncation) | One call avoids a second-boundary race between two `date` invocations; truncating zero-padded `%N` yields correct ms on GNU (3 digits) and uutils (9 digits) alike; any output carrying less than ms information fails digit validation → loud die naming observed vs expected | | Read-side hardening | `_lfs_mtime_ms` = `date -r FILE +%s.%N` with the same normalization | The uutils quirk infects the read side identically — an unnormalized `edited_ms` (19 digits) > pinned `created_ms` (13) forges a false edit signal on every comment. `date -r` is GNU/BSD/uutils, not POSIX: intended coverage is the probe's readback, which exercises `_lfs_mtime_ms` itself — a box without `-r` fails the probe, not a mid-run scan | | Per-op guard | `_lfs_require_gnu_coreutils` → behavioral check: run the normalizer once, validate a 13-digit result | Shape-checking raw tool output is the defect that let uutils through; the full pin-by-doing stays in the probe (per-op it would be redundant — ops now fail loudly themselves) | | Commit point | No-clobber hard link (`ln`) from stage file to final name | Atomic on POSIX; fails (loudly) if the target exists, so an exact-identity loser can never silently overwrite; link preserves mtime, so the pin survives commit. Stage-in-same-dir is **normative** — it guarantees same-filesystem linking, which is load-bearing for atomicity. A filesystem without hard links fails loudly at commit — acceptable (loud, not silent) | | Comment identity & duplicate seq | Identity = `{seq}-{ms}` (the filename stem). Concurrent writers that read the same seq get different `ms` → distinct identities, both land — **duplicate seq prefixes are permitted** and are "distinct comments" per the PREQ's no-overwrite criterion | The alternative (seq-prefix collision checks via glob) is TOCTOU-racy and buys nothing: ordering is normatively by `created_ms` + `comment_id` tiebreak, never by filename, and PO-ordinal derivation is a count — neither is disturbed by a duplicated prefix. The no-clobber link still guards the true identity | | Seq allocation | Derived from existing files (max `NNNN-` prefix), seqfile is an advisory cache (max of both wins); updated best-effort after commit | A crash before commit burns nothing (no gap); a lost seqfile update self-heals on the next allocation because files are the authority — resolves the reviewer-flagged recovery ambiguity | | Stale-stage GC | At post start, best-effort remove `.stage-*` with mtime older than **15 minutes** in the target comments dir; threshold is normative in SKILL.md | `kill -9` (and the fault hooks) skip EXIT traps, so orphans accumulate; a legitimate in-flight stage lives milliseconds-to-seconds, so 15 min is orders of magnitude beyond any live writer — GC can never delete a concurrent writer's in-flight temp. Residue younger than the threshold is contract-permitted (PREQ visibility definition) | | Fault-injection hooks | `_lfs_fault <point>` in the shipped primitive; inert unless `DEVWORK_LFS_FAULT` names the point, then kills the process. Points (normative, in SKILL.md): `after-alloc`, `after-partial-write`, `before-pin` | The AC mandates deterministic fault injection at named boundaries; env-guarded hooks are the only deterministic way. The contract owns their placement so tests can't drift from the spec | | Capability wording | "GNU `date`/`touch`" → "probe-verified capable `date`/`touch`" in local-fs SKILL.md, `readiness-check.md`, and repo CLAUDE.md tier table | The PREQ defines capability behaviorally (passing the probe), not by provenance; uutils qualifies once hardening lands | | `comment_on_pr.sh` included | Both posting scripts route through `_lfs_commit_comment` | It IS comment posting (the PR side) and today duplicates the identical broken protocol inline; sharing the primitive is cheaper than divergence and is how the duplication debt gets paid down | ## Technical Risks | Risk | Likelihood | Impact | Mitigation | | --- | --- | --- | --- | | Filesystem without hard links (e.g. some FUSE/FAT mounts) breaks the commit | Low | Med | Fails loudly at commit (explicit error), never corrupts; stage-in-same-dir is normative so cross-device links can't arise. Not probed at readiness — accepted as loud-failure territory | | `date -r` unsupported on some exotic non-GNU toolchain | Low | Low | Probe readback exercises `_lfs_mtime_ms` → box reports not-ready up front, never mid-run | | Flow-test churn from the protocol change (tests pre-seed `.next-comment-number`) | Med | Low | Seqfile stays honored as advisory cache (max with file scan), so pre-seeded harness state keeps working; flow tests are the regression gate (AC-9) | | Bounded commit-retry exhaustion under pathological contention | Low | Low | Loud error after N retries — loud failure for a losing writer is contract-acceptable | ## Expert Review ### Reviewers - Backend Developer (`fable`): two blocking concerns — duplicate-seq escape past the no-clobber link (collision keyed on full name while `ms` differs per writer, breaking the claimed seqfile self-heal) and `kill -9` orphaning stage files past the EXIT trap; flagged `date -r` non-POSIXness and undefined identity/ordering under duplicate seq. - Solution Architect (`fable`): same two gaps independently (seq recovery rule undefined; GC criterion undefined and potentially racing live writers) plus: make stage/target co-location normative (load-bearing for `ln` atomicity), spec the fault hooks in SKILL.md, note legacy partial comments remain visible. Confirmed dependency direction (guard delegating to the adapter's probe) and tier boundaries are right. ### Changes Made - Comment identity, duplicate-seq acceptability, and ordering are now explicit Key Decisions: identity = `{seq}-{ms}`, ordering by `created_ms` + tiebreak (already normative), duplicate prefixes allowed as distinct comments — raised independently by both reviewers. - Seq allocation redefined: existing files are the authority, seqfile an advisory cache updated best-effort after commit — replaces the broken "collide and retry" self-heal story. - Stale-stage GC made normative with a 15-minute threshold and a can't-race-live-writers rationale. - Stage/target co-location (same directory ⇒ same filesystem) promoted to a normative SKILL.md requirement. - Fault hooks specified in SKILL.md (inert without env var; the three points named by the contract). - Probe scratch prefers the devwork root's filesystem over `TMPDIR` (tmpfs mtime divergence). - Best-effort steps (`meta.json` refresh, seqfile update) emit stderr warnings when they fail. - `date -r` coverage intent stated (probe readback is the intended detection point). ### Noted (not actioned) - Legacy partial comments written by the old protocol remain visible through the read path (Architect). Not actioned by design: the PREQ explicitly rules pre-existing-partial detection/ cleanup out of scope (one-off manual remediation) — carried as out-of-scope finding F#2 on this Phase Outcome rather than new work. ## Acceptance Criteria | ID | Criterion (from PREQ) | Verification approach | | --- | --- | --- | | AC-1 | Toolchain emitting > ms fractional precision (uutils-style) still yields a correct ms pin; forge flow tests pass at pristine HEAD under it; at-least-ms output normalizes, less-than-ms fails preflight | Integration: shim-portability suite (`local-fs/test/shim-portability.sh`, new) runs post→scan roundtrip + the four flow tests under a uutils-emulating `date` PATH shim | | AC-2 | Box that cannot pin ms mtimes: readiness reports not-ready, naming the failing tool with observed vs expected; probe unable to execute likewise reports not-ready — never ready | Integration: probe scenarios in the shim suite (incapable-`date` shim; unwritable scratch dir) assert `probe_pin.sh` nonzero exit + message naming the tool | | AC-3 | Probe verifies by doing: real scratch-file pin + readback, the adapter's own operation | Integration: "liar `touch`" shim (exit 0, mtime unchanged) — probe must fail via readback; healthy box — probe exits 0. Mechanical: probe sources `_lib.sh` and calls `_lfs_pin_mtime`/`_lfs_mtime_ms` (grep) | | AC-4 | Interruption at each boundary (after alloc, after partial write, before pin) → no partial comment visible via the adapter read path | Integration: crash suite (`local-fs/test/crash.sh`, new) sets `DEVWORK_LFS_FAULT` per point, then asserts `scan_comments` sees nothing | | AC-5 | Next post after an interruption succeeds cleanly — no residue-induced numbering anomaly, no blocked/corrupted post | Integration: crash suite continues from each interrupted state; asserts post succeeds, record coherent (no gaps/duplicates from the interruption, correct ordering) | | AC-6 | Existing comment never silently overwritten; same-identity concurrent/retried posts land distinct or lose loudly | Integration: crash suite pre-stages an identity collision (existing `{seq}-{ms}.md`) → commit must fail loudly with the original byte-identical; concurrent-writer case → distinct comments, original untouched | | AC-7 | Post failing at/before commit (perm denied, disk full) → explicit error to caller; nothing visible | Integration: crash suite posts into an unwritable comments dir; asserts nonzero exit + stderr message + empty scan | | AC-8 | Never-edited comment reports unedited (no false edit signal) | Integration: existing `local-fs/test/smoke.sh` asserts `created_ms == edited_ms` post-roundtrip (also exercised under the uutils shim via AC-1) | | AC-9 | GNU box: four flow tests pass at pristine HEAD — no regression | Integration: existing `_shared/procedures/test/{smoke,disposition-flow,promotion-flow,release-flow}.sh` on the GNU baseline | | AC-10 | Every comment-posting call site routes through the shared commit primitive — no direct writes to a final `*.md` path outside it | Mechanical: grep test in the crash suite — `post_comment.sh` and `comment_on_pr.sh` contain no redirection into `$cdir` paths and both call `_lfs_commit_comment`; only `_lib.sh` touches stage/commit mechanics | (No Observability & Audit rows: project `observability.mode: none` — declared with reason in CLAUDE.md. No temporary scaffolding introduced. AC-10 is the SREQ-added mechanical route-through-helper row.) ## Implementation Scope ### Areas | Area | Files / directories involved | Nature of change | | --- | --- | --- | | Adapter lib | `.claude/skills/local-fs/bin/_lib.sh` | extend — normalized clock (both directions), behavioral per-op guard, `_lfs_commit_comment`, `_lfs_fault` | | Posting scripts | `.claude/skills/local-fs/bin/post_comment.sh`, `comment_on_pr.sh` | modify — delegate write protocol to the primitive | | Probe helper | `.claude/skills/local-fs/bin/probe_pin.sh` | new | | Readiness guard | `.claude/skills/_shared/procedures/readiness-check.md` (Step 0 local-fs block) | modify — replace inline shape checks with probe invocation, verbatim relay | | Normative spec | `.claude/skills/local-fs/SKILL.md` (post_comment steps, prerequisites, fault hooks, GC threshold, identity/ordering note) | modify | | Tier docs | repo `CLAUDE.md` (tier table wording: behavioral capability) | modify | | Tests | `.claude/skills/local-fs/test/crash.sh` (new), `local-fs/test/shim-portability.sh` + shim fixtures (new), existing flow tests untouched | new / regression | ### File Boundaries `_lib.sh` is the shared dependency — it lands first and alone. The two posting scripts, the probe helper, and the doc edits are then independent of each other (non-overlapping files). The test suites touch only `local-fs/test/`. ### Dependencies & Sequencing 1. `_lib.sh` primitives (clock + commit + fault hooks) — everything depends on these. 2. Then in parallel: posting-script migration; `probe_pin.sh`; SKILL.md/CLAUDE.md/readiness-check text. 3. Tests are authored red-first against the PREQ-derived test plan (TDD) and go green as 1–2 land. ## Constraints & Non-Goals **Constraints:** - Helper-tier baseline only: bash ≥ 3.2, `jq`, `git`, POSIX utilities — no new tool prerequisites. The `date`/`touch` capability prerequisite stays, redefined behaviorally (probe-verified). - Skill-emitted glue (the readiness guard's invocation) stays POSIX — it is an invocation + exit-code check only; all capability logic lives in the helper tier. - Forge-contract `created_ms`/edit-detection semantics unchanged (reliability only). - Read path (`scan_comments.sh` glob + header rule) unchanged. **Non-goals (do NOT build):** - Support for boxes that genuinely cannot pin ms mtimes — honest detection, not a dropped prerequisite. - Detection/cleanup of pre-existing partial comments (e.g. the axana orphan) — one-off manual remediation. - Forge-contract semantic changes. - Changes to other adapters (`tea-cli`, `glab-cli`, `gh-cli`) — they do not pin mtimes. - Atomicity for local-fs write paths other than comment posting (issue creation, body edits) — known follow-up if the pattern recurs there. - Full lock-based serialization of concurrent writers — the guarantee is no-silent-overwrite; loud failure for a losing writer is acceptable.
jbr870 commented 2026-08-13 08:05:55 +00:00
Author
Owner
Copy link

Test Plan: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment

Prerequisites

[State the scenarios need — environment specifics (paths, shim contents) are verified at
execution time, not guessed here.]

  • A scratch project workspace declaring forge.adapter: local-fs, with at least one issue
    that already carries at least one comment (needed for the no-overwrite scenario).
  • A box (or environment) with standard GNU coreutils — the regression baseline.
  • A way to present the suite with an altered toolchain (e.g. a PATH shim), able to emulate:
    (a) a date that ignores fractional-width specifiers and emits full nanosecond digits
    (uutils-style); (b) tools that cannot express or apply millisecond mtimes at all;
    (c) a toolchain whose output looks well-formed but whose mtime pin does not actually
    take effect.
  • A way to interrupt or fail a comment-posting operation partway through (fault injection at
    step boundaries), and a way to induce a write failure (e.g. an unwritable target).

Required Test Data

  • One issue with ≥1 existing, correctly pinned comment (Scenario 6).
  • One fresh issue with no comments yet (Scenario 4/5 starting state).

Test Scenarios

Scenario 1: Comment posting succeeds on a uutils-style toolchain

Acceptance criterion: "Given a box whose date emits more than millisecond fractional
precision for %s%3N (uutils-style), when a comment is posted via local-fs, then the comment
is created with a correct millisecond pin … and the forge flow tests pass at pristine HEAD on
that box."
Lane: integration-covered — local-fs/test/shim-portability.sh (new suite delivered red-first
by this slice per the PREQ's regression-harness dependency; drives the post→scan roundtrip and the
four existing flow tests under a uutils-emulating date PATH shim)

  1. Arrange the toolchain so date emits full nanosecond digits where millisecond precision was
    requested (uutils emulation).
  2. Post a comment on an issue through the adapter.
  3. Verify: the comment is visible through the adapter's read path, with a readable creation
    timestamp of millisecond precision.
  4. Verify: the comment's edit state reports unedited.
  5. Run the four forge flow tests (smoke, disposition-flow, promotion-flow, release-flow)
    at pristine HEAD in this environment.
  6. Verify: all four pass.

Expected outcome: the quirky-but-capable toolchain is tolerated — posting works, timestamps
are correct, nothing downstream fails.

Scenario 2: Incapable box fails readiness loudly, naming the tool

Acceptance criterion: "Given a box whose tools cannot perform millisecond mtime pinning at
all, when the readiness guard runs with forge.adapter: local-fs declared, then the guard
reports the box not ready, naming the failing tool with its observed vs. expected behaviour…"
Lane: integration-covered — local-fs/test/shim-portability.sh (incapable-toolchain shim;
asserts probe_pin.sh exits nonzero with a message naming the tool, observed vs expected. The
guard step is model-executed markdown that relays the probe's exit/message verbatim — the suite
asserts the decision-bearing helper directly)

  1. Arrange the toolchain so millisecond mtime pinning is impossible (e.g. no fractional-second
    support at all).
  2. Run the readiness guard on a project declaring forge.adapter: local-fs.
  3. Verify: the guard reports not ready — never ready.
  4. Verify: the report names the failing tool and shows what was observed versus what was
    expected.

Expected outcome: one loud up-front message instead of mid-phase failures.

Scenario 2b (edge): Probe unable to execute also reports not-ready

Acceptance criterion: same criterion, probe-error clause: "If the probe itself cannot
execute (e.g. no writable temp location), the guard likewise reports not-ready — never ready."
Lane: integration-covered — local-fs/test/shim-portability.sh (unwritable-scratch case;
asserts probe nonzero exit + explicit message, never a ready verdict)

  1. Arrange the environment so the probe cannot perform its check at all (e.g. no writable
    location for a scratch file).
  2. Run the readiness guard with forge.adapter: local-fs declared.
  3. Verify: the guard reports not-ready (or an explicit error) — it does not report ready.

Expected outcome: an inconclusive probe is never treated as a pass.

Scenario 3: Probe verifies by doing — output shape alone cannot fool it

Acceptance criterion: "Given the local-fs readiness probe runs, then it verifies capability
by performing a real scratch-file mtime pin and reading it back — the same operation the adapter
performs…"
Lane: integration-covered — local-fs/test/shim-portability.sh (liar-touch shim that exits 0
without applying the mtime — probe must fail via readback; plus the healthy-box pass case)

  1. Arrange a toolchain whose command output is well-formed (would satisfy a shape check) but
    whose mtime pin does not actually take effect on the file.
  2. Run the readiness guard with forge.adapter: local-fs declared.
  3. Verify: the guard reports not ready — the probe caught the failure by performing the real
    operation, not by inspecting output shape.
  4. On a healthy box, run the guard and verify it reports ready.

Expected outcome: probe-pass demonstrates the pin operation itself succeeding on that box.

Scenario 4: Interrupted post leaves no visible partial comment

Acceptance criterion: "Given comment posting is interrupted at any step boundary before its
commit point (fault-injected at minimum: after number allocation, after a partial content write,
before the pin), when the issue's comments are subsequently read, then no partial comment is
visible…"
Lane: integration-covered — local-fs/test/crash.sh (new suite delivered red-first by this
slice; DEVWORK_LFS_FAULT injection at each of the three normative boundaries, then asserts
scan_comments shows nothing)

  1. On a fresh issue, begin posting a comment and interrupt it after number allocation.
  2. Read the issue's comments through the adapter's read path.
  3. Verify: no partial comment is visible — the record shows nothing from the interrupted attempt.
  4. Repeat with the interruption after a partial content write; verify the same.
  5. Repeat with the interruption after the full write but before the pin; verify the same.

Expected outcome: at every injection point, the record shows either a complete pinned comment
or nothing at all.

Scenario 5: The next post after an interruption succeeds cleanly

Acceptance criterion: "Given a prior posting attempt was interrupted, when the next comment
is posted on the same issue, then it succeeds cleanly — no residue-induced numbering anomaly, no
blocked or corrupted post."
Lane: integration-covered — local-fs/test/crash.sh (continues from each Scenario-4
interrupted state)

  1. Continue from any interrupted state produced in Scenario 4.
  2. Post a new comment on the same issue.
  3. Verify: the post succeeds; the comment is visible, complete, and correctly pinned.
  4. Verify: reading the issue's comments shows a coherent record — no gaps or duplicates
    attributable to the earlier interruption, and the new comment's ordering is correct.

Expected outcome: an earlier crash never poisons subsequent posts.

Scenario 6: An existing comment is never silently overwritten

Acceptance criterion: "Given a comment already exists, when any posting attempt (concurrent
or retried) would land on the same comment identity, then the existing comment is never silently
overwritten — concurrent posts either land as distinct comments or the loser fails loudly."
Lane: integration-covered — local-fs/test/crash.sh (pre-staged exact-identity collision →
loud loser, original byte-identical; concurrent-writer case → distinct comments)

  1. On an issue with an existing comment, record that comment's full content and timestamp.
  2. Contrive two posting attempts that would land on the same comment identity (e.g. two writers
    started from the same observed state).
  3. Let both attempts run to completion.
  4. Verify: the pre-existing comment's content and timestamp are unchanged.
  5. Verify: either both new posts are visible as distinct comments, or exactly one is visible and
    the other attempt reported a loud failure — never a silent replacement.

Expected outcome: no write ever silently destroys a record.

Scenario 7: A failed post reports failure to its caller

Acceptance criterion: "Given a posting attempt fails at or before its commit point (e.g.
permission denied, disk full), when the failure occurs, then the caller receives an explicit
error — it never proceeds believing the comment exists."
Lane: integration-covered — local-fs/test/crash.sh (unwritable comments dir; asserts nonzero
exit, stderr message, empty scan)

  1. Arrange the environment so the posting operation must fail (e.g. the target is unwritable).
  2. Attempt to post a comment and capture the operation's result as its caller sees it.
  3. Verify: the operation signals failure explicitly (non-success result with an error message).
  4. Verify: no comment from the failed attempt is visible through the adapter's read path.

Expected outcome: failure is loud and leaves nothing behind — the caller can react instead of
continuing on a phantom record.

Scenario 8: A never-edited comment reports unedited

Acceptance criterion: "Given a comment that was posted successfully and never edited, when
its edit state is checked, then it reports unedited (no false edit signal)."
Lane: integration-covered — local-fs/test/smoke.sh (existing; asserts
created_ms == edited_ms on a fresh post) and _shared/procedures/test/smoke.sh (existing;
immutability-detection step covers the edited-comment cross-check)

  1. Post a comment normally and do not touch it afterwards.
  2. Check the comment's edit state through the adapter.
  3. Verify: it reports unedited.
  4. (Cross-check) Edit a different comment and verify that one reports edited — the detector
    distinguishes the two.

Expected outcome: edit detection fires only on real edits.

Scenario 9: No regression on a GNU-coreutils box

Acceptance criterion: "Given a GNU-coreutils box, when the forge flow tests run at pristine
HEAD, then they still pass (no regression from the hardening)."
Lane: integration-covered — _shared/procedures/test/smoke.sh, disposition-flow.sh,
promotion-flow.sh, release-flow.sh (all existing; the GNU-baseline regression harness)

  1. On the GNU baseline box, run the four forge flow tests at pristine HEAD.
  2. Verify: all four pass.
  3. Post and read back a comment normally; verify correct pin and unedited state.

Expected outcome: the hardening changes nothing for the already-working environment.

Notes

  • "Visible" throughout means through the adapter's comment-read path (the forge contract's read
    operations); temp/staging residue at the raw filesystem level is acceptable provided
    Scenario 5 holds (per the PREQ's visibility definition).
  • Detection/cleanup of partial comments created before this feature is explicitly out of
    scope; no scenario covers it.
  • The uutils behaviour needs no real uutils box — a toolchain shim emulating the output is
    sufficient (PREQ Dependencies).
<!-- test-plan:v1 issue=26 skill=technical-plan --> # Test Plan: local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment ## Prerequisites [State the scenarios need — environment specifics (paths, shim contents) are verified at execution time, not guessed here.] - [ ] A scratch project workspace declaring `forge.adapter: local-fs`, with at least one issue that already carries at least one comment (needed for the no-overwrite scenario). - [ ] A box (or environment) with standard GNU coreutils — the regression baseline. - [ ] A way to present the suite with an altered toolchain (e.g. a `PATH` shim), able to emulate: (a) a `date` that ignores fractional-width specifiers and emits full nanosecond digits (uutils-style); (b) tools that cannot express or apply millisecond mtimes at all; (c) a toolchain whose *output looks well-formed* but whose mtime pin does not actually take effect. - [ ] A way to interrupt or fail a comment-posting operation partway through (fault injection at step boundaries), and a way to induce a write failure (e.g. an unwritable target). ### Required Test Data - [ ] One issue with ≥1 existing, correctly pinned comment (Scenario 6). - [ ] One fresh issue with no comments yet (Scenario 4/5 starting state). ## Test Scenarios ### Scenario 1: Comment posting succeeds on a uutils-style toolchain **Acceptance criterion:** "Given a box whose `date` emits more than millisecond fractional precision for `%s%3N` (uutils-style), when a comment is posted via `local-fs`, then the comment is created with a correct millisecond pin … and the forge flow tests pass at pristine HEAD on that box." **Lane:** integration-covered — `local-fs/test/shim-portability.sh` (new suite delivered red-first by this slice per the PREQ's regression-harness dependency; drives the post→scan roundtrip and the four existing flow tests under a uutils-emulating `date` PATH shim) 1. Arrange the toolchain so `date` emits full nanosecond digits where millisecond precision was requested (uutils emulation). 2. Post a comment on an issue through the adapter. 3. Verify: the comment is visible through the adapter's read path, with a readable creation timestamp of millisecond precision. 4. Verify: the comment's edit state reports **unedited**. 5. Run the four forge flow tests (`smoke`, `disposition-flow`, `promotion-flow`, `release-flow`) at pristine HEAD in this environment. 6. Verify: all four pass. **Expected outcome:** the quirky-but-capable toolchain is tolerated — posting works, timestamps are correct, nothing downstream fails. ### Scenario 2: Incapable box fails readiness loudly, naming the tool **Acceptance criterion:** "Given a box whose tools cannot perform millisecond mtime pinning at all, when the readiness guard runs with `forge.adapter: local-fs` declared, then the guard reports the box **not ready**, naming the failing tool with its observed vs. expected behaviour…" **Lane:** integration-covered — `local-fs/test/shim-portability.sh` (incapable-toolchain shim; asserts `probe_pin.sh` exits nonzero with a message naming the tool, observed vs expected. The guard step is model-executed markdown that relays the probe's exit/message verbatim — the suite asserts the decision-bearing helper directly) 1. Arrange the toolchain so millisecond mtime pinning is impossible (e.g. no fractional-second support at all). 2. Run the readiness guard on a project declaring `forge.adapter: local-fs`. 3. Verify: the guard reports **not ready** — never ready. 4. Verify: the report names the failing tool and shows what was observed versus what was expected. **Expected outcome:** one loud up-front message instead of mid-phase failures. ### Scenario 2b (edge): Probe unable to execute also reports not-ready **Acceptance criterion:** same criterion, probe-error clause: "If the probe itself cannot execute (e.g. no writable temp location), the guard likewise reports not-ready — never ready." **Lane:** integration-covered — `local-fs/test/shim-portability.sh` (unwritable-scratch case; asserts probe nonzero exit + explicit message, never a ready verdict) 1. Arrange the environment so the probe cannot perform its check at all (e.g. no writable location for a scratch file). 2. Run the readiness guard with `forge.adapter: local-fs` declared. 3. Verify: the guard reports not-ready (or an explicit error) — it does not report ready. **Expected outcome:** an inconclusive probe is never treated as a pass. ### Scenario 3: Probe verifies by doing — output shape alone cannot fool it **Acceptance criterion:** "Given the `local-fs` readiness probe runs, then it verifies capability by performing a real scratch-file mtime pin and reading it back — the same operation the adapter performs…" **Lane:** integration-covered — `local-fs/test/shim-portability.sh` (liar-`touch` shim that exits 0 without applying the mtime — probe must fail via readback; plus the healthy-box pass case) 1. Arrange a toolchain whose command *output* is well-formed (would satisfy a shape check) but whose mtime pin does not actually take effect on the file. 2. Run the readiness guard with `forge.adapter: local-fs` declared. 3. Verify: the guard reports **not ready** — the probe caught the failure by performing the real operation, not by inspecting output shape. 4. On a healthy box, run the guard and verify it reports ready. **Expected outcome:** probe-pass demonstrates the pin operation itself succeeding on that box. ### Scenario 4: Interrupted post leaves no visible partial comment **Acceptance criterion:** "Given comment posting is interrupted at any step boundary before its commit point (fault-injected at minimum: after number allocation, after a partial content write, before the pin), when the issue's comments are subsequently read, then no partial comment is visible…" **Lane:** integration-covered — `local-fs/test/crash.sh` (new suite delivered red-first by this slice; `DEVWORK_LFS_FAULT` injection at each of the three normative boundaries, then asserts `scan_comments` shows nothing) 1. On a fresh issue, begin posting a comment and interrupt it **after number allocation**. 2. Read the issue's comments through the adapter's read path. 3. Verify: no partial comment is visible — the record shows nothing from the interrupted attempt. 4. Repeat with the interruption **after a partial content write**; verify the same. 5. Repeat with the interruption **after the full write but before the pin**; verify the same. **Expected outcome:** at every injection point, the record shows either a complete pinned comment or nothing at all. ### Scenario 5: The next post after an interruption succeeds cleanly **Acceptance criterion:** "Given a prior posting attempt was interrupted, when the next comment is posted on the same issue, then it succeeds cleanly — no residue-induced numbering anomaly, no blocked or corrupted post." **Lane:** integration-covered — `local-fs/test/crash.sh` (continues from each Scenario-4 interrupted state) 1. Continue from any interrupted state produced in Scenario 4. 2. Post a new comment on the same issue. 3. Verify: the post succeeds; the comment is visible, complete, and correctly pinned. 4. Verify: reading the issue's comments shows a coherent record — no gaps or duplicates attributable to the earlier interruption, and the new comment's ordering is correct. **Expected outcome:** an earlier crash never poisons subsequent posts. ### Scenario 6: An existing comment is never silently overwritten **Acceptance criterion:** "Given a comment already exists, when any posting attempt (concurrent or retried) would land on the same comment identity, then the existing comment is never silently overwritten — concurrent posts either land as distinct comments or the loser fails loudly." **Lane:** integration-covered — `local-fs/test/crash.sh` (pre-staged exact-identity collision → loud loser, original byte-identical; concurrent-writer case → distinct comments) 1. On an issue with an existing comment, record that comment's full content and timestamp. 2. Contrive two posting attempts that would land on the same comment identity (e.g. two writers started from the same observed state). 3. Let both attempts run to completion. 4. Verify: the pre-existing comment's content and timestamp are unchanged. 5. Verify: either both new posts are visible as distinct comments, or exactly one is visible and the other attempt reported a loud failure — never a silent replacement. **Expected outcome:** no write ever silently destroys a record. ### Scenario 7: A failed post reports failure to its caller **Acceptance criterion:** "Given a posting attempt fails at or before its commit point (e.g. permission denied, disk full), when the failure occurs, then the caller receives an explicit error — it never proceeds believing the comment exists." **Lane:** integration-covered — `local-fs/test/crash.sh` (unwritable comments dir; asserts nonzero exit, stderr message, empty scan) 1. Arrange the environment so the posting operation must fail (e.g. the target is unwritable). 2. Attempt to post a comment and capture the operation's result as its caller sees it. 3. Verify: the operation signals failure explicitly (non-success result with an error message). 4. Verify: no comment from the failed attempt is visible through the adapter's read path. **Expected outcome:** failure is loud and leaves nothing behind — the caller can react instead of continuing on a phantom record. ### Scenario 8: A never-edited comment reports unedited **Acceptance criterion:** "Given a comment that was posted successfully and never edited, when its edit state is checked, then it reports unedited (no false edit signal)." **Lane:** integration-covered — `local-fs/test/smoke.sh` (existing; asserts `created_ms == edited_ms` on a fresh post) and `_shared/procedures/test/smoke.sh` (existing; immutability-detection step covers the edited-comment cross-check) 1. Post a comment normally and do not touch it afterwards. 2. Check the comment's edit state through the adapter. 3. Verify: it reports unedited. 4. (Cross-check) Edit a *different* comment and verify that one reports edited — the detector distinguishes the two. **Expected outcome:** edit detection fires only on real edits. ### Scenario 9: No regression on a GNU-coreutils box **Acceptance criterion:** "Given a GNU-coreutils box, when the forge flow tests run at pristine HEAD, then they still pass (no regression from the hardening)." **Lane:** integration-covered — `_shared/procedures/test/smoke.sh`, `disposition-flow.sh`, `promotion-flow.sh`, `release-flow.sh` (all existing; the GNU-baseline regression harness) 1. On the GNU baseline box, run the four forge flow tests at pristine HEAD. 2. Verify: all four pass. 3. Post and read back a comment normally; verify correct pin and unedited state. **Expected outcome:** the hardening changes nothing for the already-working environment. ## Notes - "Visible" throughout means through the adapter's comment-read path (the forge contract's read operations); temp/staging residue at the raw filesystem level is acceptable provided Scenario 5 holds (per the PREQ's visibility definition). - Detection/cleanup of partial comments created *before* this feature is explicitly out of scope; no scenario covers it. - The uutils behaviour needs no real uutils box — a toolchain shim emulating the output is sufficient (PREQ Dependencies).
jbr870 commented 2026-08-13 08:07:07 +00:00
Author
Owner
Copy link
{
  "next_state": "developing",
  "produced": [
    {
      "kind": "sreq",
      "ref": "comment:724",
      "summary": "SREQ for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment (Tier 2)"
    },
    {
      "kind": "test-plan",
      "ref": "comment:725",
      "summary": "Test plan for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment — one+ scenario per acceptance criterion, all 10 lanes integration-covered with named suites"
    }
  ],
  "findings": [
    {
      "category": "out-of-scope",
      "summary": "Boxes that genuinely cannot pin millisecond mtimes stay unsupported; capability is defined behaviorally (passing the probe pin-and-read-back), not by GNU provenance — uutils qualifies once hardening lands",
      "reasoning": "Flagged as out of scope in the approved PREQ. Permanent policy: the fix for an incapable box is honest detection, not dropping the local-fs prerequisite.",
      "proposed_action": "accept",
      "target": null,
      "id": "F-PO-26-2-1"
    },
    {
      "category": "out-of-scope",
      "summary": "Detecting or cleaning up pre-existing partial comments (e.g. the axana orphan)",
      "reasoning": "Flagged as out of scope in the approved PREQ. One-off manual cleanup; this feature prevents new corruption, it does not remediate old records.",
      "proposed_action": "accept",
      "target": null,
      "id": "F-PO-26-2-2"
    },
    {
      "category": "out-of-scope",
      "summary": "Changing the forge contract created_ms / edit-detection semantics",
      "reasoning": "Flagged as out of scope in the approved PREQ. The mechanism stays; only its reliability changes.",
      "proposed_action": "accept",
      "target": null,
      "id": "F-PO-26-2-3"
    },
    {
      "category": "out-of-scope",
      "summary": "Other adapters (tea-cli, glab-cli, gh-cli)",
      "reasoning": "Flagged as out of scope in the approved PREQ. They do not pin mtimes; the defect class is local-fs-only.",
      "proposed_action": "accept",
      "target": null,
      "id": "F-PO-26-2-4"
    },
    {
      "category": "out-of-scope",
      "summary": "Atomicity for local-fs write paths other than comment posting (issue creation, body edits)",
      "reasoning": "Flagged as out of scope in the approved PREQ. Known follow-up candidate if the same pattern is observed on those paths — a sibling issue keeps it visible.",
      "proposed_action": "defer-to-issue",
      "target": null,
      "id": "F-PO-26-2-5"
    },
    {
      "category": "out-of-scope",
      "summary": "Full lock-based serialization of concurrent writers",
      "reasoning": "Flagged as out of scope in the approved PREQ. The chosen guarantee is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success.",
      "proposed_action": "accept",
      "target": null,
      "id": "F-PO-26-2-6"
    },
    {
      "category": "in-scope-deferrable",
      "summary": "Legacy partial comments written by the old direct-write protocol remain visible through the unchanged read path (expert-review note, Solution Architect)",
      "reasoning": "Reviewer suggestion acknowledged but not actioned in this SREQ — see Expert Review > Noted (not actioned). Already covered by the PREQ out-of-scope decision on pre-existing partial cleanup (same item as finding F#2); no new work or ticket needed beyond that disposition.",
      "proposed_action": "accept",
      "id": "F-PO-26-2-7"
    }
  ],
  "pending_decisions": [
    {
      "id": "D-PO-26-2-1",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Boxes that genuinely cannot pin millisecond mtimes stay unsupported; capability is defined behaviorally (passing the probe pin-and-read-back), not by GNU provenance — uutils qualifies once hardening lands'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-1",
      "reasoning": "Flagged as out of scope in the approved PREQ. Permanent policy: the fix for an incapable box is honest detection, not dropping the local-fs prerequisite."
    },
    {
      "id": "D-PO-26-2-2",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Detecting or cleaning up pre-existing partial comments (e.g. the axana orphan)'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-2",
      "reasoning": "Flagged as out of scope in the approved PREQ. One-off manual cleanup; this feature prevents new corruption, it does not remediate old records."
    },
    {
      "id": "D-PO-26-2-3",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Changing the forge contract created_ms / edit-detection semantics'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-3",
      "reasoning": "Flagged as out of scope in the approved PREQ. The mechanism stays; only its reliability changes."
    },
    {
      "id": "D-PO-26-2-4",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Other adapters (tea-cli, glab-cli, gh-cli)'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-4",
      "reasoning": "Flagged as out of scope in the approved PREQ. They do not pin mtimes; the defect class is local-fs-only."
    },
    {
      "id": "D-PO-26-2-5",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Atomicity for local-fs write paths other than comment posting (issue creation, body edits)'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "defer-to-issue",
      "finding_ref": "F-PO-26-2-5",
      "reasoning": "Flagged as out of scope in the approved PREQ. Known follow-up candidate if the same pattern is observed on those paths — a sibling issue keeps it visible."
    },
    {
      "id": "D-PO-26-2-6",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Out of scope: 'Full lock-based serialization of concurrent writers'. Spawn a sibling issue, or accept (no follow-up)?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-6",
      "reasoning": "Flagged as out of scope in the approved PREQ. The chosen guarantee is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success."
    },
    {
      "id": "D-PO-26-2-7",
      "type": "scope-disposition",
      "blocking": false,
      "question": "Expert-review note (not actioned): 'Legacy partial comments written by the old direct-write protocol remain visible through the unchanged read path (expert-review note, Solution Architect)'. Spawn a sibling issue, or accept the disposition?",
      "options": [
        "defer-to-issue",
        "accept"
      ],
      "recommended": "accept",
      "finding_ref": "F-PO-26-2-7",
      "reasoning": "Reviewer suggestion acknowledged but not actioned in this SREQ — see Expert Review > Noted (not actioned). Already covered by the PREQ out-of-scope decision on pre-existing partial cleanup (same item as finding F#2); no new work or ticket needed beyond that disposition."
    }
  ]
}
<!-- phase-outcome:v1 id=PO-26-2 skill=technical-plan --> ```json { "next_state": "developing", "produced": [ { "kind": "sreq", "ref": "comment:724", "summary": "SREQ for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment (Tier 2)" }, { "kind": "test-plan", "ref": "comment:725", "summary": "Test plan for local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment — one+ scenario per acceptance criterion, all 10 lanes integration-covered with named suites" } ], "findings": [ { "category": "out-of-scope", "summary": "Boxes that genuinely cannot pin millisecond mtimes stay unsupported; capability is defined behaviorally (passing the probe pin-and-read-back), not by GNU provenance — uutils qualifies once hardening lands", "reasoning": "Flagged as out of scope in the approved PREQ. Permanent policy: the fix for an incapable box is honest detection, not dropping the local-fs prerequisite.", "proposed_action": "accept", "target": null, "id": "F-PO-26-2-1" }, { "category": "out-of-scope", "summary": "Detecting or cleaning up pre-existing partial comments (e.g. the axana orphan)", "reasoning": "Flagged as out of scope in the approved PREQ. One-off manual cleanup; this feature prevents new corruption, it does not remediate old records.", "proposed_action": "accept", "target": null, "id": "F-PO-26-2-2" }, { "category": "out-of-scope", "summary": "Changing the forge contract created_ms / edit-detection semantics", "reasoning": "Flagged as out of scope in the approved PREQ. The mechanism stays; only its reliability changes.", "proposed_action": "accept", "target": null, "id": "F-PO-26-2-3" }, { "category": "out-of-scope", "summary": "Other adapters (tea-cli, glab-cli, gh-cli)", "reasoning": "Flagged as out of scope in the approved PREQ. They do not pin mtimes; the defect class is local-fs-only.", "proposed_action": "accept", "target": null, "id": "F-PO-26-2-4" }, { "category": "out-of-scope", "summary": "Atomicity for local-fs write paths other than comment posting (issue creation, body edits)", "reasoning": "Flagged as out of scope in the approved PREQ. Known follow-up candidate if the same pattern is observed on those paths — a sibling issue keeps it visible.", "proposed_action": "defer-to-issue", "target": null, "id": "F-PO-26-2-5" }, { "category": "out-of-scope", "summary": "Full lock-based serialization of concurrent writers", "reasoning": "Flagged as out of scope in the approved PREQ. The chosen guarantee is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success.", "proposed_action": "accept", "target": null, "id": "F-PO-26-2-6" }, { "category": "in-scope-deferrable", "summary": "Legacy partial comments written by the old direct-write protocol remain visible through the unchanged read path (expert-review note, Solution Architect)", "reasoning": "Reviewer suggestion acknowledged but not actioned in this SREQ — see Expert Review > Noted (not actioned). Already covered by the PREQ out-of-scope decision on pre-existing partial cleanup (same item as finding F#2); no new work or ticket needed beyond that disposition.", "proposed_action": "accept", "id": "F-PO-26-2-7" } ], "pending_decisions": [ { "id": "D-PO-26-2-1", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Boxes that genuinely cannot pin millisecond mtimes stay unsupported; capability is defined behaviorally (passing the probe pin-and-read-back), not by GNU provenance — uutils qualifies once hardening lands'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-1", "reasoning": "Flagged as out of scope in the approved PREQ. Permanent policy: the fix for an incapable box is honest detection, not dropping the local-fs prerequisite." }, { "id": "D-PO-26-2-2", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Detecting or cleaning up pre-existing partial comments (e.g. the axana orphan)'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-2", "reasoning": "Flagged as out of scope in the approved PREQ. One-off manual cleanup; this feature prevents new corruption, it does not remediate old records." }, { "id": "D-PO-26-2-3", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Changing the forge contract created_ms / edit-detection semantics'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-3", "reasoning": "Flagged as out of scope in the approved PREQ. The mechanism stays; only its reliability changes." }, { "id": "D-PO-26-2-4", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Other adapters (tea-cli, glab-cli, gh-cli)'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-4", "reasoning": "Flagged as out of scope in the approved PREQ. They do not pin mtimes; the defect class is local-fs-only." }, { "id": "D-PO-26-2-5", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Atomicity for local-fs write paths other than comment posting (issue creation, body edits)'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "defer-to-issue", "finding_ref": "F-PO-26-2-5", "reasoning": "Flagged as out of scope in the approved PREQ. Known follow-up candidate if the same pattern is observed on those paths — a sibling issue keeps it visible." }, { "id": "D-PO-26-2-6", "type": "scope-disposition", "blocking": false, "question": "Out of scope: 'Full lock-based serialization of concurrent writers'. Spawn a sibling issue, or accept (no follow-up)?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-6", "reasoning": "Flagged as out of scope in the approved PREQ. The chosen guarantee is no-silent-overwrite (loud failure acceptable for the losing writer), not universal concurrent success." }, { "id": "D-PO-26-2-7", "type": "scope-disposition", "blocking": false, "question": "Expert-review note (not actioned): 'Legacy partial comments written by the old direct-write protocol remain visible through the unchanged read path (expert-review note, Solution Architect)'. Spawn a sibling issue, or accept the disposition?", "options": [ "defer-to-issue", "accept" ], "recommended": "accept", "finding_ref": "F-PO-26-2-7", "reasoning": "Reviewer suggestion acknowledged but not actioned in this SREQ — see Expert Review > Noted (not actioned). Already covered by the PREQ out-of-scope decision on pre-existing partial cleanup (same item as finding F#2); no new work or ticket needed beyond that disposition." } ] } ```
jbr870 commented 2026-08-13 08:17:46 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "The support boundary is behavioral by design: a box that cannot pass the probe pin-and-read-back gets honest not-ready detection, which is this feature's deliverable — dropping the prerequisite instead would silently break edit detection. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-1 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "The support boundary is behavioral by design: a box that cannot pass the probe pin-and-read-back gets honest not-ready detection, which is this feature's deliverable — dropping the prerequisite instead would silently break edit detection. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:17:46 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "Pre-existing partials (e.g. the axana orphan) are a bounded one-off manual cleanup; once posting is atomic no new ones are produced, so there is no recurring mechanism to build. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-2 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "Pre-existing partials (e.g. the axana orphan) are a bounded one-off manual cleanup; once posting is atomic no new ones are produced, so there is no recurring mechanism to build. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:17:46 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "created_ms/edit-detection semantics are exactly the contract this feature makes reliable; changing them would be a different feature with cross-adapter impact and no defect motivating it. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-3 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "created_ms/edit-detection semantics are exactly the contract this feature makes reliable; changing them would be a different feature with cross-adapter impact and no defect motivating it. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:17:47 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "tea-cli/glab-cli/gh-cli delegate timestamps and write atomicity to their forge servers and never pin mtimes, so the defect class does not exist there — no sibling work to track. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-4 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "tea-cli/glab-cli/gh-cli delegate timestamps and write atomicity to their forge servers and never pin mtimes, so the defect class does not exist there — no sibling work to track. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:17:47 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "The contract guarantee deliberately chosen is no-silent-overwrite with a loud loser, not universal concurrent success; full lock serialization would add lock-liveness failure modes (stale locks blocking all posting) for a writer population that is rare and human-paced. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-6 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "The contract guarantee deliberately chosen is no-silent-overwrite with a loud loser, not universal concurrent success; full lock serialization would add lock-liveness failure modes (stale locks blocking all posting) for a writer population that is rare and human-paced. Grounded in the approved PREQ's out-of-scope list; operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:17:47 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "accept",
  "rationale": "Duplicate coverage: legacy partial visibility is the same item as finding F-PO-26-2-2, already accepted out of scope under the PREQ's pre-existing-partials decision — a second record would fork the disposition trail without adding work. Operator directed this session that the recommended dispositions be taken."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-7 --> ```json { "status": "resolved", "chosen_option": "accept", "rationale": "Duplicate coverage: legacy partial visibility is the same item as finding F-PO-26-2-2, already accepted out of scope under the PREQ's pre-existing-partials decision — a second record would fork the disposition trail without adding work. Operator directed this session that the recommended dispositions be taken." } ```
jbr870 commented 2026-08-13 08:18:01 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "defer-to-issue",
  "rationale": "Issue creation, body edits, meta.json and seqfile updates still write directly to their final visible paths, so the crash-visibility class this feature closes for comments remains open there — lower blast radius (no cross-record identity collision), but the PREQ itself names it a known follow-up candidate. A sibling issue keeps it visible in the backlog instead of evaporating when #26 closes. Operator directed this session that the recommended dispositions be taken.",
  "rejected_alternative": "accept — rejected because silence would lose the only durable pointer to a defect class already observed once on the comment path; the cost of a low-priority stub is trivial next to re-discovering it in production."
}
<!-- decision-resolution:v1 ref=D-PO-26-2-5 --> ```json { "status": "resolved", "chosen_option": "defer-to-issue", "rationale": "Issue creation, body edits, meta.json and seqfile updates still write directly to their final visible paths, so the crash-visibility class this feature closes for comments remains open there — lower blast radius (no cross-record identity collision), but the PREQ itself names it a known follow-up candidate. A sibling issue keeps it visible in the backlog instead of evaporating when #26 closes. Operator directed this session that the recommended dispositions be taken.", "rejected_alternative": "accept — rejected because silence would lose the only durable pointer to a defect class already observed once on the comment path; the cost of a low-priority stub is trivial next to re-discovering it in production." } ```
jbr870 commented 2026-08-13 08:18:10 +00:00
Author
Owner
Copy link

Linked: this issue is sibling #27 (recorded by the devwork pipeline).

Linked: this issue is **sibling** #27 (recorded by the devwork pipeline).
jbr870 commented 2026-08-13 08:37:03 +00:00
Author
Owner
Copy link
{
  "next_state": "qa",
  "produced": [
    {
      "kind": "code-bundle",
      "ref": "commit:2e58030",
      "summary": "local-fs comment-integrity implementation on feature/26-…: normalized ms clock (write+read), _lfs_commit_comment atomic stage->pin->no-clobber-link protocol with files-authority seq allocation and 15-min stale-stage GC, behavioral per-op clock guard, probe_pin.sh probe-by-doing readiness helper, both posting scripts routed through the primitive, normative SKILL.md/readiness-check.md/CLAUDE.md updates. All 10 ACs verified. Notes: (1) DEVWORK_LFS_FORCE_SEQ added as a normative test-emulation hook beyond the three SREQ fault points — needed to make the AC-6 collision path deterministic; (2) AC-9 regression ran on this box's real uutils 0.8.0 toolchain (which reproduced the defect at pristine HEAD) rather than a literal GNU box — the new code path calls only %s.%N/-r/-d/ln/find semantics identical on GNU; (3) executed outside /develop by operator direction, TDD red-first per the annotated test plan."
    },
    {
      "kind": "test-results",
      "ref": "commit:2e58030",
      "summary": "crash.sh + shim-portability.sh (new, red-first, green at commit); local-fs smoke, shared smoke, disposition-flow, promotion-flow, release-flow, slot-flow all pass; bash -n gate over all helper/adapter bin scripts passes; flow tests additionally pass under the uutils PATH shim per AC-1."
    }
  ],
  "findings": [],
  "pending_decisions": []
}
<!-- phase-outcome:v1 id=PO-26-3 skill=develop --> ```json { "next_state": "qa", "produced": [ { "kind": "code-bundle", "ref": "commit:2e58030", "summary": "local-fs comment-integrity implementation on feature/26-…: normalized ms clock (write+read), _lfs_commit_comment atomic stage->pin->no-clobber-link protocol with files-authority seq allocation and 15-min stale-stage GC, behavioral per-op clock guard, probe_pin.sh probe-by-doing readiness helper, both posting scripts routed through the primitive, normative SKILL.md/readiness-check.md/CLAUDE.md updates. All 10 ACs verified. Notes: (1) DEVWORK_LFS_FORCE_SEQ added as a normative test-emulation hook beyond the three SREQ fault points — needed to make the AC-6 collision path deterministic; (2) AC-9 regression ran on this box's real uutils 0.8.0 toolchain (which reproduced the defect at pristine HEAD) rather than a literal GNU box — the new code path calls only %s.%N/-r/-d/ln/find semantics identical on GNU; (3) executed outside /develop by operator direction, TDD red-first per the annotated test plan." }, { "kind": "test-results", "ref": "commit:2e58030", "summary": "crash.sh + shim-portability.sh (new, red-first, green at commit); local-fs smoke, shared smoke, disposition-flow, promotion-flow, release-flow, slot-flow all pass; bash -n gate over all helper/adapter bin scripts passes; flow tests additionally pass under the uutils PATH shim per AC-1." } ], "findings": [], "pending_decisions": [] } ```
jbr870 commented 2026-08-13 08:47:11 +00:00
Author
Owner
Copy link
{
  "outcome": "skipped",
  "summary": "domain e2e skipped: not applicable to this project",
  "findings": [],
  "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): markdown+shell skill suite — no application or browser surface to drive end-to-end; validation is the shell test harness plus dogfooding"
}
<!-- qa-report:v1 issue=26 skill=qa domain=e2e phase=validate --> <!-- qa-report:v1 domain=e2e phase=validate --> ```json { "outcome": "skipped", "summary": "domain e2e skipped: not applicable to this project", "findings": [], "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): markdown+shell skill suite — no application or browser surface to drive end-to-end; validation is the shell test harness plus dogfooding" } ```
jbr870 commented 2026-08-13 08:47:11 +00:00
Author
Owner
Copy link
{
  "outcome": "skipped",
  "summary": "domain a11y skipped: not applicable to this project",
  "findings": [],
  "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): no rendered UI — the deliverables are markdown skill text and shell helpers"
}
<!-- qa-report:v1 issue=26 skill=qa domain=a11y phase=validate --> <!-- qa-report:v1 domain=a11y phase=validate --> ```json { "outcome": "skipped", "summary": "domain a11y skipped: not applicable to this project", "findings": [], "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): no rendered UI — the deliverables are markdown skill text and shell helpers" } ```
jbr870 commented 2026-08-13 08:47:12 +00:00
Author
Owner
Copy link
{
  "outcome": "skipped",
  "summary": "domain security-browser skipped: not applicable to this project",
  "findings": [],
  "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): no browser surface — nothing is served or rendered"
}
<!-- qa-report:v1 issue=26 skill=qa domain=security-browser phase=validate --> <!-- qa-report:v1 domain=security-browser phase=validate --> ```json { "outcome": "skipped", "summary": "domain security-browser skipped: not applicable to this project", "findings": [], "not_applicable_reason": "Project declaration (CLAUDE.md qa_domains): no browser surface — nothing is served or rendered" } ```
jbr870 commented 2026-08-13 08:47:12 +00:00
Author
Owner
Copy link
{
  "outcome": "skipped",
  "summary": "domain api skipped: not applicable to this project",
  "findings": [],
  "not_applicable_reason": "Project declares api_invocation: mode: none (no API — markdown+shell skill suite)"
}
<!-- qa-report:v1 issue=26 skill=qa domain=api phase=validate --> <!-- qa-report:v1 domain=api phase=validate --> ```json { "outcome": "skipped", "summary": "domain api skipped: not applicable to this project", "findings": [], "not_applicable_reason": "Project declares api_invocation: mode: none (no API — markdown+shell skill suite)" } ```
jbr870 commented 2026-08-13 08:47:12 +00:00
Author
Owner
Copy link
{
  "outcome": "skipped",
  "summary": "domain security-api skipped: not applicable to this project",
  "findings": [],
  "not_applicable_reason": "Project declares api_invocation: mode: none — no API endpoints exist to secure"
}
<!-- qa-report:v1 issue=26 skill=qa domain=security-api phase=validate --> <!-- qa-report:v1 domain=security-api phase=validate --> ```json { "outcome": "skipped", "summary": "domain security-api skipped: not applicable to this project", "findings": [], "not_applicable_reason": "Project declares api_invocation: mode: none — no API endpoints exist to secure" } ```
jbr870 commented 2026-08-13 09:04:21 +00:00
Author
Owner
Copy link
{
  "outcome": "issues-found",
  "summary": "Independent adversarial review of commit 2e58030 (fresh-context reviewer agent): 5 findings — 2 blocking (set -e aborting a post after its commit point; unresolvable $LOCAL_FS_BIN in the readiness snippet), 3 deferrable test/robustness gaps. All confirmed by reproduction or verified failure sequence.",
  "findings": [
    {
      "id": "CR-1",
      "category": "in-scope-blocking",
      "severity": "serious",
      "summary": "post_comment.sh meta refresh: bare tmp=$(mktemp) under set -e aborts post-commit — a durable comment is reported as a failed post (duplicating-retry hazard)",
      "reasoning": "Reproduced: unwritable TMPDIR -> rc=1, no comment_id JSON, yet the comment is committed and pinned. Violates the SREQ best-effort-after-commit decision."
    },
    {
      "id": "CR-2",
      "category": "in-scope-blocking",
      "severity": "moderate",
      "summary": "readiness-check.md Step 0 snippet invokes $LOCAL_FS_BIN/probe_pin.sh but LOCAL_FS_BIN is assigned nowhere — false not-ready on a capable box",
      "reasoning": "Only occurrence in the repo is the use site; run as written the snippet 127s and reports SUITE PREREQS MISSING on a box where the probe passes."
    },
    {
      "id": "CR-3",
      "category": "in-scope-deferrable",
      "severity": "minor",
      "summary": "crash.sh AC-4 visibility assertion is header-filtered — blind to a truncated partial that dodges the substring match",
      "reasoning": "head -c 8 partial (<!-- cras) passes the <!-- prefix test but fails the pattern substring, so scan_count reads 0 either way; suite would still fail via the numbering check, but the assertion naming the guarantee is tautological for that arm."
    },
    {
      "id": "CR-4",
      "category": "in-scope-deferrable",
      "severity": "moderate",
      "summary": "less-than-ms clock branch of _lfs_ms_norm untested, and probe_pin.sh skips the 13-digit plausibility check — a centisecond clock would probe ready with wrong timestamps",
      "reasoning": "Deleting the length guard kept every test green; probe path never validates the normalized shape, so the only defense was unexercised."
    },
    {
      "id": "CR-5",
      "category": "in-scope-deferrable",
      "severity": "minor",
      "summary": "probe scratch defaults to the project root with no gitignore coverage — an untrapped kill leaves an untracked file that stalls clean-tree checks",
      "reasoning": "SIGKILL/Ctrl-C at guard time skips the EXIT trap; .probe-pin.* matched no ignore pattern and dirties git status read by /integrate."
    }
  ],
  "artifacts": {
    "report_path": "review delivered as structured findings by agent qa-code-reviewer; verified findings recorded here",
    "test_files": [
      ".claude/skills/local-fs/test/crash.sh",
      ".claude/skills/local-fs/test/shim-portability.sh"
    ]
  }
}
<!-- qa-report:v1 issue=26 skill=qa domain=code phase=validate --> <!-- qa-report:v1 domain=code phase=validate --> ```json { "outcome": "issues-found", "summary": "Independent adversarial review of commit 2e58030 (fresh-context reviewer agent): 5 findings — 2 blocking (set -e aborting a post after its commit point; unresolvable $LOCAL_FS_BIN in the readiness snippet), 3 deferrable test/robustness gaps. All confirmed by reproduction or verified failure sequence.", "findings": [ { "id": "CR-1", "category": "in-scope-blocking", "severity": "serious", "summary": "post_comment.sh meta refresh: bare tmp=$(mktemp) under set -e aborts post-commit — a durable comment is reported as a failed post (duplicating-retry hazard)", "reasoning": "Reproduced: unwritable TMPDIR -> rc=1, no comment_id JSON, yet the comment is committed and pinned. Violates the SREQ best-effort-after-commit decision." }, { "id": "CR-2", "category": "in-scope-blocking", "severity": "moderate", "summary": "readiness-check.md Step 0 snippet invokes $LOCAL_FS_BIN/probe_pin.sh but LOCAL_FS_BIN is assigned nowhere — false not-ready on a capable box", "reasoning": "Only occurrence in the repo is the use site; run as written the snippet 127s and reports SUITE PREREQS MISSING on a box where the probe passes." }, { "id": "CR-3", "category": "in-scope-deferrable", "severity": "minor", "summary": "crash.sh AC-4 visibility assertion is header-filtered — blind to a truncated partial that dodges the substring match", "reasoning": "head -c 8 partial (<!-- cras) passes the <!-- prefix test but fails the pattern substring, so scan_count reads 0 either way; suite would still fail via the numbering check, but the assertion naming the guarantee is tautological for that arm." }, { "id": "CR-4", "category": "in-scope-deferrable", "severity": "moderate", "summary": "less-than-ms clock branch of _lfs_ms_norm untested, and probe_pin.sh skips the 13-digit plausibility check — a centisecond clock would probe ready with wrong timestamps", "reasoning": "Deleting the length guard kept every test green; probe path never validates the normalized shape, so the only defense was unexercised." }, { "id": "CR-5", "category": "in-scope-deferrable", "severity": "minor", "summary": "probe scratch defaults to the project root with no gitignore coverage — an untrapped kill leaves an untracked file that stalls clean-tree checks", "reasoning": "SIGKILL/Ctrl-C at guard time skips the EXIT trap; .probe-pin.* matched no ignore pattern and dirties git status read by /integrate." } ], "artifacts": { "report_path": "review delivered as structured findings by agent qa-code-reviewer; verified findings recorded here", "test_files": [ ".claude/skills/local-fs/test/crash.sh", ".claude/skills/local-fs/test/shim-portability.sh" ] } } ```
jbr870 commented 2026-08-13 09:04:21 +00:00
Author
Owner
Copy link
{
  "outcome": "clean",
  "summary": "All 5 findings fixed in commit b9d3cf7 (cost x value: every fix trivial and adjacent, so fix-now over deferral). CR-1 got a dedicated regression test (unwritable TMPDIR must not fail a committed post); CR-4 got a centisecond-clock shim asserting probe and post both refuse; CR-3 a raw-glob backstop per fault point; CR-2 a self-resolving POSIX snippet; CR-5 scratch under .devwork/ plus 15-min residue self-clean. All eight suites + bash -n gate green after fixes.",
  "findings": [],
  "artifacts": {
    "test_commit": "b9d3cf7",
    "test_files": [
      ".claude/skills/local-fs/test/crash.sh",
      ".claude/skills/local-fs/test/shim-portability.sh"
    ]
  }
}
<!-- qa-report:v1 issue=26 skill=qa domain=code phase=fix --> <!-- qa-report:v1 domain=code phase=fix --> ```json { "outcome": "clean", "summary": "All 5 findings fixed in commit b9d3cf7 (cost x value: every fix trivial and adjacent, so fix-now over deferral). CR-1 got a dedicated regression test (unwritable TMPDIR must not fail a committed post); CR-4 got a centisecond-clock shim asserting probe and post both refuse; CR-3 a raw-glob backstop per fault point; CR-2 a self-resolving POSIX snippet; CR-5 scratch under .devwork/ plus 15-min residue self-clean. All eight suites + bash -n gate green after fixes.", "findings": [], "artifacts": { "test_commit": "b9d3cf7", "test_files": [ ".claude/skills/local-fs/test/crash.sh", ".claude/skills/local-fs/test/shim-portability.sh" ] } } ```
jbr870 commented 2026-08-13 09:05:07 +00:00
Author
Owner
Copy link
{
  "next_state": "integrating",
  "produced": [
    {
      "kind": "qa-report",
      "ref": "comment:751",
      "summary": "code domain, phase=validate — independent adversarial review of 2e58030 by a fresh-context reviewer agent: 5 findings (2 blocking, 3 deferrable), all verified concrete"
    },
    {
      "kind": "qa-report",
      "ref": "comment:752",
      "summary": "code domain, phase=fix — all 5 findings fixed in b9d3cf7 (all trivial-adjacent, fix-now per the cost x value criteria; no deferrals, so no scope-disposition decisions raised); regression tests added for CR-1/CR-3/CR-4; all eight suites + bash -n gate green"
    },
    {
      "kind": "qa-report",
      "ref": "comments:746-750",
      "summary": "e2e, a11y, security-browser skipped per the new CLAUDE.md qa_domains declaration (committed b238149); api, security-api skipped per api_invocation: mode: none. Coverage rule verified: all six domains carry a phase=validate qa-report"
    }
  ],
  "findings": [],
  "pending_decisions": []
}
<!-- phase-outcome:v1 id=PO-26-4 skill=qa --> ```json { "next_state": "integrating", "produced": [ { "kind": "qa-report", "ref": "comment:751", "summary": "code domain, phase=validate — independent adversarial review of 2e58030 by a fresh-context reviewer agent: 5 findings (2 blocking, 3 deferrable), all verified concrete" }, { "kind": "qa-report", "ref": "comment:752", "summary": "code domain, phase=fix — all 5 findings fixed in b9d3cf7 (all trivial-adjacent, fix-now per the cost x value criteria; no deferrals, so no scope-disposition decisions raised); regression tests added for CR-1/CR-3/CR-4; all eight suites + bash -n gate green" }, { "kind": "qa-report", "ref": "comments:746-750", "summary": "e2e, a11y, security-browser skipped per the new CLAUDE.md qa_domains declaration (committed b238149); api, security-api skipped per api_invocation: mode: none. Coverage rule verified: all six domains carry a phase=validate qa-report" } ], "findings": [], "pending_decisions": [] } ```
jbr870 commented 2026-08-13 09:22:45 +00:00
Author
Owner
Copy link
{
  "integration_branch": "main",
  "feature_branch": "feature/26-local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment",
  "repos": [
    {
      "repo": "devwork-skills",
      "base_sha": "ae802e0",
      "tip_sha": "b9d3cf7",
      "commit_count": 5,
      "commits": [
        {
          "sha": "b9d3cf7",
          "subject": "local-fs: fix five code-QA findings from the #26 review round (CR-1..CR-5)"
        },
        {
          "sha": "b238149",
          "subject": "CLAUDE.md: declare qa_domains not-applicable set (e2e, a11y, security-browser)"
        },
        {
          "sha": "2e58030",
          "subject": "local-fs: normalize the ms clock and make comment posting atomic (#26)"
        },
        {
          "sha": "f8756c0",
          "subject": "docs(local-fs-comment-integrity): test plan"
        },
        {
          "sha": "bcc0903",
          "subject": "docs(local-fs-comment-integrity): software requirements"
        }
      ],
      "remote_url": "ssh://git@forge-devwork/jbr870/devwork-skills.git",
      "compare_url": null,
      "pushed": true
    }
  ]
}
<!-- code-bundle:v1 issue=26 skill=integrate --> <!-- code-bundle:v1 issue=26 skill=integrate --> ```json { "integration_branch": "main", "feature_branch": "feature/26-local-fs-comment-integrity-portable-mtime-pinning-atomic-post-comment", "repos": [ { "repo": "devwork-skills", "base_sha": "ae802e0", "tip_sha": "b9d3cf7", "commit_count": 5, "commits": [ { "sha": "b9d3cf7", "subject": "local-fs: fix five code-QA findings from the #26 review round (CR-1..CR-5)" }, { "sha": "b238149", "subject": "CLAUDE.md: declare qa_domains not-applicable set (e2e, a11y, security-browser)" }, { "sha": "2e58030", "subject": "local-fs: normalize the ms clock and make comment posting atomic (#26)" }, { "sha": "f8756c0", "subject": "docs(local-fs-comment-integrity): test plan" }, { "sha": "bcc0903", "subject": "docs(local-fs-comment-integrity): software requirements" } ], "remote_url": "ssh://git@forge-devwork/jbr870/devwork-skills.git", "compare_url": null, "pushed": true } ] } ```
jbr870 commented 2026-08-13 09:22:46 +00:00
Author
Owner
Copy link
{
  "next_state": "uat",
  "produced": [
    {
      "kind": "code-bundle",
      "ref": "deliverable",
      "summary": "5 commits ae802e0..b9d3cf7 fast-forwarded to main and pushed to origin; feature branch deleted (never pushed)"
    }
  ],
  "findings": [],
  "pending_decisions": [
    {
      "id": "D-PO-26-5-1",
      "type": "approval",
      "blocking": true,
      "question": "UAT: accept local-fs comment-integrity as shipped on main? Review the merged skill text (local-fs SKILL.md commit protocol + probe_pin, readiness-check.md Step 0, CLAUDE.md tier wording) at b9d3cf7 and/or dogfood a local-fs run; uat.url_source declares no deploy exists. Resolve approve to queue the issue at accepted for /promote; reject to trigger the UAT return path.",
      "options": [
        "approve",
        "reject"
      ],
      "recommended": "approve",
      "reasoning": "Merge path (uat.open_pr: false): merged and pushed autonomously per the skill design — merging is never gated; accepting is. All 8 suites green pre-merge at b9d3cf7 and smoke re-verified at merged HEAD; the fix was additionally validated on this box's real uutils toolchain, which reproduced the original defect at the pre-merge HEAD."
    }
  ]
}
<!-- phase-outcome:v1 id=PO-26-5 skill=integrate --> ```json { "next_state": "uat", "produced": [ { "kind": "code-bundle", "ref": "deliverable", "summary": "5 commits ae802e0..b9d3cf7 fast-forwarded to main and pushed to origin; feature branch deleted (never pushed)" } ], "findings": [], "pending_decisions": [ { "id": "D-PO-26-5-1", "type": "approval", "blocking": true, "question": "UAT: accept local-fs comment-integrity as shipped on main? Review the merged skill text (local-fs SKILL.md commit protocol + probe_pin, readiness-check.md Step 0, CLAUDE.md tier wording) at b9d3cf7 and/or dogfood a local-fs run; uat.url_source declares no deploy exists. Resolve approve to queue the issue at accepted for /promote; reject to trigger the UAT return path.", "options": [ "approve", "reject" ], "recommended": "approve", "reasoning": "Merge path (uat.open_pr: false): merged and pushed autonomously per the skill design — merging is never gated; accepting is. All 8 suites green pre-merge at b9d3cf7 and smoke re-verified at merged HEAD; the fix was additionally validated on this box's real uutils toolchain, which reproduced the original defect at the pre-merge HEAD." } ] } ```
jbr870 commented 2026-08-13 10:25:08 +00:00
Author
Owner
Copy link
{
  "status": "resolved",
  "chosen_option": "approve",
  "rationale": "Operator reviewed and accepted the merged feature at b9d3cf7 (UAT verdict given interactively this session: approved and accepted). Evidence at hand: merged skill text on main; all eight suites green pre-merge and smoke re-verified at merged HEAD; the dev box's own uutils 0.8.0 toolchain — the exact defect environment — now passes probe_pin and the full harness."
}
<!-- decision-resolution:v1 ref=D-PO-26-5-1 --> ```json { "status": "resolved", "chosen_option": "approve", "rationale": "Operator reviewed and accepted the merged feature at b9d3cf7 (UAT verdict given interactively this session: approved and accepted). Evidence at hand: merged skill text on main; all eight suites green pre-merge and smoke re-verified at merged HEAD; the dev box's own uutils 0.8.0 toolchain — the exact defect environment — now passes probe_pin and the full harness." } ```
jbr870 commented 2026-08-13 10:25:09 +00:00
Author
Owner
Copy link
{
  "next_state": "accepted",
  "produced": [
    {
      "kind": "code-bundle",
      "ref": "comment:758",
      "summary": "acceptance run — merge/cleanup/code-bundle already performed by the first /integrate run (PO-26-5); this run records the resolved UAT approval (D-PO-26-5-1: approve) and queues the issue for /promote"
    }
  ],
  "findings": [],
  "pending_decisions": []
}
<!-- phase-outcome:v1 id=PO-26-6 skill=integrate --> ```json { "next_state": "accepted", "produced": [ { "kind": "code-bundle", "ref": "comment:758", "summary": "acceptance run — merge/cleanup/code-bundle already performed by the first /integrate run (PO-26-5); this run records the resolved UAT approval (D-PO-26-5-1: approve) and queues the issue for /promote" } ], "findings": [], "pending_decisions": [] } ```
jbr870 commented 2026-08-24 09:05:13 +00:00
Author
Owner
Copy link

Retroactive attribution, deliberate. This issue was promoted on 2026-08-24, but its
code went live earlier: the merge commit b9d3cf7 is an ancestor of dev--v0.1.0, so
the feature has been serving in every release since. Linking it to dev--v0.1.6 (the
release cut by this promote run) would have recorded false provenance, so the link
names the release that actually contains the work. Only the bookkeeping was outstanding.

status: tag-only — release.sh pushes tags to the github remote only; the Gitea forge
has no tag to hang a Release object on, so there is no release URL to record.

{
  "tag": "dev--v0.1.0",
  "status": "tag-only",
  "repos": [
    {
      "repo": ".",
      "tag": "dev--v0.1.0",
      "target_sha": "ee15687e9cd55780054d7fe92230b9de92f9b467"
    }
  ]
}
<!-- release-link:v1 issue=26 skill=promote tag=dev--v0.1.0 --> Retroactive attribution, deliberate. This issue was promoted on 2026-08-24, but its code went live earlier: the merge commit `b9d3cf7` is an ancestor of `dev--v0.1.0`, so the feature has been serving in every release since. Linking it to `dev--v0.1.6` (the release cut by this promote run) would have recorded false provenance, so the link names the release that actually contains the work. Only the bookkeeping was outstanding. `status: tag-only` — release.sh pushes tags to the `github` remote only; the Gitea forge has no tag to hang a Release object on, so there is no release URL to record. ```json { "tag": "dev--v0.1.0", "status": "tag-only", "repos": [ { "repo": ".", "tag": "dev--v0.1.0", "target_sha": "ee15687e9cd55780054d7fe92230b9de92f9b467" } ] } ```
jbr870 commented 2026-08-24 09:05:41 +00:00
Author
Owner
Copy link
{
  "next_state": "deployed",
  "produced": [
    {
      "kind": "release-link",
      "ref": "comment:1101",
      "summary": "Shipped in dev--v0.1.0 (retroactive attribution: b9d3cf7 is an ancestor of that tag; the code has been live since 2026-08-14, only the promote bookkeeping was outstanding). Deploy check passed against dev--v0.1.6 at 6f0249f."
    }
  ],
  "findings": [],
  "pending_decisions": []
}
<!-- phase-outcome:v1 id=PO-26-7 skill=promote --> ```json { "next_state": "deployed", "produced": [ { "kind": "release-link", "ref": "comment:1101", "summary": "Shipped in dev--v0.1.0 (retroactive attribution: b9d3cf7 is an ancestor of that tag; the code has been live since 2026-08-14, only the promote bookkeeping was outstanding). Deploy check passed against dev--v0.1.6 at 6f0249f." } ], "findings": [], "pending_decisions": [] } ```
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
interactive-skills
No results found.
Labels
Clear labels   
component:adapters

   
component:lifecycle

   
component:qa

   
component:setup

   
component:shared

   
component:worktrees

   
phase/accepted

   
phase/backlog

   
phase/deployed

   
phase/developing

   
phase/integrating

   
phase/planning

   
phase/qa

   
phase/requirements

   
phase/uat

   
priority:critical

   
priority:critical

   
priority:high

   
priority:high

   
priority:low

   
priority:low

   
priority:medium

   
priority:medium

   
type:bug

   
type:chore

   
type:docs

   
type:feature

   
type:infra

   
type:tech-debt
No labels
component:adapters
component:lifecycle
component:qa
component:setup
component:shared
component:worktrees
phase/accepted
phase/backlog
phase/deployed
phase/developing
phase/integrating
phase/planning
phase/qa
phase/requirements
phase/uat
priority:critical
priority:critical
priority:high
priority:high
priority:low
priority:low
priority:medium
priority:medium
type:bug
type:chore
type:docs
type:feature
type:infra
type:tech-debt
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
jbr870/devwork-skills#26
No description provided.