test(lab): verify busy-lock ownership without wall-clock timing - #4139
test(lab): verify busy-lock ownership without wall-clock timing#4139luvs01 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change replaces 500 ms contention timing checks with synchronous refusal, one owner-liveness check, and ownership-preservation checks. It updates the mutation-lock test, management endpoint test, and CL-10 regression requirements. ChangesCommunity lock contention
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Community-lock contention coverage now validates the refusal and ownership-preservation behavior directly while retaining the retryable management response contract. No concrete current-head merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Please review immutable head Please check that the updated tests retain meaningful public-list and ownership coverage, and that the revised document makes the verification change and its limit clear. Focused and import-connected checks both passed (12 tests / 69 assertions each), along with typecheck/privacy/diff checks. The final rebase added only unrelated devlog closeout commits and retained an identical patch. Full author CI is queued. Use available plain-text inspection without requiring |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== HEAD and status =='
git rev-parse HEAD
git status --short
printf '%s\n' '== Parent commits =='
git show -s --format='%H%n%P%n%s' HEAD
printf '%s\n' '== Changed files against first parent =='
git diff --name-status HEAD^ HEAD
printf '%s\n' '== Patch against first parent =='
git diff --no-ext-diff --unified=80 HEAD^ HEAD
printf '%s\n' '== Candidate source and documentation files =='
fd -t f . | rg -i 'community|lock|cl-10|closure|cache|public'
printf '%s\n' '== Busy-lock and management-route references =='
rg -n -i -C 5 \
'community_cache_busy|Retry-After|retry-after|community.*lock|lock.*community|public.*list|list.*public' \
. \
-g '!node_modules/**' -g '!dist/**' -g '!build/**' || trueLength of output: 195691 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Immutable head =='
git rev-parse HEAD
printf '%s\n' '== Commit =='
git show -s --format='commit %H%nparents %P%nsubject %s' HEAD
printf '%s\n' '== Files changed by HEAD =='
git diff-tree --no-commit-id --name-status -r HEAD
printf '%s\n' '== Statistical patch summary =='
git diff --stat HEAD^ HEAD
printf '%s\n' '== Exact patch, first parent to HEAD =='
git diff --no-ext-diff --unified=30 HEAD^ HEADLength of output: 15191 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Relevant source files =='
fd -t f . src tests | rg '(mutation-lock|community|management-api|lab-public-surfaces|lab-community-mutation-lock)\.(ts|tsx)$'
printf '%s\n' '== Source outlines =='
ast-grep outline src/lab/public/mutation-lock.ts --items all 2>/dev/null || true
ast-grep outline src/lab/public/community.ts --items all 2>/dev/null || true
ast-grep outline src/server/management-api.ts --items all 2>/dev/null || true
printf '%s\n' '== Lock implementation and all direct callers =='
rg -n -C 12 \
'\bwithPublicEvidenceMutationLock\b|\bcommunity_cache_busy\b|\blistCommunityEvidence\b' \
src/lab src/server tests/lab \
-g '*.ts'
printf '%s\n' '== Changed test context =='
sed -n '1,180p' tests/lab/lab-community-mutation-lock.test.ts
sed -n '220,360p' tests/lab/lab-public-surfaces.test.ts
printf '%s\n' '== Management route and typed-error mapping context =='
rg -n -C 15 \
'community_cache_busy|/api/lab/public/community|PublicEvidenceValidationError' \
src/server src/lab \
-g '*.ts'Length of output: 185260 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Lock acquisition and wrapper =='
sed -n '340,455p' src/lab/public/mutation-lock.ts
printf '%s\n' '== Community listing implementation =='
rg -n -A70 -B12 '^export function listCommunityEvidence\b|^export function listCommunityEvidenceContext\b' \
src/lab/public/community.ts src/lab/public/operator.ts
printf '%s\n' '== Public-surface API helper and busy test =='
rg -n -A55 -B15 '^async function api\b|^function api\b|busy community lock' \
tests/lab/lab-public-surfaces.test.ts
printf '%s\n' '== Changed regions with line numbers =='
awk 'NR >= 60 && NR <= 100 { printf "%d:%s\n", NR, $0 }' tests/lab/lab-community-mutation-lock.test.ts
awk 'NR >= 275 && NR <= 320 { printf "%d:%s\n", NR, $0 }' tests/lab/lab-public-surfaces.test.ts
awk 'NR >= 1 && NR <= 55 { printf "%d:%s\n", NR, $0 }' docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md
printf '%s\n' '== Patch whitespace check =='
git diff --check HEAD^ HEADLength of output: 18502
I found no blocking issue.
✅ Action performedReview finished.
|
리뷰 · 우선순위 56 / 80이 PR은 커뮤니티 증거(public evidence) mutation lock이 이미 살아 있는 소유자에게 막혔을 때, 테스트가 벽시계 500ms 안에 끝나야 한다는 기준을 빼는 작업이다. 현재 이 변경은 그 오라클을 고친다. mutation-lock 쪽은 지금 라인 - 라인 - 경로/심볼 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
0efdd39 to
bfc99a8
Compare
|
@coderabbitai review Please review current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head="bfc99a8257b954f993fb0db8fa596d0b8ebb500f"
base="5b8f1fcfad0e17318d919edf937b94744e393a36"
previous="0efdd39e4f56779aac26039e6cce87d792a730b4"
printf '%s\n' '== Immutable HEAD =='
git rev-parse HEAD
git show -s --format='commit %H%nparents %P%nsubject %s' HEAD
git merge-base --is-ancestor "$base" "$head"
printf 'base-is-ancestor: %s\n' "$?"
printf '%s\n' '== Current authored delta =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
printf '%s\n' '== Rebased patch equivalence =='
git range-diff "${previous}^1..${previous}" "${base}..${head}" || true
git diff --no-ext-diff --unified=45 "$base" "$head" -- \
tests/lab/lab-community-mutation-lock.test.ts \
tests/lab/lab-public-surfaces.test.ts \
docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md
printf '%s\n' '== Candidate-file outlines =='
ast-grep outline tests/lab/lab-community-mutation-lock.test.ts --items all
ast-grep outline tests/lab/lab-public-surfaces.test.ts --items all
ast-grep outline src/lab/public/mutation-lock.ts --items all
ast-grep outline src/server/management/lab-routes.ts --items all
printf '%s\n' '== Changed-test and closure-document context =='
sed -n '55,110p' tests/lab/lab-community-mutation-lock.test.ts
sed -n '260,330p' tests/lab/lab-public-surfaces.test.ts
sed -n '1,80p' docs/superpowers/plans/2026-08-14-cl10-final-review-closure.md
printf '%s\n' '== Busy refusal implementation and route mapping =='
rg -n -C 16 '\bwithPublicEvidenceMutationLock\b|community_cache_busy' \
src/lab/public/mutation-lock.ts \
src/lab/public/community.ts \
src/server/management/lab-routes.ts
printf '%s\n' '== Timeout and elapsed-time changes in the authored delta =='
git diff --no-ext-diff -U5 "$base" "$head" | rg -n -i \
'timeout|deadline|elapsed|performance\.now|setTimeout|500\s*(ms|milliseconds)' || true
printf '%s\n' '== Exact-head CI status =='
gh api repos/luvs01/opencodex/actions/runs/34416958829 \
--jq '{head_sha, status, conclusion, event, html_url, updated_at}'Length of output: 49823
I found no blocking issue.
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
bfc99a8 to
800df62
Compare
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Checked the rebased head 800df62 on current dev c15a98c. The runtime still rejects a non-reclaimable live owner before protected work; the loop only retries after reclamation or a disappearing-lock race. The management test still exercises the public community-list route and its 503 / Retry-After: 1 mapping, rather than just a mocked helper.
The replacement assertions add owner-byte and directory-identity checks, and the closure document explicitly records removal of the 500 ms oracle. These tests do not prove a numerical latency guarantee or detect every possible synchronous wait; that limitation is stated rather than hidden. No runtime, workflow timeout, or Windows/Bun workaround is removed.
Author CI run 34425724772 is independently verified successful at this exact head. No new source blocker found in this bounded test/documentation change. The PR is still Draft with an incomplete readiness checklist, so please finish the author readiness step before final integration review. I have not changed Draft status or merged it.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800df62de6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
800df62 to
f5b7591
Compare
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The f5b7591 increment adds meaningful coverage beyond the previously equivalent rebase: it spies on the real process.kill path, requires exactly one call with (process.pid, 0), retains the refusal/owner assertions, and restores the spy in finally. The document correctly limits the claim to repeated owner probes rather than all possible delays. I inspected this delta but did not run the reported mutation experiment locally. Author CI 34439155705 is currently queued at this head, so the earlier green head does not settle this revision. Keep the Draft/readiness gate until that evidence completes.
3 participants
Summary
The live community-lock regressions measured filesystem and management-route work against a 500 ms wall-clock limit. A Windows CI shard failed at 506.02 ms before reaching its response assertions, while the unchanged case later passed in isolation. The original host timing cause was not established.
Replace that documented timing oracle with the actual refusal and ownership contract: synchronous
community_cache_busy, no protected callback execution, unchanged owner bytes and lock directory identity, and the real management API's 503 / Retry-After: 1 response. Existing stale-lock recovery, quarantine cleanup and public-list integration coverage remain. Update the CL-10 closure document to retain the history of the original 500 ms criterion and explain the new verification method.This changes two tests and their verification document only. Runtime code and test deadlines are unchanged. The new assertions do not establish a numerical response-time guarantee or directly detect every possible short synchronous polling loop; the current direct-throw/no-polling runtime path remains unchanged and was inspected separately.
Verification
f5b759138788b4e4f688425cfdeeef4867726867, based ondev 386b6a0d9a8acef818b9c40ebd472e4974750199.34439155705: 26/26 jobs passed, bound tof5b759138788b4e4f688425cfdeeef4867726867. The checklist CI attestation refers to this completed matrix; local focused results are listed separately.Checklist
Review readiness checklist
Readiness base check: 7 commits behind current dev; within the repository allowance of ten.