didericis/bot-bottle
1
0
Fork 0
Code Issues 7 Pull Requests 15 Actions Packages Projects Releases Wiki Activity

PRD 0031: Provisioned-wins merge + EgressRoute inherits Route #121

Merged
didericis merged 5 commits from prd-0031 into main 2026-06-02 02:02:23 -04:00
Conversation 7 Commits 5 Files Changed 3
+368 -196
didericis-claude commented 2026-06-02 01:09:11 -04:00
Collaborator
Copy Link
Copy Source

Closes #120.

PRD: ae33d1abfb/docs/prds/0031-split-merge-provider-route.md

Summary

  • Replace _merge_provider_route’s five-case nested conditional with a flat provisioned-wins merge: provider routes own their hosts, manifest routes fill the gaps, token slot assignment is a single final pass.
  • Add _route_to_yaml_fields as the single authoritative EgressRoute→YAML field mapping to prevent silent type drift between EgressRoute and egress_addon_core.Route.
  • Folds in the Route type fragmentation task from #117.

Changes (2 commits)

  • docs(prd): add PRD 0031 — split _merge_provider_route into named case helpers
  • docs(prd): revise PRD 0031 — provisioned-wins merge + Route type consolidation
Closes #120. PRD: https://gitea.dideric.is/didericis/bot-bottle/src/commit/ae33d1abfbb32bc756ab6313a20eaab6bb98d073/docs/prds/0031-split-merge-provider-route.md ## Summary - Replace `_merge_provider_route`’s five-case nested conditional with a flat provisioned-wins merge: provider routes own their hosts, manifest routes fill the gaps, token slot assignment is a single final pass. - Add `_route_to_yaml_fields` as the single authoritative `EgressRoute`→YAML field mapping to prevent silent type drift between `EgressRoute` and `egress_addon_core.Route`. - Folds in the Route type fragmentation task from #117. ## Changes (2 commits) - `docs(prd): add PRD 0031 — split _merge_provider_route into named case helpers` - `docs(prd): revise PRD 0031 — provisioned-wins merge + Route type consolidation`
didericis-claude added 1 commit 2026-06-02 01:09:12 -04:00
docs(prd): add PRD 0031 — split _merge_provider_route into named case helpers
test / unit (pull_request) Successful in 31s
Details
test / integration (pull_request) Successful in 41s
Details
f596464f3f
didericis commented 2026-06-02 01:20:01 -04:00
Owner
Copy Link
Copy Source

Let's fold the other egress issue in the unchecked task item from #117 into this PRD and the issue here #120 ... I think we can probably create a cleaner clean up by first determining how to consolidate our route declarations.

I think the flow should be mostly additive. Hierarchy should be like this:

provisioned_routes -> user_routes

A user can't redeclare a route that's provisioned/provisioning wins (for now at least). Have it match on host.

I also think the request specific logic with the mitmproxy specific logic should probably just live in a pure function, but I'm not sure. Revise the plan with this in mind.

Let's fold the other egress issue in the unchecked task item from https://gitea.dideric.is/didericis/bot-bottle/issues/117 into this PRD and the issue here https://gitea.dideric.is/didericis/bot-bottle/issues/120 ... I think we can probably create a cleaner clean up by first determining how to consolidate our route declarations. I think the flow should be mostly additive. Hierarchy should be like this: provisioned_routes -> user_routes A user can't redeclare a route that's provisioned/provisioning wins (for now at least). Have it match on host. I also think the `request` specific logic with the mitmproxy specific logic should probably just live in a pure function, but I'm not sure. Revise the plan with this in mind.
didericis-claude commented 2026-06-02 01:23:12 -04:00
Author
Collaborator
Copy Link
Copy Source

RE #121 (comment)

Got it — expanding scope and revising the merge design. Here's my read before I update the PRD:

Provisioned-wins merge — this lets us delete almost all of _merge_provider_route. The new flow is: start with provider routes (they own their hosts outright), then append any manifest routes for hosts not already claimed by a provider. No upgrade logic, no conflict detection. Token slot assignment becomes a single pass over the merged list in order. I'll refactor egress_manifest_routes so it doesn't assign slots itself; slot assignment becomes a final step after the merge.

Route type consolidation — egress_addon_core.Route can't go away (it's stdlib-only, runs inside the container where bot_bottle isn't installed). The risk is that EgressRoute and Route drift silently. My plan: add a _to_route_fields(route: EgressRoute) -> dict pure function in egress.py that produces exactly the fields _parse_one expects. egress_render_routes uses it exclusively — one place where the mapping between the two types is spelled out. The existing round-trip test (test_round_trip_through_addon_core) stays as the formal contract.

Mitmproxy pure function — decide() and is_git_push_request() in egress_addon_core already cover all the decision logic as pure functions. request() in egress_addon.py is already minimal mitmproxy glue: read host/path/headers from the flow, call the pure functions, apply the result. The split is clean. I'd leave request() as-is and not restructure further — the main thing missing is the explicit _to_route_fields mapping above.

Updating the PRD now.

RE https://gitea.dideric.is/didericis/bot-bottle/pulls/121#issuecomment-1006 Got it — expanding scope and revising the merge design. Here's my read before I update the PRD: **Provisioned-wins merge** — this lets us delete almost all of `_merge_provider_route`. The new flow is: start with provider routes (they own their hosts outright), then append any manifest routes for hosts not already claimed by a provider. No upgrade logic, no conflict detection. Token slot assignment becomes a single pass over the merged list in order. I'll refactor `egress_manifest_routes` so it doesn't assign slots itself; slot assignment becomes a final step after the merge. **Route type consolidation** — `egress_addon_core.Route` can't go away (it's stdlib-only, runs inside the container where `bot_bottle` isn't installed). The risk is that `EgressRoute` and `Route` drift silently. My plan: add a `_to_route_fields(route: EgressRoute) -> dict` pure function in `egress.py` that produces exactly the fields `_parse_one` expects. `egress_render_routes` uses it exclusively — one place where the mapping between the two types is spelled out. The existing round-trip test (`test_round_trip_through_addon_core`) stays as the formal contract. **Mitmproxy pure function** — `decide()` and `is_git_push_request()` in `egress_addon_core` already cover all the decision logic as pure functions. `request()` in `egress_addon.py` is already minimal mitmproxy glue: read host/path/headers from the flow, call the pure functions, apply the result. The split is clean. I'd leave `request()` as-is and not restructure further — the main thing missing is the explicit `_to_route_fields` mapping above. Updating the PRD now.
👍 1
didericis added 1 commit 2026-06-02 01:26:22 -04:00
docs(prd): revise PRD 0031 — provisioned-wins merge + Route type consolidation
test / unit (pull_request) Successful in 42s
Details
test / integration (pull_request) Successful in 1m0s
Details
ae33d1abfb 
Expands scope to cover both remaining egress hotspot tasks from #117:
- Replaces the named-helper design with a flat provisioned-wins merge
  (provider routes own their hosts; manifest fills gaps; no upgrade or
  conflict-detection logic needed).
- Adds _route_to_yaml_fields as the single authoritative EgressRoute→Route
  mapping to prevent silent type drift between host and addon.
- Notes that the mitmproxy pure-function split is already clean (decide +
  is_git_push_request) and requires no structural change.
didericis added 2 commits 2026-06-02 01:45:37 -04:00
refactor(egress): provisioned-wins merge + _route_to_yaml_fields (PRD 0031) 10d0872043 
Replace _merge_provider_route's five-case nested conditional with a flat
provisioned-wins merge: provider routes claim their hosts outright, manifest
routes for unclaimed hosts append unchanged. Token slot assignment moves to a
single _assign_token_slots pass over the merged list.

Add _route_to_yaml_fields as the single authoritative EgressRoute→YAML mapping,
eliminating the risk of EgressRoute and egress_addon_core.Route silently
drifting apart when new fields are added.

egress_manifest_routes is now a pure lifter with no slot assignment.
_merge_provider_route and _find_or_alloc_token_env are removed.

Tests updated: conflict-die case removed, upgrade-bare replaced with
provider-wins semantics, slot-assignment tests moved to TestSlotAssignment.
complete(prd): mark PRD 0031 active
test / unit (pull_request) Successful in 39s
Details
test / integration (pull_request) Successful in 46s
Details
f15721b424 
Provisioned-wins merge and _route_to_yaml_fields are implemented and all
tests pass.
didericis reviewed 2026-06-02 01:49:24 -04:00
docs/prds/0031-split-merge-provider-route.md
@@ -0,0 +42,4 @@
### 2. Three-way Route type fragmentation
`EgressRoute` (in `egress.py`) and `egress_addon_core.Route` are
separate dataclasses with overlapping but not identical field sets:
didericis commented 2026-06-02 01:49:24 -04:00
Owner
Copy Link
Copy Source

Couldn't we just solve this by having EgressRoute inherit Route?

Couldn't we just solve this by having `EgressRoute` inherit `Route`?
didericis-claude commented 2026-06-02 01:50:53 -04:00
Author
Collaborator
Copy Link
Copy Source

RE #121 (comment)

With EgressRoute inheriting Route (not the other way), the import direction is fine — egress.py imports from egress_addon_core, the addon never imports from egress.py. So the stdlib-only constraint isn't the blocker here.

The real issue is serialization. If EgressRoute extends Route, egress_render_routes can’t just dataclasses.asdict the whole thing — host-only fields (token_ref, roles, tls_passthrough) must not appear in the YAML the addon reads. You’d still need something that says “only serialize the Route base fields” — e.g. iterating dataclasses.fields(Route) explicitly.

That implicit boundary (iterate base class fields to exclude host-only ones) versus the explicit _route_to_yaml_fields function is a real tradeoff. Inheritance gives you a formal subtype relationship and auto-propagates new Route fields to EgressRoute; _route_to_yaml_fields makes the wire boundary a named, documented thing without adding an import coupling between the two modules.

Either works. I went with _route_to_yaml_fields as the minimal change that doesn’t restructure the type hierarchy, but if you prefer the inheritance approach I can revise.

RE https://gitea.dideric.is/didericis/bot-bottle/pulls/121#issuecomment-1018 With `EgressRoute` inheriting `Route` (not the other way), the import direction is fine — `egress.py` imports from `egress_addon_core`, the addon never imports from `egress.py`. So the stdlib-only constraint isn't the blocker here. The real issue is serialization. If `EgressRoute` extends `Route`, `egress_render_routes` can’t just `dataclasses.asdict` the whole thing — host-only fields (`token_ref`, `roles`, `tls_passthrough`) must not appear in the YAML the addon reads. You’d still need something that says “only serialize the `Route` base fields” — e.g. iterating `dataclasses.fields(Route)` explicitly. That implicit boundary (iterate base class fields to exclude host-only ones) versus the explicit `_route_to_yaml_fields` function is a real tradeoff. Inheritance gives you a formal subtype relationship and auto-propagates new `Route` fields to `EgressRoute`; `_route_to_yaml_fields` makes the wire boundary a named, documented thing without adding an import coupling between the two modules. Either works. I went with `_route_to_yaml_fields` as the minimal change that doesn’t restructure the type hierarchy, but if you prefer the inheritance approach I can revise.
didericis commented 2026-06-02 01:56:54 -04:00
Owner
Copy Link
Copy Source

RE #121 (comment)

We want that coupling, that's the whole point of the change. Do the refactor.

RE https://gitea.dideric.is/didericis/bot-bottle/pulls/121#issuecomment-1020 We want that coupling, that's the whole point of the change. Do the refactor.
didericis-claude commented 2026-06-02 01:57:15 -04:00
Author
Collaborator
Copy Link
Copy Source

RE #121 (comment)

On it.

RE https://gitea.dideric.is/didericis/bot-bottle/pulls/121#issuecomment-1021 On it.
didericis added 1 commit 2026-06-02 01:59:04 -04:00
refactor(egress): EgressRoute inherits Route from egress_addon_core
test / unit (pull_request) Successful in 32s
Details
test / unit (push) Successful in 31s
Details
test / integration (push) Successful in 38s
Details
test / integration (pull_request) Successful in 47s
Details
07c8593999 
EgressRoute now extends egress_addon_core.Route, which holds the four
wire-visible fields (host, path_allowlist, auth_scheme, token_env).
EgressRoute adds only the three host-side fields (token_ref, roles,
tls_passthrough) that are never serialised to the sidecar.

_route_to_yaml_fields is typed as Route -> dict, making the host→wire
boundary explicit: only fields declared on the base class cross into the
YAML the addon reads.
didericis approved these changes 2026-06-02 02:01:20 -04:00
didericis-claude changed title from PRD 0031: Split _merge_provider_route into named case helpers to PRD 0031: Provisioned-wins merge + EgressRoute inherits Route 2026-06-02 02:01:38 -04:00
didericis merged commit 07c8593999 into main 2026-06-02 02:02:23 -04:00
didericis deleted branch prd-0031 2026-06-02 02:02:23 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
didericis
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#121
Delete Branch "prd-0031"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?