Repository navigation
fix(protocol): one lease comment per step, edited in place (#69) - #91
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #69
What changed
claimandrenew(lib.sh) now go through_write_lease: if this account already wrote a lease comment for the same step on the item, it is edited in place (PATCH); otherwise one is posted. Three claims of a step leave one lease comment. Each step keeps its own comment. A comment from another account is never edited, and if the edit fails a new comment is posted, so the fallback is only noisier.lease_live(lib.sh) andleaseState(helpers.mjs) now take the lease written last (updated_at, elsecreated_at, else thread order). Without this, re-claiming an earlier step's comment while a later step's comment sat after it would read the wrong expiry.expires=inside the marker. The comment's edit time only orders leases and is never read as an expiry (the check the issue asked for), and a unit test covers a lease edited a minute ago whose marker expired an hour ago.ownLeaseCommentand anown-leaseCLI command inhelpers.mjs, so the shell and a no-shell runner share one implementation.NO_SHELL_OPSand the protocol's Leases and Markers sections are updated.Tests
tests/lib.test.sh: stubghnow supportsPATCHof a comment, comment ids and timestamps. New cases for one comment after three claims, the newest expiry, a separate comment per step,renewediting in place, another account's comment untouched, the failed-edit fallback, and last write beats thread order. The existing expiry and trust tests pass unchanged.markers.json) gain two ordering cases that both the shell and Node suites run.helpers.test.mjscoversownLeaseComment, the CLI and the edit-time-is-not-expiry case.lib.sh(5 failures) and pass with this change.bash tests/lib.test.sh168 passed,node --test376 passed,npm run validate,lint,sync:checkandcheck-plugin-versionall clean.Notes
commenteffect with theagent:leasemarker. That is unchanged here, since scenario worlds start without lease comments.