chore(locks): audit and retag every TODO:[Locks] marker (#2040) - #2376
Conversation
…e, drop the duplicated marker tables
Greptile SummaryThis PR audits and retags Locks TODO markers without changing production behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/hooks/useCreateLockContent/useCreateLockContent.ts | Adds documentation for deferred publish-path lock URL validation; executable behavior is unchanged. |
| src/test-utils/locks.ts | Introduces a typed shared lock-file fixture matching the duplicated fixtures it replaces. |
| docs/locks.md | Updates Locks contracts, marker tracking, and shared-fixture documentation. |
| src/core/application/locks/locks.ts | Retags and consolidates deferred-work comments without changing application logic. |
Reviews (2): Last reviewed commit: "docs(locks): update the release-gate mar..." | Re-trigger Greptile
|
@greptile-apps |
Orlandohub
left a comment
There was a problem hiding this comment.
It looked good for me overall therefore will approve.
Leaving some comments from AI
[P2] Preserve tracking for the unvalidated registerGuardedResource response.
locks.ts:262 still casts and trusts an unparsed creator response, but this PR removes its TODO and only tracks createContentLock. pubky/locks#22 explicitly covers both endpoints. Retag this call to that issue or validate its response now; otherwise malformed creator/resource values can produce unreachable attachment URIs while the remaining TODO no longer points reviewers here.
[P2] The audit introduces an unowned marker despite requiring issue ownership.
useCreateLockContent.ts:90 adds a bare TODO even though docs/locks.md:117 says every shortcut carries its owning ticket, and closing issue #2040 requires deferred work to be retracked rather than left bare. The head has 25 Locks markers, three without identifiers. I would reopen the existing resolved Greptile thread or change the documented convention explicitly.
|
@Orlandohub
|
Only docs/locks.md conflicted. #2376 replaced the release-gate marker table with a shorter marker-tracking note and moved the shared test fixtures to src/test-utils/locks.ts; this branch had added the Unlocked screen section. Kept both: the new section stays, the marker table gives way to #2376's text, and the heading is now "Phase 1 & marker tracking". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #2040.
Audit of every
TODO:[Locks]marker. No behaviour changes — comments, one shared test fixture, and the docs.grep -rn "TODO:\[Locks\] #1998" src/now returns zero.Removed — already resolved
models/post/details/postDetails.tslockis assigned in the constructor. Every sibling field is too, so it said nothing.services/nexus/nexus.types.tsorganisms/DialogLocksAuth/connectiframe sandbox set to be verified against the live flow. Verified on dev.organisms/LockedPostContentRetagged
dev-static+ password verifier (6 places)creatorPubkymissing on the SDK sessioninit()is neededLockFiletype hand-mirrored from the SDKlock.jsoninstead of the response headerAdded
useCreateLockContent— the publish path builds a lock URL and posts it without running it throughisValidLockUrl, which the read path already does. Marked, not fixed.Also
lock.jsontest fixture was copied verbatim into three test files. Moved tosrc/test-utils/locks.tsasmockLockFile().docs/locks.md— dropped the marker tables. They duplicated whatgrepand the issues already say, and had already drifted.Filed upstream
Not in scope
#2312 is a real bug that is now unblocked (#2027 closed,
PubkyAppPost.new_with_lockexists), but #1998 moved it out of the epic. Left as is.