Skip to content

id(discovery): refuse single-sided topic/secret pair under --no-default-topic (#4756) - #4763

Open
dezren39 wants to merge 1 commit into
mainfrom
queue/6754-4756-no-default-topic-pair
Open

id(discovery): refuse single-sided topic/secret pair under --no-default-topic (#4756)#4763
dezren39 wants to merge 1 commit into
mainfrom
queue/6754-4756-no-default-topic-pair

Conversation

@dezren39

@dezren39 dezren39 commented Nov 3, 2026 •
edited by blacksmith-sh Bot
Loading

Copy link
Copy Markdown
Member

Fixes #4756.

Problem

resolve_config resolved the gossip topic and secret as independent fallback chains, so --no-default-topic with only one of --topic / --topic-secret silently paired the custom half with the public hardcoded default for the other half — contradicting the step-7 cascade doc.

Change (one behaviour, documented)

Under --no-default-topic, topic + secret now resolve as a pair:

  • both CLI values when both are given,
  • both hardcoded constants when neither is given,
  • Err naming the missing flag (--topic / --topic-secret) when exactly one is given — a custom half is never paired with a public default half.

resolve_config now returns anyhow::Result<ResolvedConfig>; the serve and peers call sites propagate with ?. The non-no_default_topic cascade (conf pair, per-field CLI override) is unchanged. Step-7 cascade doc updated with an # Errors section.

Tests

  • New: test_resolve_config_no_default_topic_with_only_cli_topic_is_refused, test_resolve_config_no_default_topic_with_only_cli_secret_is_refused (both assert the error names the missing flag).
  • Existing test_resolve_config_no_default_topic* updated for the Result return and keep passing.
  • cargo test -p id-core: 283 passed, 1 ignored; doctests 5 passed.
  • Note: cargo check --no-default-features on the parent id crate has pre-existing E0432/E0433 failures in chat.rs/invite.rs (verified identical on pristine tree); no new errors from this change.

Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

@dezren39
id(discovery): refuse single-sided topic/secret pair under --no-defau…
7cb1ffb 
…lt-topic (#4756)

resolve_config treated the gossip topic and its secret as independent
fallback chains, so --no-default-topic with only one of --topic /
--topic-secret silently paired the custom half with the public
hardcoded default for the other half.

Resolve the two as a pair under --no-default-topic: both CLI values
when both are given, both hardcoded constants when neither is, and an
error naming the missing flag when exactly one is given. Step-7
cascade doc updated; non-no_default_topic cascade unchanged.

Tests: two new single-sided refusal cases; existing
test_resolve_config_no_default_topic* keep passing (updated for the
Result return). Callers (serve, peers) propagate the error with ?.
@dezren39
dezren39 requested a review from a team as a code owner November 3, 2026 05:02
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 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 ⚠️ Failed 2026-11-03T05:02:07.380769Z
We couldn’t complete this review. Comment @codex review to try again.		 7cb1ffb PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 52 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ec5ca711-2c64-4722-ba22-93625576ae5c
Commits

Reviewing files that changed from the base of the PR and between c0e61d0 and 7cb1ffb.

Files selected for processing: 3
  • pkgs/id/crates/id-core/src/discovery.rs
  • pkgs/id/src/commands/peers.rs
  • pkgs/id/src/commands/serve.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@dezren39 dezren39 mentioned this pull request Nov 3, 2026
Open

This branch has not been deployed

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

Reviewers

No reviews

Assignees

@dezren39 dezren39

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

id(discovery): resolve_config pairs a CLI topic/secret with the public default for the other half under --no-default-topic

1 participant

@dezren39