Skip to content

Fix Fanvil provisioning server replacement - #76

Open
Amygos wants to merge 3 commits into
masterfrom
fix/fanvil-registration
Open

Amygos wants to merge 3 commits into
masterfrom
fix/fanvil-registration

Conversation

@Amygos

@Amygos Amygos commented Sep 14, 2026

Copy link
Copy Markdown
Member

Summary

Re-registering an existing Fanvil phone fails with Error:server_not_existed because its server is deleted before the phone is deregistered. The provisioning URL cannot be updated while the server remains in use.

Deregister the phone before replacing its MAC-named server and registering it again. Check the deregistration response while allowing an absent device, accept both missing-server error spellings during deletion, and return errors safely when they have no wrapped cause.

Validate the replacement URL before changing the remote state, serialize updates for the same MAC, and use a bounded HTTP timeout. If a provider error interrupts the replacement, Falconieri returns its existing HTTP 500 error response. The client can repeat the same PUT: cleanup accepts an already absent phone or server, so the request can resume from any partial state. The phone can remain unregistered until a retry succeeds.

The flow continues to use one server per phone, named after its MAC address.

How to test

  1. Build with go build and start Falconieri with a configured Fanvil account.
  2. Send PUT /providers/fanvil/<MAC> with a provisioning URL for a test phone, then repeat the request with an updated URL. Both requests should return HTTP 200.
  3. Inject a temporary failure before or after each Fanvil operation. Confirm that the request returns HTTP 500, then repeat it and confirm it succeeds.
  4. Confirm that the phone remains registered to its MAC-named server, the provisioning settings are updated, and other device registrations are preserved.

Validation performed:

  • Local failure and retry tests pass with the race detector; go vet ./... and go build ./... pass.
  • The supplied local provider script returned HTTP 200 for all six requests and its YMCS PIN check passed.
  • Two additional Fanvil requests returned HTTP 200; readback confirmed the phone is registered to the expected MAC-named group and provisioning host.
  • Staticcheck reports four pre-existing warnings outside the changed Fanvil code.

Accept both missing-server responses during cleanup so first-time
registrations can continue. Return other errors safely when they
have no wrapped cause.

Assisted-by: Codex:GPT-6-Astra
Fanvil refuses to delete a server while its phone is registered.
Deregister the phone before replacing its server to prevent the
subsequent creation from failing because the name already exists.

Check the deregistration response and allow an absent device.

Assisted-by: Codex:GPT-6-Astra
@Amygos Amygos self-assigned this Sep 14, 2026
@Amygos
Amygos marked this pull request as ready for review September 14, 2026 10:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T10:35:41.366713Z e409e0a Draft marked ready
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e409e0a9ed

ℹ️ 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".

Comment thread providers/fanvil.go
Amygos added a commit that referenced this pull request Sep 14, 2026
Snapshot managed settings and registration before replacing a Fanvil
group. Read back uncertain mutations and make one verified recovery pass
so failed updates restore the original registered or unregistered state.
Serialize updates per phone and return success for completed or
identical requests.

Use structured XML/JSON decoding with Fanvil's response charsets and a
30-second timeout. Keep recovery details free of configuration values.

Verified with local failure injection and race tests, go vet, go build,
and live test-phone recovery plus the supplied Fanvil script case.

Review:
#76 (comment)

Assisted-by: Codex:GPT-6-Astra
Comment thread providers/fanvil_client.go Fixed
@Amygos
Amygos force-pushed the fix/fanvil-registration branch from 61c0f23 to e409e0a Compare September 14, 2026 11:44
Amygos added a commit that referenced this pull request Sep 14, 2026
Snapshot managed settings and registration before replacing a Fanvil
group. Read back uncertain mutations and make one verified recovery pass
so failed updates restore the original registered or unregistered state.
Serialize updates per phone and return success for completed or
identical requests.

Use structured XML and JSON decoding with a 30-second timeout. Keep
recovery details free of configuration values and preserve the existing
Fanvil XML-RPC credential transform.

Review:
#76 (comment)

Assisted-by: Codex:GPT-5.6-Sol
Amygos added a commit that referenced this pull request Sep 14, 2026
Validate the replacement URL before deregistering the phone. Serialize
updates for the same MAC and bound every Fanvil request with a timeout.

Missing devices and groups remain valid cleanup states, so clients can
repeat the request after a partial or transient failure. The phone can
remain unregistered until a retry succeeds.

Review:
#76 (comment)

Assisted-by: Codex:GPT-5.6-Sol
@Amygos
Amygos force-pushed the fix/fanvil-registration branch from 61d1d05 to c9a12da Compare September 14, 2026 12:21
Comment thread providers/fanvil.go Fixed
Validate the replacement URL before deregistering the phone. Serialize
updates for the same MAC and bound every Fanvil request with a timeout.

Missing devices and groups remain valid cleanup states, so clients can
repeat the request after a partial or transient failure. The phone can
remain unregistered until a retry succeeds.

Review:
#76 (comment)

Assisted-by: Codex:GPT-5.6-Sol
@Amygos
Amygos force-pushed the fix/fanvil-registration branch from c9a12da to 4089d16 Compare September 14, 2026 12:24
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.

2 participants