docs: annotate ADR 0046 with allowed_ips validation results

[maruiz93]

Summary

• Annotates ADR 0046 (host-side API server design) with validation results from OpenShell PR fix(#1559): preserve prior successful review on agent failure #1560
• allowed_ips is no longer required when endpoints are explicitly declared with host+port — validated on OpenShell v0.0.83 (fullsend-ai/experiments#42)
• The 0.0.0.0 bind requirement and NVIDIA/OpenShell#1633 remain relevant for eliminating all-interface exposure

Test plan

• Annotations follow the ADR template guidelines (short notes linking to newer decisions, no substantial rewrites)
• No changes to Context, Decision, or Consequences prose — only appended annotation notes

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Review · Started 9:44 AM UTC
Commit: 2660d57 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Annotate ADR 0046 with allowed_ips validation results

📝 Documentation 🕐 Less than 10 minutes

AI Description

• Add 2026-07-16 annotations to ADR 0046 referencing OpenShell PR #1560 outcomes.
• Document that allowed_ips is no longer required for explicitly declared endpoints.
• Reaffirm 0.0.0.0 bind risk and keep NVIDIA/OpenShell#1633 as the mitigation path.

Diagram

graph TD
  adr46["ADR 0046 (doc)"] --> note["2026-07-16 annotation"] --> pr1560["OpenShell PR #1560"] --> allowed["`allowed_ips` optional"]
  note --> exp42["Validation: experiments#42"]
  note --> issue1633["OpenShell issue #1633"] --> bind["Mitigate `0.0.0.0` exposure"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Create a follow-up ADR (new decision record)

• ➕ Clear separation between original decision and later validation/changes
• ➕ Better chronological audit trail if the decision meaningfully changed
• ➖ Higher process overhead for a small clarification
• ➖ Harder for readers to find the updated nuance when reading ADR 0046

2. Edit the original Decision/Consequences text directly

• ➕ Readers see the updated facts without scanning annotations
• ➖ Violates ADR immutability expectations; can blur what was known at the time
• ➖ Makes historical reasoning harder to reconstruct

Recommendation: Keep the current approach (short appended annotations with links). This matches ADR best practices for documenting subsequent validation without rewriting the original Context/Decision/Consequences, while still pointing reviewers to the upstream PR and the remaining mitigation work (#1633).

Files changed (1) +12 / -1

Documentation (1) +12 / -1

0046-host-side-api-server-design.mdAdd ADR annotation about 'allowed_ips' no longer being required +12/-1

Add ADR annotation about 'allowed_ips' no longer being required

• Appends a dated note documenting that OpenShell PR #1560 removed the 'allowed_ips' requirement for explicitly declared endpoints, with an external validation link. Adds a second inline note clarifying that the '0.0.0.0' bind risk remains until issue #1633 ships.

docs/ADRs/0046-host-side-api-server-design.md

[github-actions]

Site preview

Preview: https://a7453827-site.fullsend-ai.workers.dev

Commit: 3ddf83f1b8e2d72a66baed7eba6de483a2f791b3

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. ADR edit guidance conflict ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

This PR adds post-acceptance annotations inside Decision/Consequences, but ADR 0046’s embedded
guidance says accepted ADRs are frozen except status/superseding links, while the repo ADR template
explicitly allows minor annotations; this ambiguity can cause future review/process churn.

Code

docs/ADRs/0046-host-side-api-server-design.md[R151-158]

+_Note (2026-07-16): OpenShell PR
+[#1560](https://github.com/NVIDIA/OpenShell/pull/1560) removed the
+`allowed_ips` requirement for explicitly declared endpoints. The `0.0.0.0`
+bind is still needed (rootless Podman cannot bind to the bridge gateway IP),
+but `allowed_ips` templating is no longer part of the setup. Validated in
+[fullsend-ai/experiments#42](https://github.com/fullsend-ai/experiments/pull/42).
+[#1633](https://github.com/NVIDIA/OpenShell/issues/1633) remains relevant for
+eliminating the `0.0.0.0` exposure itself._

Relevance

⭐⭐⭐ High

Repo policy updated to allow minor annotations; team previously clarified ADR mutability rules (PRs
#1966, #161).

PR-#1966
PR-#161

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

ADR 0046’s embedded rule says accepted content is frozen, yet the template explicitly allows minor
annotations, and this PR adds such annotations inside the frozen sections—creating a documented
policy conflict.

docs/ADRs/0046-host-side-api-server-design.md[15-24]
docs/ADRs/0046-host-side-api-server-design.md[151-158]
docs/ADRs/0000-adr-template.md[18-23]
docs/ADRs/0018-scripted-pipeline-for-multi-agent-orchestration.md[21-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
ADR 0046 includes an HTML comment stating that once accepted, its content is frozen and only status changes/superseding links should be added. However, the repository ADR template explicitly allows minor annotations after acceptance. This PR adds annotations, surfacing the policy mismatch.

### Issue Context
This is primarily a governance/maintainability issue: unclear precedence between per-ADR embedded guidance and the repository template guidance.

### Fix Focus Areas
- docs/ADRs/0046-host-side-api-server-design.md[21-24]
- docs/ADRs/0046-host-side-api-server-design.md[151-158]

### Suggested resolutions (pick one)
1) **Update ADR 0046’s embedded comment** to match `0000-adr-template.md` (allow minor annotations but prohibit substantial rewrites), or
2) **Move the new notes** to a clearly designated “Annotations” area allowed by the ADR’s embedded rule (e.g., near Status), keeping Decision/Consequences untouched per the comment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Nonportable # issue reference ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new Consequences annotation uses a bare #1633, which relies on GitHub autolinking and may
render as plain text elsewhere, reducing navigability and making the ADR inconsistent with its other
explicit issue links.

Code

docs/ADRs/0046-host-side-api-server-design.md[R177-179]

+  ships. _Note (2026-07-16): `allowed_ips` templating is no longer required
+  (OpenShell PR [#1560](https://github.com/NVIDIA/OpenShell/pull/1560)). The
+  `0.0.0.0` bind and its attack surface remain until #1633 ships._

Relevance

⭐⭐ Medium

Docs link hygiene is mixed: some link fixes accepted (PR #226) but “add links for plain references”
rejected (PR #770).

PR-#226
PR-#770

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Within the same ADR, the issue is referenced via an explicit markdown link in multiple places, but
the newly added note uses a bare #1633, creating inconsistent linking behavior depending on the
renderer.

docs/ADRs/0046-host-side-api-server-design.md[146-179]
PR-#226

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In `docs/ADRs/0046-host-side-api-server-design.md`, the newly added note under Consequences uses a bare `#1633` reference. This may not produce a clickable link outside GitHub and is inconsistent with other explicit links in the same ADR.

### Issue Context
The same document already links to the issue explicitly elsewhere (e.g., `NVIDIA/OpenShell#1633`).

### Fix Focus Areas
- docs/ADRs/0046-host-side-api-server-design.md[177-179]

### Suggested change
Replace `#1633` with an explicit markdown link, e.g. `[NVIDIA/OpenShell#1633](https://github.com/NVIDIA/OpenShell/issues/1633)` (or `[#1633](https://github.com/NVIDIA/OpenShell/issues/1633)` for consistency with the earlier note).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:53 AM UTC · Completed 10:03 AM UTC
Commit: 6b66758 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [adr-immutability-violation] docs/ADRs/0046-host-side-api-server-design.md:21 — The PR replaces the ADR's inline immutability comment (lines 21–26) with a more permissive version. The original restricted post-acceptance edits to "status changes and links to superseding ADRs"; the replacement permits "minor annotations", "cross-references", "short notes", and "clarifying remarks". While AGENTS.md already permits minor annotations on Accepted ADRs, rewriting the governance comment within a specific Accepted ADR is itself a substantive edit — not a minor annotation. The two > **Note (2026-07):** annotation blocks are valid minor annotations and can land independently.
  Remediation: Drop the immutability comment rewrite from this PR. If the inline comment template needs updating repo-wide, handle that separately and consistently across all ADRs.

---

Labels: PR modifies ADR documentation under docs/ADRs/.

Previous run

Review

Findings

Low

• [annotation-formatting] docs/ADRs/0046-host-side-api-server-design.md:151 — Annotation formatting diverges from de facto convention. All five existing ADR annotations (ADRs 0002, 0033, 0034, 0066) use blockquote with bold and month precision: > **Note (YYYY-MM):**. This PR uses inline italic with day precision: _Note (2026-07-16):. No written rule mandates this format, but the convention is universal across all existing annotations.
  Remediation: Change both annotations to use blockquote with bold and month precision: > **Note (2026-07):**. This aligns with the pattern in ADRs 0002, 0033, 0034, and 0066.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:11 AM UTC · Completed 10:25 AM UTC
Commit: 3ddf83f · View workflow run →

[rh-hemartin]

LGTM even if I find confusing that Podman is mentioned here, they just happen to share "0.0.0.0" as string somewhere in their configurations.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 10:29 AM UTC · Completed 10:42 AM UTC
Commit: 3ddf83f · View workflow run →

[fullsend-ai-retro]

Retro: PR #5234 — docs: annotate ADR 0046 with allowed_ips validation results

Timeline

1. 2026-07-17 09:43 UTC — PR opened by maruiz93. Single-file docs change (+19/−4) annotating ADR 0046 with validation results from OpenShell PR fix(#1559): preserve prior successful review on agent failure #1560.
2. 09:44 — Qodo review bot and fullsend review agent Run 1 start in parallel.
3. 09:46 — Qodo posts findings: (a) bare #1633 reference, (b) ADR governance comment conflicts with repo template.
4. 09:52 — Author pushes commit 6b66758 fixing both Qodo findings (rewrites governance HTML comment to match template, adds explicit markdown link). Run 1 is cancelled mid-execution (~9 min wasted).
5. 09:53 — Review Run 2 starts on the fixed commit. Completes at 10:03. Finds annotation-formatting convention divergence (low severity). Approves the PR.
6. 10:10 — Author pushes commit 3ddf83f fixing annotation formatting (italic→blockquote, day→month precision).
7. 10:11 — Review Run 3 starts (incremental review). Completes at 10:25. Finds the governance comment rewrite is itself a substantive edit to an accepted ADR (medium severity). Adds requires-manual-review label.
8. 13:42 — Human reviewer rh-hemartin approves with LGTM.
9. 2026-07-21 10:26 — PR merged.

Assessment

The review system worked well overall. The fullsend review agent caught legitimate issues across both successful runs: annotation formatting convention (Run 2) and governance comment scope concern (Run 3). The requires-manual-review label correctly routed the governance policy question to a human, who exercised judgment to approve.

Piecemeal finding discovery across runs (evidence for #4970, #5107). Run 2 reviewed the full PR diff (including the governance comment rewrite) and approved with only a low-severity formatting finding. Run 3 then caught the governance comment rewrite as medium severity. Transcript analysis shows Run 2's sub-agents all saw the governance comment change but classified it as benign template alignment. The gap was in scope/process assessment: no sub-agent evaluated whether bundling a governance comment rewrite into an annotation PR was appropriate. This is a concrete instance of findings being discovered piecemeal across iterations rather than in a single pass.

Cascading fix pattern across review bots (evidence for #664). Qodo identified a conflict between the ADR's embedded governance comment and the repo template. The author resolved it by rewriting the governance comment — which the fullsend review agent then flagged as a substantive edit. If the review agent had incorporated Qodo's prior finding and the author's response (per #664), it could have contextualized the governance comment change as a deliberate response to legitimate feedback rather than an unprompted substantive edit.

Auto-fixable low-severity finding (evidence for #5001). The annotation-formatting finding (italic→blockquote, day→month precision) was mechanically fixable. Suggesting /fs-fix could have saved the author a manual push and avoided triggering a third review run.

Human approved with unresolved medium finding (evidence for #2099). The human reviewer approved and merged without addressing Run 3's medium-severity governance comment finding. This may reflect appropriate human judgment (the rewrite aligns with AGENTS.md policy) or indicate the finding wasn't read. The ambiguity is exactly the scenario #2099 aims to surface.

Positive signals:

• All review agent findings were technically valid and well-reasoned.
• The incremental review (Run 3) correctly preserved Run 2's finding as historical context.
• The requires-manual-review label worked as intended for the governance edge case.
• Total review compute (~35 min across 3 runs, ~9 min wasted on cancellation) was reasonable for the value delivered.