Skip to content

fix(proxy): reject conflicting multi-instance admin options - #17

Merged
soulteary merged 3 commits into
mainfrom
fix/proxy-admin-option-conflicts
Sep 6, 2026
Merged

soulteary merged 3 commits into
mainfrom
fix/proxy-admin-option-conflicts

Conversation

@soulteary

Copy link
Copy Markdown
Owner

Summary

  • validate process-wide stats and Web settings before starting proxy instances
  • reject conflicting endpoint addresses, public-access policies, and Web event capacities
  • remove the previous order-dependent “first non-empty value wins” behavior
  • add focused tests for every conflict class and the consistent configuration path

Why

Multi-instance proxy processes expose one shared stats endpoint and one shared Web panel. Silently selecting the first declaration made security and observability behavior depend on YAML ordering, including whether an endpoint was publicly exposed.

Validation

Existing CI plus new unit coverage exercises address, allow_public, and web_log_max consolidation.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 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-06T13:56:53.134404Z 0918fb2 Manual request
ℹ️ 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: 2c9b28f36a

ℹ️ 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 main.go Outdated
Comment on lines +1108 to +1109
if webSet && out.webLogMax != opt.webLogMax {
return out, errors.New(i18n.T(i18n.KeyErrConflictingMultiOption, "web_log_max", out.webLogMax, opt.webLogMax))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Ignore omitted capacities when detecting conflicts

When multiple proxy entries repeat the same web_addr, an entry that omits web_log_max has already been expanded to the default 1000 before this comparison. Consequently, a valid configuration where one entry omits the setting and another explicitly sets (for example) 200 is rejected as a 1000-vs-200 conflict, preventing the proxy from starting. Preserve whether ProxyConfig.WebLogMax was explicitly provided and only treat differing explicit capacities as conflicts, as the forward multi-instance collector already does.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 3d69353b29

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

@soulteary
soulteary merged commit bed299e into main Sep 6, 2026
9 checks passed
@soulteary
soulteary deleted the fix/proxy-admin-option-conflicts branch September 6, 2026 13:52

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: bed299ea10

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

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