join: follow proxy_to perimeters for contract redirect #202

tuhalf · 2025-12-18T10:40:31Z

Resolve proxy_to redirect chains during diode join contract sync so “home” perimeters can proxy devices into operating perimeters (supports nested redirects). Apply k/vs from the effective (proxied) perimeter with loop detection and max depth, and fall back to the last working perimeter when a target has no readable properties. Refactor on-chain property fetch helpers to take an explicit contract address and print the proxy chain/effective perimeter when it changes.

Note

Follow proxy_to chains to select an effective perimeter (with loop detection, max depth, and fallback) and refactor contract property fetching to target explicit contracts, improving logging and trimming values.

Join/Contract Resolution:
- Add proxy_to chain resolution (buildProxyToChain) with loop detection, address validation, and max depth, plus fallback selection (selectContractPropsWithFallback).
- contractSync now builds the chain, chooses the effective contract, applies its props, and prints proxy chain/effective perimeter when changed.
Contract Fetching:
- Refactor batch getter to getPropertyValuesAt(deviceAddr, contractAddr, keys) and fetchContractPropsFromContract(...) to fetch from an explicit contract.
- Trim whitespace from fetched values and enhance debug logs to include contract and result summaries.
State/Logging:
- Track lastEffectiveContract and lastProxyToChain to control output.
- Improved error handling and partial-result reporting during sync.

^{Written by Cursor Bugbot for commit 9b3b274. This will update automatically on new commits. Configure here.}

gemini-code-assist · 2025-12-18T10:40:49Z

Summary of Changes

Hello @tuhalf, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the diode join contract synchronization mechanism by introducing robust support for proxy_to redirect chains. It ensures that devices can be correctly proxied between perimeters, even with nested redirects, by resolving the full chain, applying configurations from the effective perimeter, and handling potential loops or unreadable contracts gracefully. The changes also include refactoring of contract interaction helpers for improved maintainability and clearer logging of the resolution process.

Highlights

Proxy_to Chain Resolution: Implemented logic to resolve proxy_to redirect chains during diode join contract synchronization, allowing 'home' perimeters to proxy devices into 'operating' perimeters, including support for nested redirects.
Effective Perimeter Property Application: Key-value pairs are now applied from the effective (proxied) perimeter, ensuring configurations are sourced from the correct, final contract in a redirect chain.
Robust Proxy Handling: Added loop detection and a maximum depth limit (16) to prevent infinite redirect loops in proxy_to chains. A fallback mechanism is also in place to use the last working perimeter if a target contract has no readable properties.
Refactored Contract Property Fetching: On-chain property fetch helper functions (getPropertyValues, fetchContractProps) have been refactored to explicitly accept a contract address, improving flexibility and clarity.
Enhanced Logging and Visibility: The system now prints the full proxy_to chain and the effective perimeter when it changes, providing better visibility into the contract resolution process.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

…r only print when the effective perimeter changes

gemini-code-assist

Code Review

This pull request introduces support for proxy_to redirects for join contracts, a significant feature that allows for more flexible perimeter configurations. The implementation correctly handles redirect chain resolution, loop detection, and fallback mechanisms. The associated refactoring of property-fetching functions to be more generic is also well-executed. My review includes a couple of suggestions to improve maintainability and a minor efficiency tweak.

cmd/diode/join.go

tuhalf · 2025-12-22T08:38:54Z

I tested it and it's ready to merge @dominicletz

dominicletz · 2025-12-22T09:39:26Z

cmd/diode/join.go

+		if proxyTo == "" {
+			return chain, fetchErr
+		}
+		if idx := strings.IndexAny(proxyTo, " \t\r\n"); idx >= 0 {


Why not strings.TrimSpace instead?

Actually I think we should just call strings.TrimSpace once in getPropertyValuesAt when doing results[key] = strings.TrimSpace(value)

dominicletz · 2025-12-22T09:46:45Z

cmd/diode/join.go

+			proxyTo = strings.TrimSpace(props["proxy_to"])
+		}
+		if proxyTo == "" {
+			return chain, fetchErr


The re-use of fetchErr is confusing because it's always nil. Better to just return nil here and in the following cases if that is the intention

dominicletz

Added two small comments can ask copilot to fix.

dominicletz · 2025-12-22T09:53:01Z

@copilot update buildProxyToChain to not do it own trim implementation using strings.IndexAny(proxyTo, " \t\r\n") but instead add a strings.TrimSpace call to the getPropertyValuesAt function when filling the results[key] dictionary so that all retrieved property values are always trimmed.

Secondly buildProxyToChain remove the continuous use of fetchErr after the initial fetch and instead use nil as the explicit error return value where that is the intention.

Copilot · 2025-12-22T09:53:16Z

@dominicletz I've opened a new pull request, #204, to work on those changes. Once the pull request is ready, I'll request review from you.

…ProxyToChain (#204) * Initial plan * Refactor trim logic and fix error returns in buildProxyToChain Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

cursor

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

cursor · 2025-12-22T10:07:45Z

cmd/diode/join.go

+		}
+		if proxyTo == "" || strings.EqualFold(proxyTo, current) {
+			return chain, nil
+		}


Duplicate empty string check creates unreachable dead code

The condition proxyTo == "" at line 271 is unreachable dead code because line 268 already checks if proxyTo == "" and returns at line 269. The second if statement only needs to check strings.EqualFold(proxyTo, current). This appears to be leftover code from refactoring the trim logic.

cursor · 2025-12-22T10:07:45Z

cmd/diode/join.go

+	if cfg != nil && cfg.Logger != nil {
+		cfg.Logger.Warn("proxy_to chain exceeded max depth (%d); stopping at %s", maxProxyToDepth, current)
+	}
+	return chain, err


Implicit nil return violates reviewer's explicit request (Bugbot Rules)

The reviewer explicitly requested that buildProxyToChain use nil as the explicit error return value instead of relying on the uninitialized named return variable err. At line 297, return chain, err returns the named return value which happens to be nil, but the reviewer asked for explicit nil to make the intent clear.

join: follow proxy_to perimeters for contract redirect

5cf8119

tuhalf linked an issue Dec 18, 2025 that may be closed by this pull request

Support "join" command for perimeter redirect #178

Closed

Adjusted proxy_to logging so Perimeter Proxy Chain/Effective Perimete…

c2e253e

…r only print when the effective perimeter changes

gemini-code-assist bot reviewed Dec 18, 2025

View reviewed changes

cmd/diode/join.go Show resolved Hide resolved

cmd/diode/join.go Show resolved Hide resolved

tuhalf marked this pull request as ready for review December 22, 2025 08:36

dominicletz reviewed Dec 22, 2025

View reviewed changes

dominicletz requested changes Dec 22, 2025

View reviewed changes

dominicletz assigned Copilot and tuhalf Dec 22, 2025

Copilot AI mentioned this pull request Dec 22, 2025

Centralize property value trimming and clarify error returns in buildProxyToChain #204

Merged

cursor bot reviewed Dec 22, 2025

View reviewed changes

dominicletz merged commit 80a216e into master Dec 22, 2025
17 checks passed

dominicletz deleted the tuhalf/support-join-command-for-perimeter-redirect branch December 22, 2025 11:54

join: follow proxy_to perimeters for contract redirect #202

join: follow proxy_to perimeters for contract redirect #202

Uh oh!

Conversation

tuhalf commented Dec 18, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Dec 18, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

tuhalf commented Dec 22, 2025

Uh oh!

dominicletz Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

dominicletz Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

dominicletz Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dominicletz left a comment

Choose a reason for hiding this comment

Uh oh!

dominicletz commented Dec 22, 2025

Uh oh!

Copilot AI commented Dec 22, 2025

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

cursor bot Dec 22, 2025

Choose a reason for hiding this comment

Duplicate empty string check creates unreachable dead code

Uh oh!

cursor bot Dec 22, 2025

Choose a reason for hiding this comment

Implicit nil return violates reviewer's explicit request (Bugbot Rules)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tuhalf commented Dec 18, 2025 •

edited by cursor bot

Loading

dominicletz Dec 22, 2025 •

edited

Loading