Grandiras/ATProto.NET

Fork 0

refactor(hosting): shared base for the PDS container resources #143

Merged

Grandiras merged 2 commits from issue-116-hosting-base into main

2026-09-24 18:01:36 +00:00

Grandiras commented

2026-09-24 12:50:23 +00:00

Owner

Closes #116.

Consolidates ATProtoNet.Aspire.Hosting. This change removes duplication only. Every public With* method and all Tranquil support stay, and both servers produce exactly the same application model and manifest as before.

Shared base for the two container resources

New AtProtoPdsContainerResourceBase : ContainerResource, IResourceWithConnectionString holds what both resources used to declare separately: HttpEndpointName, HealthCheckPath, JwtSecretParameter, the internal Hostname, and ConnectionStringExpression.
AtProtoPdsContainerResource and AtProtoTranquilPdsContainerResource derive from it and keep only their server-specific members (admin password or admin account, PLC rotation key, DPoP secret, master key, database URL, development mode).
The constructor is private protected, so no one outside the package can derive from the base.

One implementation per duplicated method pair

Was (one copy per server)	Now
`WithHostname(string)` ×2, `WithHostname(IResourceBuilder<ParameterResource>)` ×2	`WithHostname<T>(…) where T : AtProtoPdsContainerResourceBase`, both overloads, on `AtProtoPdsHostingExtensions`
`WithJwtSecret` ×2	`WithJwtSecret<T>(…)`, same constraint and class
`ClearDataMounts` / `ClearBlobMounts` plus the bind/volume bodies	internal `ReplaceStorageMount<T>(builder, source, target, isBindMount)`
`WithAtProtoPds` / `WithAtProtoTranquilPds` bodies	internal `WithPdsConfiguration<T, TPds>(…)`. Each public method passes only its own admin-configuration writer.

The generic methods return IResourceBuilder<T>, so they chain straight into server-specific methods such as .WithHostname(…).WithAdminAccount(…).
WithDataBindMount/WithDataVolume and WithBlobBindMount/WithBlobVolume keep their names and signatures. They mount different directories (/pds holds all reference-PDS state, while Tranquil's mount holds only blobs), so only their implementation is shared.
The 18 environment-variable convenience methods (WithPlcUrl, WithCrawlers, WithHandleDomains, …) are unchanged, as the issue asks.

Breaking changes

WithHostname (both overloads) and WithJwtSecret are now generic methods on AtProtoPdsHostingExtensions. AtProtoTranquilPdsHostingExtensions no longer declares them.
- Source-compatible for extension-method calls. The unchanged 0.6.0 test files compiled and passed (104 tests) against the refactored library before they were restructured.
- Binary-breaking, so a recompile is needed.
- Migration: recompile. Rewrite an explicit static call such as AtProtoTranquilPdsHostingExtensions.WithHostname(pds, …) as pds.WithHostname(…).
Moving HttpEndpointName, HealthCheckPath, JwtSecretParameter and ConnectionStringExpression into the base class is not breaking. Constants are inlined at the call site, inherited members still resolve through the derived type names, and adding a base class is an allowed change.

Tests

New PdsContainerResourceTests covers the shared behaviour as theories over a PdsFlavour enum, with expected per-server values in InlineData: image and tag, port mapping and health check, connection string, argument validation, hostname (run mode, publish mode, literal, parameter), WithJwtSecret, dropping an overridden parameter from the publish model, storage mount replacement, and consumer wiring (URL, admin password, plaintext opt-in in run versus publish mode, the wait, null checks).
AspireTestHost holds the shared helpers: PublishModeBuilder, GetEnvironmentAsync, SingleResource<T>, and the flavour dispatchers. GetEnvironmentAsync used to be duplicated, and PublishModeBuilder was written out inline four times.
The per-server files keep only what is specific to one server: hex secrets, admin password and rotation key; Tranquil's database, admin account, dev-mode relaxations, DPoP secret, master key and S3.
Replaced assertion-free tests with real ones:
- The 9 reference-PDS With*_SetsEnvironmentAnnotation / ConfiguresAdminPasswordEnvironment tests only asserted that some environment callback existed, which AddAtProtoPds always adds. FluentApi_AllowsChaining now pins every variable the convenience methods set, including WithProductionMode overriding the run-mode PDS_DEV_MODE=true.
- The two ImplementsIResourceWithConnectionString tests checked something the compiler already guarantees. A theory now pins the connection-string template instead.
New assertions:
- The reference wiring sets neither AtProto__Pds__Authentication nor AdminIdentifier.
- WithDataVolume() and WithBlobVolume() without a name fall back to the default volume name after a bind mount.
Test methods in the hosting test classes went from 82 (in two classes) to 55 (in three), and test lines are down 180 net.

Verification

dotnet build of the full solution in Debug, --no-incremental and Release: 0 warnings, 0 errors.
Unit tests: 1,712 total, 1,707 passed, 0 failed, 5 skipped (the manifest tests, gated on ATPROTO_ASPIRE_MANIFEST). The Aspire folder has 103 tests (98 passed, 5 skipped).
CI manifest step reproduced: dotnet run --project samples/ManagedPdsSample.AppHost --no-build -- --publisher manifest before and after, in Debug and Release. The manifests are byte-identical, and AspireManifestTests with ATPROTO_ASPIRE_MANIFEST and ATPROTO_REQUIRE_INTEGRATION=1 gives 6/6 passed.
The sample AppHost covers only the reference PDS with defaults, so I also ran a scratch AppHost (not committed). It has 7 PDS resources (3 reference, 4 Tranquil) exercising every With* method, including parameter and literal hostnames, every secret override, bind mounts and volumes, WithDatabase/WithDatabaseUrl, the derived admin handle, and consumers with and without the wait.
- It dumped the evaluated model in run and publish mode: resources, annotation order, image, endpoints, mounts, waits, health checks, evaluated environment and parameter defaults.
- It also published a manifest.
- All three outputs are byte-identical before and after.
Mutation check: breaking the shared mount-clearing filter fails all 6 storage theory cases, across both servers.

Deferred

The optional part of WP7, removing the 18 environment-variable wrappers, is out of scope because the issue says to keep them.
XML-doc compaction in Hosting/ belongs to WP17.
The source reduction is smaller than the research estimate: −56 net lines in src/. The duplicated code is gone, but the new base class and the internal helpers carry their own XML docs, and the merged docs now cover both servers.
Two small duplications remain in the Add* methods, left alone to keep the diff focused: the PdsContainerPort/DefaultTag constants and the 4-line publish-mode hostname parameter.

🤖 Generated with Claude Code

Closes #116. Consolidates `ATProtoNet.Aspire.Hosting`. This change removes duplication only. Every public `With*` method and all Tranquil support stay, and both servers produce exactly the same application model and manifest as before. ## Shared base for the two container resources - New `AtProtoPdsContainerResourceBase : ContainerResource, IResourceWithConnectionString` holds what both resources used to declare separately: `HttpEndpointName`, `HealthCheckPath`, `JwtSecretParameter`, the internal `Hostname`, and `ConnectionStringExpression`. - `AtProtoPdsContainerResource` and `AtProtoTranquilPdsContainerResource` derive from it and keep only their server-specific members (admin password or admin account, PLC rotation key, DPoP secret, master key, database URL, development mode). - The constructor is `private protected`, so no one outside the package can derive from the base. ## One implementation per duplicated method pair | Was (one copy per server) | Now | |---|---| | `WithHostname(string)` ×2, `WithHostname(IResourceBuilder<ParameterResource>)` ×2 | `WithHostname<T>(…) where T : AtProtoPdsContainerResourceBase`, both overloads, on `AtProtoPdsHostingExtensions` | | `WithJwtSecret` ×2 | `WithJwtSecret<T>(…)`, same constraint and class | | `ClearDataMounts` / `ClearBlobMounts` plus the bind/volume bodies | internal `ReplaceStorageMount<T>(builder, source, target, isBindMount)` | | `WithAtProtoPds` / `WithAtProtoTranquilPds` bodies | internal `WithPdsConfiguration<T, TPds>(…)`. Each public method passes only its own admin-configuration writer. | - The generic methods return `IResourceBuilder<T>`, so they chain straight into server-specific methods such as `.WithHostname(…).WithAdminAccount(…)`. - `WithDataBindMount`/`WithDataVolume` and `WithBlobBindMount`/`WithBlobVolume` keep their names and signatures. They mount different directories (`/pds` holds all reference-PDS state, while Tranquil's mount holds only blobs), so only their implementation is shared. - The 18 environment-variable convenience methods (`WithPlcUrl`, `WithCrawlers`, `WithHandleDomains`, …) are unchanged, as the issue asks. ## Breaking changes - `WithHostname` (both overloads) and `WithJwtSecret` are now generic methods on `AtProtoPdsHostingExtensions`. `AtProtoTranquilPdsHostingExtensions` no longer declares them. - Source-compatible for extension-method calls. The unchanged 0.6.0 test files compiled and passed (104 tests) against the refactored library before they were restructured. - Binary-breaking, so a recompile is needed. - **Migration:** recompile. Rewrite an explicit static call such as `AtProtoTranquilPdsHostingExtensions.WithHostname(pds, …)` as `pds.WithHostname(…)`. - Moving `HttpEndpointName`, `HealthCheckPath`, `JwtSecretParameter` and `ConnectionStringExpression` into the base class is not breaking. Constants are inlined at the call site, inherited members still resolve through the derived type names, and adding a base class is an allowed change. ## Tests - New `PdsContainerResourceTests` covers the shared behaviour as theories over a `PdsFlavour` enum, with expected per-server values in `InlineData`: image and tag, port mapping and health check, connection string, argument validation, hostname (run mode, publish mode, literal, parameter), `WithJwtSecret`, dropping an overridden parameter from the publish model, storage mount replacement, and consumer wiring (URL, admin password, plaintext opt-in in run versus publish mode, the wait, null checks). - `AspireTestHost` holds the shared helpers: `PublishModeBuilder`, `GetEnvironmentAsync`, `SingleResource<T>`, and the flavour dispatchers. `GetEnvironmentAsync` used to be duplicated, and `PublishModeBuilder` was written out inline four times. - The per-server files keep only what is specific to one server: hex secrets, admin password and rotation key; Tranquil's database, admin account, dev-mode relaxations, DPoP secret, master key and S3. - Replaced assertion-free tests with real ones: - The 9 reference-PDS `With*_SetsEnvironmentAnnotation` / `ConfiguresAdminPasswordEnvironment` tests only asserted that some environment callback existed, which `AddAtProtoPds` always adds. `FluentApi_AllowsChaining` now pins every variable the convenience methods set, including `WithProductionMode` overriding the run-mode `PDS_DEV_MODE=true`. - The two `ImplementsIResourceWithConnectionString` tests checked something the compiler already guarantees. A theory now pins the connection-string template instead. - New assertions: - The reference wiring sets neither `AtProto__Pds__Authentication` nor `AdminIdentifier`. - `WithDataVolume()` and `WithBlobVolume()` without a name fall back to the default volume name after a bind mount. - Test methods in the hosting test classes went from 82 (in two classes) to 55 (in three), and test lines are down 180 net. ## Verification - `dotnet build` of the full solution in Debug, `--no-incremental` and Release: **0 warnings, 0 errors**. - Unit tests: **1,712 total, 1,707 passed, 0 failed, 5 skipped** (the manifest tests, gated on `ATPROTO_ASPIRE_MANIFEST`). The Aspire folder has 103 tests (98 passed, 5 skipped). - CI manifest step reproduced: `dotnet run --project samples/ManagedPdsSample.AppHost --no-build -- --publisher manifest` before and after, in Debug and Release. The manifests are **byte-identical**, and `AspireManifestTests` with `ATPROTO_ASPIRE_MANIFEST` and `ATPROTO_REQUIRE_INTEGRATION=1` gives 6/6 passed. - The sample AppHost covers only the reference PDS with defaults, so I also ran a scratch AppHost (not committed). It has 7 PDS resources (3 reference, 4 Tranquil) exercising every `With*` method, including parameter and literal hostnames, every secret override, bind mounts and volumes, `WithDatabase`/`WithDatabaseUrl`, the derived admin handle, and consumers with and without the wait. - It dumped the evaluated model in run and publish mode: resources, annotation order, image, endpoints, mounts, waits, health checks, evaluated environment and parameter defaults. - It also published a manifest. - All three outputs are **byte-identical** before and after. - Mutation check: breaking the shared mount-clearing filter fails all 6 storage theory cases, across both servers. ## Deferred - The optional part of WP7, removing the 18 environment-variable wrappers, is out of scope because the issue says to keep them. - XML-doc compaction in `Hosting/` belongs to WP17. - The source reduction is smaller than the research estimate: −56 net lines in `src/`. The duplicated code is gone, but the new base class and the internal helpers carry their own XML docs, and the merged docs now cover both servers. - Two small duplications remain in the `Add*` methods, left alone to keep the diff focused: the `PdsContainerPort`/`DefaultTag` constants and the 4-line publish-mode hostname parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-24 12:50:23 +00:00

refactor(hosting): shared base for the PDS container resources (closes #116 )

CI / pds-integration (pull_request) Successful in 23s

Details

CI / build-and-test (pull_request) Successful in 58s

Details

173832a8d2

AtProtoPdsContainerResource and AtProtoTranquilPdsContainerResource each
declared the same endpoint name, health path, JWT secret, hostname and
connection string, and the two extension classes carried parallel copies of
WithHostname (x2), WithJwtSecret, the clear-mounts helper and the project
wiring. Both resources now derive from AtProtoPdsContainerResourceBase;
WithHostname and WithJwtSecret are single methods generic over it, and the
wiring and mount replacement each have one internal implementation. Every
With* method and all Tranquil support stay.

The generic methods are source-compatible but binary-breaking. The run- and
publish-mode application model and the published manifests of both servers
are byte-identical before and after.

The parallel Pds/Tranquil tests become theories over the flavour in
PdsContainerResourceTests, with the builder and environment helpers shared.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

opencode-bot commented

2026-09-24 12:53:19 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T12-53-19-570Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer reviewed

2026-09-24 12:56:17 +00:00

opencode-reviewer left a comment

💬 Verdict: comment — the reviewer did not submit a structured review; the raw output follows.

(empty)

⏱ 2m 58s · 8 steps · $0.0342 · tokens: 147.7k in / 6.7k out (+289.4k cache-read) · Go: 5h 17% · wk 93% · mo 79%
📜 Full session transcript

💬 **Verdict: comment** — the reviewer did not submit a structured review; the raw output follows. (empty) --- ⏱ 2m 58s · 8 steps · $0.0342 · tokens: 147.7k in / 6.7k out (+289.4k cache-read) · Go: 5h 17% · wk 93% · mo 79% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T12-53-19-570Z.md)

Grandiras commented

2026-09-24 12:57:29 +00:00

Author

Owner

@opencode-reviewer

opencode-bot commented

2026-09-24 12:57:31 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T12-57-31-262Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer reviewed

2026-09-24 13:27:33 +00:00

opencode-reviewer left a comment

💬 Verdict: comment — the reviewer did not submit a structured review; the raw output follows.

Let me check the recent git history to understand what changed since my last (empty) review pass:

⏱ 30m 2s · 16 steps · $0.1115 · tokens: 365.7k in / 21.5k out (+866.3k cache-read) · Go: 5h 22% · wk 95% · mo 80%
📜 Full session transcript

💬 **Verdict: comment** — the reviewer did not submit a structured review; the raw output follows. Let me check the recent git history to understand what changed since my last (empty) review pass: --- ⏱ 30m 2s · 16 steps · $0.1115 · tokens: 365.7k in / 21.5k out (+866.3k cache-read) · Go: 5h 22% · wk 95% · mo 80% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T12-57-31-262Z.md)

Grandiras commented

2026-09-24 13:28:06 +00:00

Author

Owner

@claude-reviewer

claude-bot commented

2026-09-24 13:28:10 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/574); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-574.md) — updating every ~5 minutes while the task runs.

Grandiras commented

2026-09-24 17:52:16 +00:00

Author

Owner

@claude-reviewer

claude-bot commented

2026-09-24 17:52:18 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/575); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-575.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-24 17:53:44 +00:00

Dismissed

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Solid, mechanical refactor. The new AtProtoPdsContainerResourceBase cleanly hoists the shared members (HttpEndpointName, HealthCheckPath, JwtSecretParameter, Hostname, ConnectionStringExpression), and the private protected constructor correctly seals it to the assembly while staying public for API discoverability. The generic WithHostname<T>/WithJwtSecret<T>/ReplaceStorageMount<T>/WithPdsConfiguration<T,TPds> methods preserve extension-method resolution (T is inferred from the concrete resource type at call sites), and tracing through the before/after bodies for WithAtProtoPds/WithAtProtoTranquilPds and the mount-clearing helpers shows behavior is preserved exactly (same env keys, same admin wiring, same mount-replacement semantics).

The binary-compatibility reasoning in the PR description also checks out: moving const fields is compile-time-inlined at call sites so it can't break already-compiled consumers, and moving a non-virtual instance member (ConnectionStringExpression) to a base class is resolved by the CLR through the inheritance chain, so existing callers still bind correctly.

Test coverage is thorough — the new PdsContainerResourceTests theories exercise both flavours through the shared surface, and the per-server test files were correctly pruned to keep only server-specific assertions. Nothing incorrect found; this looks safe to merge.

⏱ 1m 10s · 1 turns · tokens: 58.0k in / 6.8k out (+18.6k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid, mechanical refactor. The new `AtProtoPdsContainerResourceBase` cleanly hoists the shared members (`HttpEndpointName`, `HealthCheckPath`, `JwtSecretParameter`, `Hostname`, `ConnectionStringExpression`), and the `private protected` constructor correctly seals it to the assembly while staying `public` for API discoverability. The generic `WithHostname<T>`/`WithJwtSecret<T>`/`ReplaceStorageMount<T>`/`WithPdsConfiguration<T,TPds>` methods preserve extension-method resolution (T is inferred from the concrete resource type at call sites), and tracing through the before/after bodies for `WithAtProtoPds`/`WithAtProtoTranquilPds` and the mount-clearing helpers shows behavior is preserved exactly (same env keys, same admin wiring, same mount-replacement semantics). The binary-compatibility reasoning in the PR description also checks out: moving `const` fields is compile-time-inlined at call sites so it can't break already-compiled consumers, and moving a non-virtual instance member (`ConnectionStringExpression`) to a base class is resolved by the CLR through the inheritance chain, so existing callers still bind correctly. Test coverage is thorough — the new `PdsContainerResourceTests` theories exercise both flavours through the shared surface, and the per-server test files were correctly pruned to keep only server-specific assertions. Nothing incorrect found; this looks safe to merge. --- ⏱ 1m 10s · 1 turns · tokens: 58.0k in / 6.8k out (+18.6k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-575.md)

opencode-bot commented

2026-09-24 17:54:29 +00:00

Collaborator

🤖 Resolving the merge conflicts — the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the fix runs.

🤖 Resolving the merge conflicts — the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T17-54-28-042Z.md) — updating every ~5 minutes while the fix runs.

Grandiras commented

2026-09-24 17:55:08 +00:00

Author

Owner

@claude /review — opencode-bot is paused at its weekly Go cap (its two runs on this PR produced no structured review), so requesting a claude-bot review instead.

claude-bot commented

2026-09-24 17:55:12 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/576); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-576.md) — updating every ~5 minutes while the task runs.

Grandiras referenced this pull request

2026-09-24 17:56:38 +00:00

chore: merge CHANGELOG.md with the union driver #144

claude-reviewer approved these changes

2026-09-24 17:57:08 +00:00

Dismissed

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This PR is unchanged in substance from the version I approved in my previous pass — same shared base class, same generic WithHostname<T>/WithJwtSecret<T>/ReplaceStorageMount<T>/WithPdsConfiguration<T,TPds> extraction, same test consolidation into PdsContainerResourceTests with AspireTestHost helpers.

I re-traced the mechanics that matter most for a refactor like this:

AtProtoPdsContainerResourceBase's private protected constructor correctly seals construction to the assembly while the class itself stays public abstract for discoverability; both derived sealed classes call it with (name, jwtSecret) matching the base signature.
PdsParameterOverrides.Replace<TResource> is already generic over IResource, so the new generic WithHostname<T>/WithJwtSecret<T> bind cleanly against it — no surprises there.
IResourceBuilder<out T> in Aspire.Hosting is covariant (confirmed via the Aspire source/docs), so the test helpers' AddPds/WithPds/ReplaceStorage pattern-matching against IResourceBuilder<AtProtoPdsContainerResourceBase> and switching on the concrete IResourceBuilder<AtProtoPdsContainerResource>/IResourceBuilder<AtProtoTranquilPdsContainerResource> types compiles and behaves as intended.
The binary-compatibility claim (moving non-virtual instance members like JwtSecretParameter and ConnectionStringExpression into a new base class is not breaking) matches documented .NET assembly-versioning behavior: member resolution against a MemberRef token walks the full inheritance chain, not just directly-declared members, so already-compiled callers resolve correctly against the refactored library.
WithPdsConfiguration<T, TPds> preserves the exact env-var wiring and ordering both WithAtProtoPds and WithAtProtoTranquilPds had before (URL, admin config callback, run-mode plaintext opt-in, wait), and ReplaceStorageMount<T> preserves the mount-clearing-then-add semantics for both the reference PDS's /pds and Tranquil's blob directory.

No correctness issues found. Good to merge.

⏱ 1m 43s · 7 turns · tokens: 76.6k in / 10.4k out (+555.4k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This PR is unchanged in substance from the version I approved in my previous pass — same shared base class, same generic `WithHostname<T>`/`WithJwtSecret<T>`/`ReplaceStorageMount<T>`/`WithPdsConfiguration<T,TPds>` extraction, same test consolidation into `PdsContainerResourceTests` with `AspireTestHost` helpers. I re-traced the mechanics that matter most for a refactor like this: - `AtProtoPdsContainerResourceBase`'s `private protected` constructor correctly seals construction to the assembly while the class itself stays `public abstract` for discoverability; both derived sealed classes call it with `(name, jwtSecret)` matching the base signature. - `PdsParameterOverrides.Replace<TResource>` is already generic over `IResource`, so the new generic `WithHostname<T>`/`WithJwtSecret<T>` bind cleanly against it — no surprises there. - `IResourceBuilder<out T>` in Aspire.Hosting is covariant (confirmed via the Aspire source/docs), so the test helpers' `AddPds`/`WithPds`/`ReplaceStorage` pattern-matching against `IResourceBuilder<AtProtoPdsContainerResourceBase>` and switching on the concrete `IResourceBuilder<AtProtoPdsContainerResource>`/`IResourceBuilder<AtProtoTranquilPdsContainerResource>` types compiles and behaves as intended. - The binary-compatibility claim (moving non-virtual instance members like `JwtSecretParameter` and `ConnectionStringExpression` into a new base class is not breaking) matches documented .NET assembly-versioning behavior: member resolution against a MemberRef token walks the full inheritance chain, not just directly-declared members, so already-compiled callers resolve correctly against the refactored library. - `WithPdsConfiguration<T, TPds>` preserves the exact env-var wiring and ordering both `WithAtProtoPds` and `WithAtProtoTranquilPds` had before (URL, admin config callback, run-mode plaintext opt-in, wait), and `ReplaceStorageMount<T>` preserves the mount-clearing-then-add semantics for both the reference PDS's `/pds` and Tranquil's blob directory. No correctness issues found. Good to merge. --- ⏱ 1m 43s · 7 turns · tokens: 76.6k in / 10.4k out (+555.4k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-576.md)

opencode-bot added 1 commit

2026-09-24 17:59:06 +00:00

Merge remote-tracking branch 'origin/main' into issue-116-hosting-base

CI / pds-integration (pull_request) Successful in 27s

Details

CI / build-and-test (pull_request) Successful in 1m0s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

e479625dfe

# Conflicts:
#	CHANGELOG.md

opencode-bot commented

2026-09-24 17:59:06 +00:00

Collaborator

Merge resolved and committed (e479625), working tree clean, no conflict markers anywhere.

What conflicted: CHANGELOG.md only — the only file both sides touched. Both this PR (#116) and main's d0c4e6b (#142/#110) had inserted entries directly under [Unreleased]'s new ### Breaking changes / ### Changed headers, and main additionally added ### Fixed and ### Removed sections; the Security section below merged cleanly.

How I resolved it: Kept every bullet from both sides. Within the two shared subsections I ordered main's #110 bullets above the PR's #116 bullets, following the repo's established convention of prepending the newest entry at the top of a section (verified against how #102, #99, and #93 entries were layered in past merges). Main's Fixed and Removed sections and the PR's docs/code were untouched.

Verified:

Diffed the merge result against both parents: relative to origin/main only the PR's 11 files (+2 changelog bullets) remain; relative to the PR branch only main's 168-file metadata cleanup remains — neither side lost anything.
dotnet build of ATProtoNet.Tests (which pulls in ATProtoNet.Aspire.Hosting, the project this PR refactors): 0 errors, 0 warnings after main's Directory.Build.props consolidation.
Aspire test suite: 98 passed, 0 failed, 5 skipped — the skips are the pre-existing [RequiresAspireManifestFact] environment gate, unchanged by the merge.

→ Merge resolved and pushed; a re-review follows.

⏱ 4m 39s · 22 steps · $0.0477 · tokens: 144.6k in / 9.4k out (+648.1k cache-read) · Go: 5h 0% · wk 95% · mo 80%

📜 Full session transcript

Merge resolved and committed (`e479625`), working tree clean, no conflict markers anywhere. **What conflicted:** `CHANGELOG.md` only — the only file both sides touched. Both this PR (#116) and main's d0c4e6b (#142/#110) had inserted entries directly under `[Unreleased]`'s new `### Breaking changes` / `### Changed` headers, and main additionally added `### Fixed` and `### Removed` sections; the `Security` section below merged cleanly. **How I resolved it:** Kept every bullet from both sides. Within the two shared subsections I ordered main's #110 bullets above the PR's #116 bullets, following the repo's established convention of prepending the newest entry at the top of a section (verified against how #102, #99, and #93 entries were layered in past merges). Main's `Fixed` and `Removed` sections and the PR's docs/code were untouched. **Verified:** - Diffed the merge result against both parents: relative to `origin/main` only the PR's 11 files (+2 changelog bullets) remain; relative to the PR branch only main's 168-file metadata cleanup remains — neither side lost anything. - `dotnet build` of `ATProtoNet.Tests` (which pulls in `ATProtoNet.Aspire.Hosting`, the project this PR refactors): 0 errors, 0 warnings after main's `Directory.Build.props` consolidation. - Aspire test suite: 98 passed, 0 failed, 5 skipped — the skips are the pre-existing `[RequiresAspireManifestFact]` environment gate, unchanged by the merge. → Merge resolved and pushed; a re-review follows. --- ⏱ 4m 39s · 22 steps · $0.0477 · tokens: 144.6k in / 9.4k out (+648.1k cache-read) · Go: 5h 0% · wk 95% · mo 80% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T17-54-28-042Z.md)

opencode-bot commented

2026-09-24 18:00:07 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T18-00-07-150Z.md) — updating every ~5 minutes while the review runs.

Grandiras referenced this pull request

2026-09-24 18:00:11 +00:00

revert: drop the union merge driver for CHANGELOG.md #145

Grandiras commented

2026-09-24 18:00:19 +00:00

Author

Owner

@claude /review — the head moved to opencode-bot's conflict-resolution merge (e479625, CHANGELOG only); re-review for the current head.

@claude /review — the head moved to opencode-bot's conflict-resolution merge (e479625, CHANGELOG only); re-review for the current head.

claude-bot commented

2026-09-24 18:00:24 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/579); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-579.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-24 18:01:01 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This diff is materially identical to the version I approved in my two previous passes — same AtProtoPdsContainerResourceBase extraction, same generic WithHostname<T>/WithJwtSecret<T>/ReplaceStorageMount<T>/WithPdsConfiguration<T,TPds> methods, same PdsContainerResourceTests/AspireTestHost test consolidation. I re-traced the same mechanics again:

The private protected base constructor correctly restricts construction to the assembly while keeping the base public abstract for discoverability; both sealed derived classes pass (name, jwtSecret) through correctly.
WithPdsConfiguration<T, TPds> preserves the exact env-var wiring and ordering both WithAtProtoPds and WithAtProtoTranquilPds had before (WithReference, PDS URL env, admin-config callback, run-mode plaintext opt-in, then the wait) — the writeAdminConfiguration delegate parameter for the reference PDS writes only AdminPasswordConfigurationKey, and for Tranquil writes AuthenticationConfigurationKey/AdminIdentifierConfigurationKey/AdminPasswordConfigurationKey, matching the pre-refactor bodies.
ReplaceStorageMount<T> preserves the mount-clear-then-add semantics for both the reference PDS's /pds and Tranquil's blob directory, and both WithDataBindMount/WithDataVolume/WithBlobBindMount/WithBlobVolume call sites pass the right isBindMount flag.
The moved const fields (HttpEndpointName, HealthCheckPath) and the non-virtual ConnectionStringExpression/JwtSecretParameter members resolve correctly for already-compiled callers per standard CLR member-resolution-through-inheritance-chain behavior, matching the PR's binary-compatibility claim.
Test coverage (PdsContainerResourceTests theories over PdsFlavour) exercises the shared surface for both servers, and the per-server test files were pruned to keep only server-specific assertions without losing coverage (e.g., WithPds_ThrowsOnNullPds now checks both WithAtProtoPds and WithAtProtoTranquilPds).

No correctness issues found. Good to merge.

⏱ 0m 23s · 1 turns · tokens: 54.5k in / 2.1k out (+23.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This diff is materially identical to the version I approved in my two previous passes — same `AtProtoPdsContainerResourceBase` extraction, same generic `WithHostname<T>`/`WithJwtSecret<T>`/`ReplaceStorageMount<T>`/`WithPdsConfiguration<T,TPds>` methods, same `PdsContainerResourceTests`/`AspireTestHost` test consolidation. I re-traced the same mechanics again: - The `private protected` base constructor correctly restricts construction to the assembly while keeping the base `public abstract` for discoverability; both sealed derived classes pass `(name, jwtSecret)` through correctly. - `WithPdsConfiguration<T, TPds>` preserves the exact env-var wiring and ordering both `WithAtProtoPds` and `WithAtProtoTranquilPds` had before (`WithReference`, PDS URL env, admin-config callback, run-mode plaintext opt-in, then the wait) — the `writeAdminConfiguration` delegate parameter for the reference PDS writes only `AdminPasswordConfigurationKey`, and for Tranquil writes `AuthenticationConfigurationKey`/`AdminIdentifierConfigurationKey`/`AdminPasswordConfigurationKey`, matching the pre-refactor bodies. - `ReplaceStorageMount<T>` preserves the mount-clear-then-add semantics for both the reference PDS's `/pds` and Tranquil's blob directory, and both `WithDataBindMount`/`WithDataVolume`/`WithBlobBindMount`/`WithBlobVolume` call sites pass the right `isBindMount` flag. - The moved `const` fields (`HttpEndpointName`, `HealthCheckPath`) and the non-virtual `ConnectionStringExpression`/`JwtSecretParameter` members resolve correctly for already-compiled callers per standard CLR member-resolution-through-inheritance-chain behavior, matching the PR's binary-compatibility claim. - Test coverage (`PdsContainerResourceTests` theories over `PdsFlavour`) exercises the shared surface for both servers, and the per-server test files were pruned to keep only server-specific assertions without losing coverage (e.g., `WithPds_ThrowsOnNullPds` now checks both `WithAtProtoPds` and `WithAtProtoTranquilPds`). No correctness issues found. Good to merge. --- ⏱ 0m 23s · 1 turns · tokens: 54.5k in / 2.1k out (+23.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-579.md)

Grandiras merged commit 0febfcf278 into main

2026-09-24 18:01:36 +00:00

Grandiras deleted branch issue-116-hosting-base

2026-09-24 18:01:36 +00:00

Grandiras referenced this pull request from a commit

2026-09-24 18:01:38 +00:00

refactor(hosting): shared base for the PDS container resources (#143)

opencode-reviewer approved these changes

2026-09-24 18:02:25 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Verified the consolidation against the previous state: the shared base (AtProtoPdsContainerResourceBase) hoists exactly the duplicated members, both sealed resources derive from it with the private protected constructor correctly limiting derivation to the package, and the extracted helpers (WithPdsConfiguration, ReplaceStorageMount, generic WithHostname/WithJwtSecret) reproduce the old bodies step for step — same environment keys and ordering, same admin wiring, same mount-replacement semantics and default volume names. Inherited consts still resolve through the derived type names used in the Add* methods, so application-model output is unchanged. Extension-method calls keep resolving through builder covariance, and the documented binary break (recompile; rewrite explicit static calls) is legitimate and called out in the CHANGELOG. Samples contain no calls to the removed non-generic overloads. Test restructuring replaces assertion-free tests with real assertions rather than dropping coverage. No blocking issues found; the two remaining small duplications noted in the PR description are consciously deferred and not blockers. Good to merge.

⏱ 2m 18s · 9 steps · $0.0296 · tokens: 109.5k in / 5.6k out (+299.0k cache-read) · Go: 5h 0% · wk 95% · mo 80%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Verified the consolidation against the previous state: the shared base (`AtProtoPdsContainerResourceBase`) hoists exactly the duplicated members, both sealed resources derive from it with the `private protected` constructor correctly limiting derivation to the package, and the extracted helpers (`WithPdsConfiguration`, `ReplaceStorageMount`, generic `WithHostname`/`WithJwtSecret`) reproduce the old bodies step for step — same environment keys and ordering, same admin wiring, same mount-replacement semantics and default volume names. Inherited consts still resolve through the derived type names used in the `Add*` methods, so application-model output is unchanged. Extension-method calls keep resolving through builder covariance, and the documented binary break (recompile; rewrite explicit static calls) is legitimate and called out in the CHANGELOG. Samples contain no calls to the removed non-generic overloads. Test restructuring replaces assertion-free tests with real assertions rather than dropping coverage. No blocking issues found; the two remaining small duplications noted in the PR description are consciously deferred and not blockers. Good to merge. --- ⏱ 2m 18s · 9 steps · $0.0296 · tokens: 109.5k in / 5.6k out (+299.0k cache-read) · Go: 5h 0% · wk 95% · mo 80% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-143/run-2026-09-24T18-00-07-150Z.md)

Grandiras referenced this pull request

2026-09-24 18:14:11 +00:00

fix(identity)!: spec-conformant identifier types and a monotonic TID generator #146