fix(update): preserve service repair on scan failure - #1375
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe restart flow now distinguishes failed listener scans from confirmed empty scans. Failed scans preserve service repair. A regression test verifies that the repair command runs after listener inspection fails. ChangesService restart recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request has been marked Ready for Review. |
|
Please rebase this PR onto the latest |
8e9b7ad to
fbf4a7f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/update-job.test.ts`:
- Around line 465-476: Add a spawnStart stub to the restartAfterUpdateForTests
options in the regression test, ensuring the direct-start fallback cannot launch
production processes or poll real OS state. Keep the stub deterministic and
aligned with the existing spawnStart seam in restartAfterUpdate.
- Around line 471-477: Update the test setup around runService in the relevant
update-job test to capture its args, then assert the invocation includes the
"repair" argument in addition to verifying serviceRuns is 1. Preserve the
existing probeProxy behavior and validate the command executed by the restart
path.
- Around line 451-464: Update the UpdateJobState fixture in the test around the
job object to include the required releaseNotesUrl property, using an empty
string so the persisted job matches the current schema.
🪄 Autofix
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c21e7f30-5c21-4251-9f1c-bf9e6fcd4756
📒 Files selected for processing (2)
src/update/job.tstests/update-job.test.ts
|
An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
fbf4a7f to
afa26aa
Compare
Rate Limit Exceeded
|
Rate Limit Exceeded
|
afa26aa to
33558af
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Pull request metadata =='
gh pr view 1375 --repo lidge-jun/opencodex \
--json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,statusCheckRollup \
--jq '{number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,statusCheckRollup:[.statusCheckRollup[]|{name,conclusion,status}]}'
echo
echo '== Pull request diff =='
gh pr diff 1375 --repo lidge-jun/opencodex -- src/update/job.ts tests/update-job.test.ts
echo
echo '== Source outline =='
ast-grep outline src/update/job.ts --items all --view expanded
echo
echo '== Test outline =='
ast-grep outline tests/update-job.test.ts --items all --view expandedLength of output: 1669 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed-file patches on current PR head =='
gh api 'repos/lidge-jun/opencodex/pulls/1375/files?per_page=100' \
--jq '.[] | "===== \(.filename) =====\n\(.patch // "(no textual patch)")"'
echo
echo '== Current source structure =='
ast-grep outline src/update/job.ts --items all --view expanded
echo
echo '== Current test structure =='
ast-grep outline tests/update-job.test.ts --items all --view expanded
echo
echo '== Listener-scan references and contracts =='
rg -n -C 5 --glob '!node_modules/**' --glob '!dist/**' \
'listListenPids|listen.*Pid|Listener.*scan|listener.*scan|waitForPort|reclaim' \
src tests
echo
echo '== Current PR-head checks =='
gh pr checks 1375 --repo lidge-jun/opencodex --json name,state,link \
--jq '.[] | {name,state,link}'Length of output: 128163
I found no correctness issues in the changed code.
The readiness-gate record is stale. It identifies completed head You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/update-job.test.ts`:
- Line 466: Update the call to updateJobPath in the test setup to match its
declared zero-argument signature, removing the job.id argument while preserving
the existing writeFileSync behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17754528-bd4d-4179-8f21-95652f81e65c
📒 Files selected for processing (1)
tests/update-job.test.ts
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr="1375"
previous_head="33558af5c2abcf8d0f0082ed273dfb949561b45e"
echo '== PR metadata and checks =='
gh pr view "$pr" --repo "$repo" \
--json headRefOid,baseRefOid,mergeable,statusCheckRollup,commits \
--jq '{
headRefOid,
baseRefOid,
mergeable,
commits: [.commits[] | {oid: .oid, messageHeadline: .messageHeadline}],
checks: [.statusCheckRollup[] | {name, status, conclusion}]
}'
echo
echo '== Files changed since the previously reviewed head =='
git diff --name-status "$previous_head"...HEAD
echo
echo '== Current PR patches =='
gh api "repos/$repo/pulls/$pr/files?per_page=100" \
--jq '.[] | "===== \(.filename) =====\n\(.patch // "(no textual patch)")"'
echo
echo '== Source and test structure =='
ast-grep outline src/update/job.ts --items all --view expanded
ast-grep outline tests/update-job.test.ts --items all --view expanded
echo
echo '== Listener-scan and service-repair context =='
rg -n -C 8 --glob '!node_modules/**' --glob '!dist/**' \
'listListenPids|ListenPidScan|waitForPort|skipServiceInstall|reclaim|service.*install|service.*repair' \
src/update/job.ts tests/update-job.test.tsLength of output: 76860 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr="1375"
head="3ee2aa6b02e599092cb7865cca489164687872cd"
echo '== Final commit patch =='
git show --format=fuller --find-renames --find-copies "$head" -- tests/update-job.test.ts
echo
echo '== Production control-flow range =='
sed -n '1107,1210p' src/update/job.ts
echo
echo '== Regression test range =='
sed -n '449,486p' tests/update-job.test.ts
echo
echo '== Checks attached to the current head =='
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
--jq '.check_runs[] | {
name,
status,
conclusion,
started_at,
completed_at,
details_url,
head_sha
}'
echo
echo '== Current head commit statuses =='
gh api "repos/$repo/commits/$head/status" \
--jq '{sha, state, statuses: [.statuses[] | {context, state, target_url, updated_at}]}'
echo
echo '== Ready-gate comment =='
gh pr view "$pr" --repo "$repo" --comments --json comments \
--jq --arg head "$head" \
'[.comments[] | select(.body | contains("opencodex-pr-gate-state") and contains($head)) | {
author: .author.login,
createdAt,
body
}]'Length of output: 13140
I found no correctness issues.
The current head has a successful You are interacting with an AI system. |
|
Thanks @luvs01! Appreciate the focused service-repair fix, deterministic regression coverage, and the careful follow-up on the review findings. Merged. 🙏 |
2 participants
Summary
Verification
bun test tests/update-job.test.ts— 54 passed.bun run typecheck— passed.bun run privacy:scan— passed.git diff --check HEAD^ HEAD— passed.Checklist
dev.Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Tests