Skip to content

feat: persist the localAdoptionServer probe field#791

Open
PavelKopecky wants to merge 5 commits intomasterfrom
automated-adoption-flow
Open

feat: persist the localAdoptionServer probe field#791
PavelKopecky wants to merge 5 commits intomasterfrom
automated-adoption-flow

Conversation

@PavelKopecky
Copy link
Contributor

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

Walkthrough

Adds support for a nullable JSON localAdoptionServer field across the probe stack: migration column for gp_probes; new LocalAdoptionServer type and localAdoptionServerSchema; plumbing in AdoptedProbes (DB mapping, fetch/format/update) to persist and expose the field on DProbe; WebSocket subscription probe:adoption:ready with a handler that validates and assigns probe.localAdoptionServer; adoption-related messages now include an adopted boolean. Tests and fixtures updated to expect the new fields.

Possibly related PRs

Suggested reviewers

  • MartinKolarik
  • alexey-yarmosh
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding persistence for the localAdoptionServer probe field, which is the core focus of this PR.
Description check ✅ Passed The description is related to the changeset by referencing the external requirement that motivates the changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch automated-adoption-flow

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/lib/override/adopted-probes.ts`:
- Around line 213-223: The format function for localAdoptionServer returns
objects with malformed expiresAt because Date.parse can be NaN and the current
check (parsedDate && parsedDate <= Date.now()) lets NaN bypass expiry; change
the expiry check in localAdoptionServer.format so that parsedDate is treated as
expired when it's not a finite number (e.g., use Number.isFinite(parsedDate) or
isFinite(parsedDate) and return null when parsedDate is non-finite or already <=
Date.now()), keeping the existing early-return conditions (value falsy or
dProbe?.userId) intact.
🧹 Nitpick comments (2)
src/probe/handler/local-adoption-server.ts (2)

1-2: Consider consolidating imports from the same module.

Both SocketProbe and LocalAdoptionServer are imported from '../types.js'. They can be combined into a single import statement.

♻️ Proposed refactor
-import type { SocketProbe } from '../types.js';
-import type { LocalAdoptionServer } from '../types.js';
+import type { SocketProbe, LocalAdoptionServer } from '../types.js';

5-14: Use validation.value instead of raw data for assignment.

Joi's validate() may coerce or transform the input. Using validation.value ensures any Joi transformations (like type coercion or default values) are applied to the stored data.

♻️ Proposed fix
 export const handleAdoptionServerStart = (probe: SocketProbe) => async (data: LocalAdoptionServer, callback?: (arg: string) => void) => {
 	const validation = localAdoptionServerSchema.validate(data);

 	if (validation.error) {
 		callback?.('error');
 		throw validation.error;
 	}

-	probe.localAdoptionServer = data;
+	probe.localAdoptionServer = validation.value;
 };

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/probe/handler/local-adoption-server.ts`:
- Around line 4-13: The handler handleAdoptionServerStart currently only invokes
the ack callback on validation failure; update it to also call the callback on
success so socket clients receive an acknowledgement. After setting
probe.localAdoptionServer = validation.value in handleAdoptionServerStart,
invoke callback?.('ok') (or callback?.(null) if you prefer a null/no-error
convention) so successful paths mirror the error path's acknowledgement
behavior.
🧹 Nitpick comments (1)
src/probe/handler/local-adoption-server.ts (1)

4-4: async is unused.

The function contains no await expressions. Either remove async or add a brief comment if it's kept intentionally for interface consistency with other handlers.

Proposed fix
-export const handleAdoptionServerStart = (probe: SocketProbe) => async (data: LocalAdoptionServer, callback?: (arg: string) => void) => {
+export const handleAdoptionServerStart = (probe: SocketProbe) => (data: LocalAdoptionServer, callback?: (arg: string) => void) => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments