test(lab): verify busy-lock ownership without wall-clock timing - #4139
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.
f5b7591 to
c4542a8
Compare
Ready-for-review evidenceRebased onto the current Local run — No unresolved Codex or CodeRabbit findings on this PR. |
|
Follow-up to Ingwannu's 2026-09-12 review: the requested refresh was performed afterward. Current head 4b524e9 was committed at 2026-09-12T05:39:59Z on dev@81f6cd591. Both reviewed upstream runs (34601671513 and 34601671501) passed the older 179e493 head; they are not evidence for this revision. Its exact-head fork run 34676995586 completed red. Besides recurring shared failures, page-loading-contract.test.tsx:304 failed in Dev has advanced again to c27a483; this branch remains mergeable, but the latest-dev and CI readiness boxes are unchecked. The test-only patch remains a candidate requiring integration validation before merge, rather than relying on the older green runs. |
4b524e9 to
7f0a492
Compare
|
Follow-up to your current-head CI report: the active reactivation Save-disabled failure now has a landed, source-specific fixture fix in #4385, commit 657e068 (present on current dev). I inspected that correction: gui/tests/page-loading-contract.test.tsx captures the zero-delay activation callback, asserts exactly one callback, invokes it inside act, and verifies the old expiry timer was cancelled. The final enabled-Save and preserved-draft assertions remain. It does not explain the 20.44ms value as a timeout or remove the failing assertion. Please incorporate the current baseline/integration fixes before the next full run and keep unrelated failures individually accounted for. This is a concrete resolution candidate for that shared GUI fixture, not proof that every failure in #4139 is fixed, and the older green 179e493 runs remain historical only. I have not rerun or approved this new revision automatically. |
7f0a492 to
4e927af
Compare
4e927af to
2233172
Compare
2233172 to
efd8b78
Compare
|
Updated to efd8b78 on dev@dc33113a9. This head includes both the activation-callback fixture fix 657e068 and the Windows Devin fixture correction 720ea97. The two authored contention tests still preserve owner identity/bytes, synchronous refusal and 503/Retry-After behavior. Focused tests: 12 pass, 0 fail, 71 assertions; typecheck, structure and privacy checks passed. Full validation of this head is pending at https://github.com/luvs01/opencodex/actions/runs/34737277447; the PR remains Draft. |
efd8b78 to
5ad26c1
Compare
5ad26c1 to
2a8ca25
Compare
3 participants
Summary
Exercise the public community-mutation lock boundary with contention tests. Both mutation-lock and public-surface tests preserve the existing ownership and refusal contract.
Current verification
Head:
5ad26c1dd632f009b3454b52a894bd6b9b576119.Focused lab tests: 12 pass, 0 fail, 71 assertions. Typecheck, structure and privacy checks passed.
Full cross-platform CI has not been verified for this published head. Earlier CI run passed a different head and is historical evidence only. Rebase and focused checks do not turn that result into a full-suite pass on this head.
Review readiness evidence
Rebased onto
devat15fbd49f3and pushed; the branch was 0 commits behind at push time. Published head is2a8ca25cb.Local verification on that head:
bun run typecheck,bun run structure:check,bun run privacy:scanandgit diff --checkall pass.bun test tests/lab/lab-community-mutation-lock.test.ts tests/lab/lab-public-surfaces.test.tsreports 12 pass / 0 fail.Hosted cross-platform CI has not been dispatched on this head. On current
devthewindowsshard fails independently of this pull request:tests/clients/desktop-app-restart-posix.test.tsfails 9 of 17 on Windows becauseisUnderRootbuilds its boundary frompath.sepalone and because thedarwin/linuxcurrentUid()helpers callprocess.getuid(), which does not exist on Windows. That is adevregression, not a defect of this change, and it is fixed separately in #4564. The first box is ticked on the local run recorded above, which is what its wording asks for.Review readiness checklist