Skip to content

🐛 fix(eligibility): soften mismatch blockers during agent trigger re-registration (display only) #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
scttbnsn merged 2 commits into from
Aug 4, 2026
Merged

🐛 fix(eligibility): soften mismatch blockers during agent trigger re-registration (display only) #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1451b34
🐛 fix(update-eligibility): soften agent-mismatch during trigger regis…
scttbnsn Aug 4, 2026
487c16b
🐛 fix(agent): set isRegisteringComponents during edge component sync
scttbnsn Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **"Agent Mismatch" no longer appears in the container list/SSE display during the brief window an agent's docker/dockercompose trigger is still (re)registering** ([#605](https://github.com/CodesWhat/drydock/issues/605)). Eligibility is recomputed live on every read, and `AgentClient._doHandshake()` deregisters the agent's components before awaiting the `/api/triggers` fetch and re-register. A read in that window found zero triggers for the agent and `computeUpdateEligibility` raised a hard `agent-mismatch` blocker, disabling the Update button, even though nothing was actually misconfigured — the condition self-corrected once registration finished. `agent-mismatch` now downgrades to a soft blocker (button stays enabled) on display surfaces whenever the container's own agent is mid-registration, per the new `AgentClient.isRegisteringComponents` flag (true only for the deregister→re-register span, not the whole reconnect backoff). Update **admission** (`app/updates/request-update.ts`) is unaffected and stays hard/fail-closed throughout, so an update can never be enqueued through a wrong-agent trigger during that window.
- **WebSocket log streams no longer reject anonymous-auth sessions** ([#636](https://github.com/CodesWhat/drydock/issues/636)). Both WS upgrade paths — the system log stream and the container log stream — gated on `isAuthenticatedSession()` requiring `session.passport.user`, which `passport-anonymous` never sets, so under `DD_ANONYMOUS_AUTH_CONFIRM=true` the log stream WebSocket always rejected the upgrade even though every REST endpoint worked. `isAuthenticatedSession` now also accepts the session when anonymous authentication is the registered mode.
- **Maturity clock: swallowed auth errors surfaced, per-container threshold respected** ([#604](https://github.com/CodesWhat/drydock/issues/604)). `getImagePublishedAt` failures — including GHCR/LSCR 401/403 auth errors — now log at `warn` instead of `debug`, so the maturity gate's silent fallback from the registry `publishedAt` to `updateDetectedAt` is no longer invisible. `getRawUpdateMaturityLevel` (`app/model/container.ts`) and `getContainerMaturityLevel` (`app/api/container/maturity-filter.ts`) now resolve each container's own `updatePolicy.maturityMinAgeDays` before falling back to the global `DD_UI_MATURITY_THRESHOLD_DAYS`, matching the gate's own `isUpdateSuppressed`/`isMaturityGatePending` logic so the hot/mature badge can no longer disagree with the gate in the same API response.
- **Container start/stop/restart/rollback return an explicit 501 instead of an ambiguous 404 for agent containers without lifecycle transport** ([#637](https://github.com/CodesWhat/drydock/issues/637)). `POST /:id/start|stop|restart` and `POST /:id/rollback` returned a bare 404 `No docker trigger found for this container` whenever the lookup missed, indistinguishable from "container not found" — for agent-owned containers this was the only signal the UI got. That lookup miss now returns 501 naming the likely cause (the agent's connection typically hasn't advertised `usesControllerDockerTransport`) when `container.agent` is set; non-agent containers still get the existing 404. This complements the native-transport support that shipped in rc.11 via [#651](https://github.com/CodesWhat/drydock/pull/651), which closed #637's core gap — this is the remaining explicit-error half.
Expand Down
98 changes: 97 additions & 1 deletion app/agent/AgentClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,50 @@ describe('AgentClient', () => {
expect(client.isConnected).toBe(true);
});

test('sets isRegisteringComponents for the deregister -> re-register span and resets after completion (#605)', async () => {
const observedDuringDeregister: boolean[] = [];
const observedDuringWatcherFetch: boolean[] = [];
const observedDuringTriggerFetch: boolean[] = [];

vi.mocked(registry.deregisterAgentComponents).mockImplementationOnce(async () => {
observedDuringDeregister.push(client.isRegisteringComponents);
});

axios.get
.mockResolvedValueOnce({ data: [] }) // containers
.mockImplementationOnce(async () => {
observedDuringWatcherFetch.push(client.isRegisteringComponents);
return { data: [] };
})
.mockImplementationOnce(async () => {
observedDuringTriggerFetch.push(client.isRegisteringComponents);
return { data: [] };
});

storeContainer.getContainers.mockReturnValue([]);

expect(client.isRegisteringComponents).toBe(false);
await client.handshake();

expect(observedDuringDeregister).toEqual([true]);
expect(observedDuringWatcherFetch).toEqual([true]);
expect(observedDuringTriggerFetch).toEqual([true]);
expect(client.isRegisteringComponents).toBe(false);
});

test('resets isRegisteringComponents to false when a mid-handshake step throws (#605)', async () => {
axios.get.mockResolvedValueOnce({ data: [] }); // containers succeeds
vi.mocked(registry.deregisterAgentComponents).mockRejectedValueOnce(
new Error('deregister failed'),
);
storeContainer.getContainers.mockReturnValue([]);

await expect(client.handshake()).rejects.toThrow('deregister failed');

expect(client.isRegisteringComponents).toBe(false);
expect(client.isConnected).toBe(false);
});

test('should emit agent-connected when transitioning to connected state', async () => {
axios.get
.mockResolvedValueOnce({ data: [] })
Expand Down Expand Up @@ -1120,6 +1164,13 @@ describe('AgentClient', () => {
expect(spy).toHaveBeenCalled();
});

test('should reset isRegisteringComponents to false on disconnect (#605)', () => {
client.isConnected = true;
client.isRegisteringComponents = true;
client.scheduleReconnect(1000);
expect(client.isRegisteringComponents).toBe(false);
});

Comment on lines +1167 to +1173

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.ts

Repository: 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.ts

Repository: 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.ts

Repository: 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')
PY

Repository: 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.

test('should not schedule duplicate reconnects', () => {
const spy = vi.spyOn(client, 'startSse').mockImplementation(() => {});
client.scheduleReconnect(1000);
Expand Down Expand Up @@ -7575,10 +7626,23 @@ describe('AgentClient', () => {
});

describe('handleComponentSync (edge agent public shim)', () => {
test('deregisters agent components and re-registers watchers and triggers', async () => {
test('keeps isRegisteringComponents true through edge component replacement and resets it after success (#605)', async () => {
const watchers = [{ type: 'docker', name: 'local', configuration: {} }];
const triggers = [{ type: 'mock', name: 'update', configuration: {} }];
const observedSteps: string[] = [];

vi.mocked(registry.deregisterAgentComponents).mockImplementationOnce(async () => {
observedSteps.push(`deregister:${client.isRegisteringComponents}`);
});
vi.mocked(registry.registerComponent)
.mockImplementationOnce(async (component) => {
observedSteps.push(`${component.kind}:${client.isRegisteringComponents}`);
})
.mockImplementationOnce(async (component) => {
observedSteps.push(`${component.kind}:${client.isRegisteringComponents}`);
});

expect(client.isRegisteringComponents).toBe(false);
await client.handleComponentSync(watchers, triggers);

expect(registry.deregisterAgentComponents).toHaveBeenCalledWith('test-agent');
Expand All @@ -7588,6 +7652,38 @@ describe('AgentClient', () => {
expect(registry.registerComponent).toHaveBeenCalledWith(
expect.objectContaining({ kind: 'trigger', provider: 'mock', name: 'update' }),
);
expect(observedSteps).toEqual(['deregister:true', 'watcher:true', 'trigger:true']);
expect(client.isRegisteringComponents).toBe(false);
});

test('resets isRegisteringComponents when edge watcher registration throws (#605)', async () => {
const observedDuringDeregister: boolean[] = [];
let observedDuringWatcherRegistration = false;

vi.mocked(registry.deregisterAgentComponents)
.mockImplementationOnce(async () => {
observedDuringDeregister.push(client.isRegisteringComponents);
})
.mockImplementationOnce(async () => {
observedDuringDeregister.push(client.isRegisteringComponents);
});
vi.mocked(registry.registerComponent).mockImplementationOnce(async () => {
observedDuringWatcherRegistration = client.isRegisteringComponents;
throw new Error('watcher registration failed');
});

await expect(
client.handleComponentSync(
[{ type: 'docker', name: 'local', configuration: {} }],
[{ type: 'mock', name: 'update', configuration: {} }],
),
).rejects.toThrow('watcher registration failed');

expect(observedDuringDeregister).toEqual([true, true]);
expect(observedDuringWatcherRegistration).toBe(true);
expect(registry.deregisterAgentComponents).toHaveBeenCalledTimes(2);
expect(registry.registerComponent).toHaveBeenCalledTimes(1);
expect(client.isRegisteringComponents).toBe(false);
});

test('works with empty watchers and triggers (no-op)', async () => {
Expand Down
137 changes: 86 additions & 51 deletions app/agent/AgentClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,20 @@ export class AgentClient {
// Parsed once at construction when authmode is 'ed25519'; undefined in token mode.
private readonly ed25519PrivateKey?: KeyObject;
public isConnected: boolean;
/**
* True only while this agent's components are being replaced — the span
* inside `_doHandshake()` (and the equivalent edge-path
* `handleComponentSync()`) between deregistering and finishing the
* re-registration (watchers, then triggers).
* Eligibility display surfaces (container list, SSE enrichment)
* read this to soften `agent-mismatch` / `no-update-trigger-configured` to a
* soft blocker during that transient window — see issue #605. Always reset
* in a `finally` so a handshake failure, or a disconnect via
* `scheduleReconnect()`, reverts to the hard-blocker default. Admission
* (`app/updates/request-update.ts`) never reads this field and stays
* fail-closed throughout.
*/
public isRegisteringComponents: boolean;
public info: AgentClientRuntimeInfo;
private reconnectTimer: NodeJS.Timeout | null;
private reconnectAttempts: number;
Expand Down Expand Up @@ -310,6 +324,7 @@ export class AgentClient {
}

this.isConnected = false;
this.isRegisteringComponents = false;
this.info = {};
this.reconnectTimer = null;
this.reconnectAttempts = 0;
Expand Down Expand Up @@ -945,53 +960,62 @@ export class AgentClient {
const containers = response.data;
this.log.info(`Handshake successful. Received ${containers.length} containers.`);

// Unregister existing components for this agent
await registry.deregisterAgentComponents(this.name);

// Fetch and register watchers
// isRegisteringComponents is true for the entire deregister → re-register
// span below, including the container-inventory apply that happens
// between watcher and trigger registration. The `finally` guarantees it
// reverts to false whether registration succeeds or this method throws.
this.isRegisteringComponents = true;
try {
const responseWatchers = await axios.get<AgentComponentDescriptor[]>(
`${this.baseUrl}/api/watchers`,
this.buildRequestConfig('GET', '/api/watchers'),
);
await this.registerAgentWatchersTransactional(responseWatchers.data);
// Only transfer update-enrichment ownership after every controller-side
// watcher/delegate has registered successfully.
this.setControllerDockerTransportWatchers(responseWatchers.data);
this.seedWatcherSnapshotCacheFromHandshake(responseWatchers.data);
} catch (error: unknown) {
this.log.warn(`Failed to fetch/register watchers: ${getErrorMessage(error)}`);
}

// Apply inventory only after watcher registration. Controller-transport
// descriptors change which fields are authoritative: Portwing owns live
// runtime state, while Drydock's native watcher owns update enrichment.
await this.processAuthoritativeContainers(containers);
// A zero-container handshake is ambiguous: it could mean the agent has
// no running containers, or its in-memory store is fresh-empty after a
// restart while docker still has running containers. Defer the prune
// until the first authoritative watcher snapshot arrives — that path is
// unambiguous because the snapshot is only emitted after a successful
// enumeration with no enrichment errors (#362, #386 / d02080ae).
// Pruning here would wipe last-known state for an agent that's about to
// re-populate it in seconds via its first watch cycle.
if (containers.length > 0) {
this.pruneOldContainers(containers);
} else if (this.hasConnectedOnce) {
this.log.warn(
'Handshake returned 0 containers; preserving last-known state until the first watch cycle completes',
);
}
// Unregister existing components for this agent
await registry.deregisterAgentComponents(this.name);

// Fetch and register triggers
try {
const responseTriggers = await axios.get<AgentComponentDescriptor[]>(
`${this.baseUrl}/api/triggers`,
this.buildRequestConfig('GET', '/api/triggers'),
);
await this.registerAgentComponents('trigger', responseTriggers.data);
} catch (error: unknown) {
this.log.warn(`Failed to fetch/register triggers: ${getErrorMessage(error)}`);
// Fetch and register watchers
try {
const responseWatchers = await axios.get<AgentComponentDescriptor[]>(
`${this.baseUrl}/api/watchers`,
this.buildRequestConfig('GET', '/api/watchers'),
);
await this.registerAgentWatchersTransactional(responseWatchers.data);
// Only transfer update-enrichment ownership after every controller-side
// watcher/delegate has registered successfully.
this.setControllerDockerTransportWatchers(responseWatchers.data);
this.seedWatcherSnapshotCacheFromHandshake(responseWatchers.data);
} catch (error: unknown) {
this.log.warn(`Failed to fetch/register watchers: ${getErrorMessage(error)}`);
}

// Apply inventory only after watcher registration. Controller-transport
// descriptors change which fields are authoritative: Portwing owns live
// runtime state, while Drydock's native watcher owns update enrichment.
await this.processAuthoritativeContainers(containers);
// A zero-container handshake is ambiguous: it could mean the agent has
// no running containers, or its in-memory store is fresh-empty after a
// restart while docker still has running containers. Defer the prune
// until the first authoritative watcher snapshot arrives — that path is
// unambiguous because the snapshot is only emitted after a successful
// enumeration with no enrichment errors (#362, #386 / d02080ae).
// Pruning here would wipe last-known state for an agent that's about to
// re-populate it in seconds via its first watch cycle.
if (containers.length > 0) {
this.pruneOldContainers(containers);
} else if (this.hasConnectedOnce) {
this.log.warn(
'Handshake returned 0 containers; preserving last-known state until the first watch cycle completes',
);
}

// Fetch and register triggers
try {
const responseTriggers = await axios.get<AgentComponentDescriptor[]>(
`${this.baseUrl}/api/triggers`,
this.buildRequestConfig('GET', '/api/triggers'),
);
await this.registerAgentComponents('trigger', responseTriggers.data);
} catch (error: unknown) {
this.log.warn(`Failed to fetch/register triggers: ${getErrorMessage(error)}`);
}
} finally {
this.isRegisteringComponents = false;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

this.isConnected = true;
Expand Down Expand Up @@ -1070,6 +1094,10 @@ export class AgentClient {
const reconnectDelay = delay ?? this.getNextReconnectDelayMs();
const wasConnected = this.isConnected;
this.isConnected = false;
// A disconnect is never a "still registering" state — it's a hard loss of
// the agent. Reset unconditionally so a disconnect that races a still-running
// _doHandshake() cannot leave eligibility softened after the connection drops.
this.isRegisteringComponents = false;
if (wasConnected) {
void emitAgentDisconnected({
agentName: this.name,
Expand Down Expand Up @@ -2157,12 +2185,19 @@ export class AgentClient {
watchers: AgentComponentDescriptor[],
triggers: AgentComponentDescriptor[],
): Promise<void> {
this.setControllerDockerTransportWatchers([]);
await registry.deregisterAgentComponents(this.name);
await this.registerAgentWatchersTransactional(watchers);
this.setControllerDockerTransportWatchers(watchers);
this.seedWatcherSnapshotCacheFromHandshake(watchers);
await this.registerAgentComponents('trigger', triggers);
// Same deregister → re-register window as _doHandshake(): keep transient
// eligibility blockers soft while components are being replaced.
this.isRegisteringComponents = true;
try {
this.setControllerDockerTransportWatchers([]);
await registry.deregisterAgentComponents(this.name);
await this.registerAgentWatchersTransactional(watchers);
this.setControllerDockerTransportWatchers(watchers);
this.seedWatcherSnapshotCacheFromHandshake(watchers);
await this.registerAgentComponents('trigger', triggers);
} finally {
this.isRegisteringComponents = false;
}
}

/**
Expand Down
Loading
Loading