π fix(eligibility): soften mismatch blockers during agent trigger re-registration (display only) - #661
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
π WalkthroughWalkthrough
Possibly related PRs
π₯ Pre-merge checks | β 2β Passed checks (2 passed)
β¨ Finishing Touches π‘ 1π οΈ Fix failing CI checks π‘
π Generate docstrings
π§ͺ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
π€ 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 `@app/agent/AgentClient.ts`:
- Around line 952-1007: Extend isRegisteringComponents coverage to
handleComponentSync(), wrapping its deregister, watcher registration, inventory
application, and trigger registration flow in try/finally so the flag is true
throughout awaited synchronization and always resets to false. Update the
fieldβs contract/comments to cover both _doHandshake() and
handleComponentSync(), and add a regression test verifying transient list/SSE
blockers remain active during edge component sync and are cleared afterward.
In `@app/updates/request-update.test.ts`:
- Around line 880-902: Update the test βstays a hard agent-mismatch rejection
even when the agent is still completing registrationβ to mark the existing agent
fixture as registration-pending by setting the established
isRegisteringComponents or isAgentPendingRegistration state before
enqueueContainerUpdate. Preserve the expected 404 rejection, and ensure the test
genuinely covers the pending-registration admission path rather than only an
ordinary edge-2 versus edge-1 mismatch.
πͺ 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: CHILL
Plan: Pro Plus
Run ID: 1fb57f71-aefd-4664-b438-5044c798c059
β Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
π Files selected for processing (10)
app/agent/AgentClient.test.tsapp/agent/AgentClient.tsapp/api/container/handlers/list.test.tsapp/api/container/handlers/list.tsapp/api/sse-container-enrichment.test.tsapp/api/sse-container-enrichment.tsapp/model/update-eligibility.test.tsapp/model/update-eligibility.tsapp/updates/request-update.test.tsapp/updates/request-update.ts
β¦tration window Fixes: #605 AgentClient._doHandshake() deregisters an agent's components before awaiting the /api/triggers fetch and re-register. A read during that window found zero triggers for the agent and computeUpdateEligibility raised agent-mismatch as a hard blocker, disabling the Update button for a condition that was purely transient and self-corrected once registration finished. - makeBlocker() accepts an optional severity override - UpdateEligibilityContext gains isAgentPendingRegistration(agentName), wired into the three eligibility call sites (container list, SSE enrichment, manual update request) via the agent manager's getAgent(...)?.isConnected - agent-mismatch downgrades to soft when the mismatched trigger's agent is still connecting; unaffected otherwise
- π fix: handleComponentSync() now wraps its deregister β re-register sequence in the same isRegisteringComponents try/finally as _doHandshake(), so eligibility display stays soft during an awaited edge component sync - β test: cover the flag through the sync sequence, including reset on watcher-registration failure - β test: pin the registration-pending admission path β enqueueContainerUpdate still hard-rejects agent-mismatch while the agent is mid-registration Fixes: #605
9498ac5 to
487c16b
Compare
|
Deployment failed with the following error: Learn More: https://vercel.com/codeswhat?upgradeToPro=build-rate-limit |
|
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. |
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 `@app/agent/AgentClient.test.ts`:
- Around line 1167-1173: Extend the disconnect test around scheduleReconnect to
cover an in-flight _doHandshake: block the initial container request, trigger
scheduleReconnect, then resolve the request and handshake, and verify the stale
handshake cannot restore isRegisteringComponents or handshakeInProgress or
proceed through deregister/re-register. Implement cancellation or a
reconnect-generation guard at the handshake entry and before the
deregister-to-register span, preserving the reset behavior for the current
connection.
πͺ 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: CHILL
Plan: Pro Plus
Run ID: 0ef2b244-b2e7-4cea-a6e9-f40d02fbeec9
β Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
π Files selected for processing (10)
app/agent/AgentClient.test.tsapp/agent/AgentClient.tsapp/api/container/handlers/list.test.tsapp/api/container/handlers/list.tsapp/api/sse-container-enrichment.test.tsapp/api/sse-container-enrichment.tsapp/model/update-eligibility.test.tsapp/model/update-eligibility.tsapp/updates/request-update.test.tsapp/updates/request-update.ts
π§ Files skipped from review as they are similar to previous changes (9)
- app/api/container/handlers/list.test.ts
- app/model/update-eligibility.test.ts
- app/api/container/handlers/list.ts
- app/updates/request-update.ts
- app/model/update-eligibility.ts
- app/api/sse-container-enrichment.ts
- app/api/sse-container-enrichment.test.ts
- app/agent/AgentClient.ts
- app/updates/request-update.test.ts
| test('should reset isRegisteringComponents to false on disconnect (#605)', () => { | ||
| client.isConnected = true; | ||
| client.isRegisteringComponents = true; | ||
| client.scheduleReconnect(1000); | ||
| expect(client.isRegisteringComponents).toBe(false); | ||
| }); | ||
|
|
There was a problem hiding this comment.
π― Functional Correctness | π Major | β‘ Quick win
π§© Analysis chain
π Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline app/agent/AgentClient.ts --items all --match 'handshake|_doHandshake|scheduleReconnect'
rg -n -C 12 'async _doHandshake|scheduleReconnect\(|isRegisteringComponents' app/agent/AgentClient.tsRepository: CodesWhat/drydock
Length of output: 11202
π Script executed:
#!/bin/bash
set -euo pipefail
printf '--- AgentClient outline relevant sections ---\n'
ast-grep outline app/agent/AgentClient.ts --items all --match 'AgentClient|connector|connected|sse|SSE|handshake|disconnected|ScheduleReconnect' || true
printf '\n--- AgentClient.test relevant tests/context ---\n'
sed -n '1130,1190p' app/agent/AgentClient.test.ts
printf '\n--- All references to isRegisteringComponents in tests ---\n'
rg -n -C 8 'isRegisteringComponents|should reset|disconnect `#605`|scheduleReconnect' app/agent/AgentClient.test.ts
printf '\n--- Reconnect/disconnect related symbols ---\n'
rg -n -C 10 'isConnected|agentConnected|agentDisconnected|startSse|startConnector|scheduleReconnect|disconnect' app/agent/AgentClient.test.tsRepository: CodesWhat/drydock
Length of output: 246
π Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- AgentClient test relevant range ---'
sed -n '1130,1190p' app/agent/AgentClient.test.ts
printf '%s\n' '--- isRegisteringComponents tests/mentions ---'
rg -n -C 8 'isRegisteringComponents|should reset|disconnect `#605`|scheduleReconn' app/agent/AgentClient.test.ts
printf '%s\n' '--- reconnect/disconnect-related test references ---'
rg -n -C 8 'isConnected|agentConnected|agentDisconnected|startSse|scheduleReconn|disconnect' app/agent/AgentClient.test.ts
printf '%s\n' '--- AgentClient relevant implementation ranges ---'
sed -n '930,1030p' app/agent/AgentClient.ts
sed -n '1080,1115p' app/agent/AgentClient.ts
printf '%s\n' '--- source references to isRegisteringComponents assignment ---'
rg -n -C 6 'this\.isRegisteringComponents\s*= true|isRegisteringComponents = false|agentConnected|agentDisconnected|startSse' app/agent/AgentClient.ts app/agent/AgentClient.test.tsRepository: CodesWhat/drydock
Length of output: 50374
π Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- _doHandshake implementation ---'
sed -n '939,1030p' app/agent/AgentClient.ts
printf '%s\n' '--- scheduleReconnect implementation ---'
sed -n '1089,1115p' app/agent/AgentClient.ts
printf '%s\n' '--- tests in concurrent handshake/reconnect areas ---'
sed -n '869,970p' app/agent/AgentClient.test.ts
printf '%s\n' '--- read-only deterministic invariant probe: identify pending container request then disconnect ordering ---'
python3 - <<'PY'
from pathlib import Path
src = Path('app/agent/AgentClient.ts').read_text()
lines = src.splitlines()
for name, pattern in [('startSse guard', 'if (this.stopped)'), ('scheduleReconnect ignore pending', 'if (this.stopped || this.reconnectTimer)'), ('scheduleReconnect clears flag', 'if (!wasConnected)' if False else 'this.isRegisteringComponents = false'), ('_doHandshake container request', 'const response = await axios.get'), ('_doHandshake sets flag after response', 'this.isRegisteringComponents = true'), ('_doHandshake finally sets false', 'finally'), ('_doHandshake finally sets false statement', 'this.isRegisteringComponents = false')]:
if pattern == 'this.isRegisteringComponents = false' or pattern == 'this.isRegisteringComponents = true':
for i,l in enumerate(lines,1):
if pattern in l:
print(f'{name}: line {i}: {l.strip()}')
else:
for i,l in enumerate(lines,1):
l2=l.strip()
if '//' in l2:
j = l2.index('//')
before=l2[:j].strip()
if not before:
continue
l2=before
if pattern in l2:
print(f'{name}: line {i}: {l.strip()}')
print('probe: first connection request await then disconnect can clear already false flag without canceling this stale handshake; no later assignment in this file matches a generation token guard for isRegisteringComponents')
PYRepository: CodesWhat/drydock
Length of output: 11212
Cover the pending-handshake disconnect race.
This only asserts the pre-set flag is cleared. _doHandshake() sets isRegisteringComponents after the initial container request resolves. Start a handshake, block the container request, call scheduleReconnect(), then resolve the request and resolve handshake(). That stale handshake can set the flag back to true, or keep it true through component registration. Cancel stale handshakes or guard isRegisteringComponents / handshakeInProgress with a reconnect-generation token before entering the deregister β re-register span.
π€ Prompt for 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.
In `@app/agent/AgentClient.test.ts` around lines 1167 - 1173, Extend the
disconnect test around scheduleReconnect to cover an in-flight _doHandshake:
block the initial container request, trigger scheduleReconnect, then resolve the
request and handshake, and verify the stale handshake cannot restore
isRegisteringComponents or handshakeInProgress or proceed through
deregister/re-register. Implement cancellation or a reconnect-generation guard
at the handshake entry and before the deregister-to-register span, preserving
the reset behavior for the current connection.
Wholesale-tree sync ahead of the v1.6.0-rc.12 cut: the head commit's tree is `origin/dev/v1.6`'s tree verbatim (`630c64ec`), parented on main's current head (`67f23419`). Squash-merging makes main tree-identical to `dev/v1.6`, satisfying the release-cut drift guard (`git diff --quiet origin/main origin/dev/v1.6`). Brings in since rc.11: #659 security pins, #662/#660/#661/#663 runtime fixes (#636/#604/#605/#637), #664 guard floor, #653β#656 deps, #665 Crowdin, #666 rc.12 prep. After merge: dispatch `release-cut.yml --ref main -f release_tag=v1.6.0-rc.12`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Changelog - π Updated dependency security pins and security-version tests. - β¨ Added anonymous-authenticated WebSocket log streams with IP-based rate limits. - β¨ Added agent component-registration state tracking. - π§ Downgraded selected update blockers during agent registration. - π§ Added controller-Docker transport detection. - π Added HTTP `501` responses for unsupported agent lifecycle actions. - π Fixed maturity filtering to use per-container thresholds. - π Changed Docker publish-date lookup failures to warning logs. - π§ Updated dependency versions, release metadata, documentation, mocks, and changelog links from `v1.6.0-rc.11` to `v1.6.0-rc.12`. - β¨ Added release highlights for `v1.6.0-rc.12`. ## Concerns - Fix the French `containerLogs.stderr` value if `"sdterr"` is not intentional. - Verify the release date `August 4, 2026`. - Dispatch the release-cut workflow for `main` with `release_tag=v1.6.0-rc.12` after merging. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Fixes #605.
AgentClient._doHandshake()deregisters the agent's components before re-registering them; in that window eligibility showed a hardagent-mismatch(and, with no other trigger visible,no-update-trigger-configured) blocker and disabled the Update button.AgentClient.isRegisteringComponentsflag: true only for the deregisterβre-register span inside the handshake, reset in afinallyand force-cleared byscheduleReconnect()β offline/backoff agents keep hard blockers (Codex review caught thatisConnected === falsewas overbroad).blocker.severity, zero UI changes.request-update.tsdeliberately does not wire the callback, so a manual update during the window still fail-closed rejects β Codex flagged that softening admission could enqueue through a wrong-agent user-selected trigger.Review provenance: adversarial verify round (extended the fix to the no-trigger branch), independent Codex review (2 findings, both repaired), re-verified approve, full app suite green (12,518).
Changelog
π Fixed transient
agent-mismatchandno-update-trigger-configuredblockers during agent component registration.β¨ Added
AgentClient.isRegisteringComponentsfor the deregistration and re-registration window.π§ Downgraded affected display blockers to soft in update eligibility, container lists, and SSE enrichment.
π Preserved fail-closed manual update admission during registration.
β¨ Added coverage for handshakes, reconnects, eligibility, container lists, SSE enrichment, and update admission.
Confirm that all display surfaces use
isAgentPendingRegistration.Keep
request-update.tsindependent of the display-only registration callback.