Skip to content

fix(cli): let child profiles override inherited allow_http2 - #1684

Draft
Frankie-Xu wants to merge 1 commit into
nolabs-ai:mainfrom
Frankie-Xu:fix/1632-http2-profile-override
Draft

fix(cli): let child profiles override inherited allow_http2#1684
Frankie-Xu wants to merge 1 commit into
nolabs-ai:mainfrom
Frankie-Xu:fix/1632-http2-profile-override

Conversation

@Frankie-Xu

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1632

Summary

Fix profile inheritance for network.allow_http2: the field was OR-merged (base || child), so any ancestor with allow_http2: true permanently forced HTTP/2 on all descendants. A child setting "allow_http2": false was silently ignored.

This PR changes allow_http2 to InheritableValue<bool> (same three-state model as network_profile): absent inherits, explicit true/false overrides, null clears an inherited true. Call sites use resolved_allow_http2(); the CLI --allow-http2 flag is still OR'd with the resolved profile value at launch (unchanged).

Files consulted: crates/nono-cli/src/profile/mod.rs (merge_profiles, InheritableValue), sandbox_prepare.rs, proxy_command.rs, nono-profile.schema.json, profile-authoring-guide.md, docs/cli/features/networking.mdx.

Agent Disclosure

This PR is submitted by an AI coding agent (Cursor AI) on behalf of @Frankie-Xu, per AGENTS.md / CLAUDE.md. Intent and approach were disclosed in #1632 (comment). The change is CLI profile merge only; no library policy changes. This contribution complies with repository coding and security requirements.

Test Plan

  • make ci (clippy, fmt-check, tests, cargo audit, lint-aliases, lint-docs) — passed locally on macOS
  • Unit tests for merge semantics (inherit / override / null-clear / multi-level)
  • Integration tests via extends profile loading
  • proxy_command tests: child false overrides inherited true; CLI --allow-http2 still ORs with resolved profile false
  • Schema shape test: allow_http2 accepts boolean or null

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates

Agent Compliance Check (Required for AI/Automated PRs)

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

Made with Cursor

@github-actions github-actions Bot added bug Something isn't working nono-cli size/large labels Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +308
Lines removed -15
Total changed 323
Classification Large (> 300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Broad

This PR touches: source code,documentation,configuration / policy files


Updated automatically on each push to this PR.

@nogent-nolabs-ai nogent-nolabs-ai 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.

nogent code review

No issues found.

Findings: none flagged in scope.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Signed-off-by: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@Frankie-Xu
Frankie-Xu force-pushed the fix/1632-http2-profile-override branch from 66217ed to 64100bd Compare August 21, 2026 06:38
@Frankie-Xu

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (includes v0.74.0 / #1685 / #1686). Resolved a conflict in proxy_command.rs by keeping both the new --extends tests and the allow_http2 inheritance override tests. InheritableValue<bool> merge semantics are unchanged. Local make ci is green (one unrelated TMPDIR/system_read_macos flake on first test-cli run; re-run passed).

@Frankie-Xu
Frankie-Xu marked this pull request as draft August 21, 2026 10:56
@Frankie-Xu

Copy link
Copy Markdown
Contributor Author

Putting this back to draft so I stay within the 1–2 open PR limit. Clippy failed after the rebase onto main; I'll deal with that while it's a draft. No review needed right now.

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

Labels

bug Something isn't working nono-cli size/large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

network.allow_http2 cannot be disabled by a descendant profile (OR-merge up the inheritance chain)

1 participant