Claude: changes for issue #78 #79

Merged
Grandiras merged 2 commits from claude/issue-78 into main 2026-07-26 01:03:14 +00:00
Collaborator

Closes #78.

Closes #78.
feat: support Tranquil PDS for hosting and administration
All checks were successful
CI / pds-integration (pull_request) Successful in 19s
CI / build-and-test (pull_request) Successful in 50s
7f9d6515aa
Tranquil is a community AT Protocol PDS — a single Rust binary rather than
the reference server's Node.js runtime, and a superset of it. It can now be
hosted from an Aspire AppHost and administered with PdsAdminClient, through
the same APIs the reference PDS uses.

AddAtProtoTranquilPds adds the atcr.io/tranquil.farm/tranquil-pds container.
Tranquil keeps its repositories in PostgreSQL, so the call also provisions a
server and database and waits on them; WithDatabase / WithDatabaseUrl point
it at one you already have and drop the generated resources from the model.
It is handed a postgres:// URI, not the ADO.NET connection string Aspire's
PostgresDatabaseResource exposes, which would not parse.

The difference an application has to care about is authentication. The
reference PDS has one server-wide admin password used with HTTP Basic;
Tranquil has none, and flags individual accounts as administrators
authenticated with an ordinary session. PdsAdminOptions.Authentication
selects the scheme and AdminIdentifier names the account. Under AdminAccount
the client signs in lazily — the account may not exist when the client is
resolved — reuses the session, and re-authenticates once if the server
rejects it.

The administrator account is deliberately not created by the AppHost:
Tranquil flags the first account on an empty instance as an administrator,
so the application registers it with CreateAccountAsync, which works
unauthenticated. Its handle is derived from the hostname rather than fixed,
so WithHostname alone leaves a handle the server will issue, and defaults to
pdsadmin rather than admin, which Tranquil rejects as reserved.

Running locally the container also gets the relaxations a development
instance needs, and none of them when publishing. Two are load-bearing: an
empty Tranquil mints a bootstrap invite code only to its log, and login is
blocked until an account has a verified communication channel. At Tranquil's
own defaults no account could be created locally and none could sign in.

closes #78
Author
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/96); 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-79/run-96.md) — updating every ~5 minutes while the task runs.
claude-reviewer requested changes 2026-07-26 00:34:21 +00:00
Dismissed
claude-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

Big, well-documented PR (Tranquil PDS Aspire hosting + PdsAdminClient account-auth support). The PdsAdminClient session/retry logic, the Replace/parameter-override plumbing, and the development-mode env-var layering all look internally consistent and match their test coverage. One likely compile-breaking issue stood out (see finding) — everything else is solid.


⏱ 3m 13s · 20 turns · tokens: 89.3k in / 15.9k out (+1889.2k cache-read)
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. Big, well-documented PR (Tranquil PDS Aspire hosting + `PdsAdminClient` account-auth support). The `PdsAdminClient` session/retry logic, the `Replace`/parameter-override plumbing, and the development-mode env-var layering all look internally consistent and match their test coverage. One likely compile-breaking issue stood out (see finding) — everything else is solid. --- ⏱ 3m 13s · 20 turns · tokens: 89.3k in / 15.9k out (+1889.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-79/run-96.md) <!-- claude-verdict:request_changes --> <!-- claude-reviewed-head:7f9d6515aa2e4d2a6ca1a0126f9b87fdaca1e7db -->
@ -0,0 +139,4 @@
if (resource.DevelopmentMode)
{
ApplyDevelopmentDefaults(context.EnvironmentVariables);
}
Collaborator

database.Resource.UriExpression (also used again in WithDatabase, line ~490) assumes Aspire.Hosting.PostgreSQL's PostgresDatabaseResource exposes a postgres://-style UriExpression member. As far as I can tell that type only exposes ConnectionStringExpression, which produces the ADO.NET key/value format (Host=...;Port=...;...) — exactly the format this PR's own CHANGELOG/docs say PostgresDatabaseResource.ConnectionStringExpression produces and which "would not parse at all" for Tranquil's DATABASE_URL. Nowhere in this file is a postgres:// URI actually assembled from the database's host/port/username/password/database — contrast with AtProtoTranquilPdsContainerResource.ConnectionStringExpression a few files over, which manually builds its URL via ReferenceExpression.Create($"http://{...}:{...}") because no library helper does it. If UriExpression doesn't exist, AddAtProtoTranquilPds and WithDatabase won't compile; if it does exist in the pinned Aspire version, it's still worth double-checking it actually yields postgres:// rather than Host=... — the test only asserts StartsWith("postgresql://"), which would need this member to already behave as claimed with zero code here to make it so.

`database.Resource.UriExpression` (also used again in `WithDatabase`, line ~490) assumes `Aspire.Hosting.PostgreSQL`'s `PostgresDatabaseResource` exposes a `postgres://`-style `UriExpression` member. As far as I can tell that type only exposes `ConnectionStringExpression`, which produces the ADO.NET key/value format (`Host=...;Port=...;...`) — exactly the format this PR's own CHANGELOG/docs say `PostgresDatabaseResource.ConnectionStringExpression` produces and which "would not parse at all" for Tranquil's `DATABASE_URL`. Nowhere in this file is a `postgres://` URI actually assembled from the database's host/port/username/password/database — contrast with `AtProtoTranquilPdsContainerResource.ConnectionStringExpression` a few files over, which manually builds its URL via `ReferenceExpression.Create($"http://{...}:{...}")` because no library helper does it. If `UriExpression` doesn't exist, `AddAtProtoTranquilPds` and `WithDatabase` won't compile; if it does exist in the pinned Aspire version, it's still worth double-checking it actually yields `postgres://` rather than `Host=...` — the test only asserts `StartsWith("postgresql://")`, which would need this member to already behave as claimed with zero code here to make it so.
Owner

@claude-bot Please work on the feedback.

@claude-bot Please work on the feedback.
Author
Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

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

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/97); 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/issue-79/run-97.md) — updating every ~5 minutes while the task runs.
test: pin the DATABASE_URL expression Tranquil is handed
All checks were successful
CI / pds-integration (pull_request) Successful in 21s
CI / build-and-test (pull_request) Successful in 50s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s
69ecf9db97
Review flagged PostgresDatabaseResource.UriExpression as possibly
nonexistent. It exists in Aspire 13.4.6 and resolves to
postgresql://{user}:{password}@{host}:{port}/{database}; the ADO.NET
form is on ConnectionStringExpression. The tests only checked the
expression's prefix and suffix, which left that distinction unpinned,
so assert the whole template instead and name the alternative at both
call sites.
Author
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/98); 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-79/run-98.md) — updating every ~5 minutes while the task runs.
Author
Collaborator

Pushed 69ecf9d to claude/issue-78.

On the review finding

The blocking finding doesn't hold — I verified it directly rather than reasoning about it. PostgresDatabaseResource.UriExpression does exist in the pinned Aspire.Hosting.PostgreSQL 13.4.6, and it does produce a URI, not the ADO.NET form. I resolved both members side by side in a throwaway test (since deleted):

UriExpression              => postgresql://{u.value}:{p.value}@{pg.bindings.tcp.host}:{pg.bindings.tcp.port}/tranquil_pds
ConnectionStringExpression => {pg.connectionString};Database=tranquil_pds

So the type exposes both shapes. ConnectionStringExpression is the Host=... form the CHANGELOG says would not parse; UriExpression is the postgresql:// one, which is why nothing in this file hand-assembles a URI — unlike AtProtoTranquilPdsContainerResource, here the library helper genuinely exists. AddAtProtoTranquilPds and WithDatabase compile and always did.

What I changed anyway

The review's second half was a fair hit: the tests asserted only StartsWith("postgresql://") / EndsWith("/tranquil_pds"), which is weak enough that a swap to ConnectionStringExpression — or credentials landing in the wrong position — could slip through. That the assertions passed didn't prove much, which is exactly what made the code hard to review. So:

  • AddAtProtoTranquilPds_PassesAPostgresUriAsDatabaseUrl now pins the entire expression template, including user, password, host, and port placement.
  • WithDatabase_UsesTheGivenDatabaseAndDropsTheGeneratedOne now asserts the scheme and the referenced server's host/port binding, not just the /pds suffix.
  • Both call sites carry a comment naming ConnectionStringExpression as the wrong sibling, so the next reader doesn't have to reconstruct the distinction.

No runtime behavior changed, so no new CHANGELOG bullet — the feature's existing [Unreleased] entry already covers it, and per CLAUDE.md comment/test-only changes are the stated exception.

Verification

I installed the .NET 10 SDK in the container (it wasn't present) and ran:

  • dotnet build -c Release across the full solution — succeeded, 0 warnings, 0 errors (CS1591-as-error is on for this project).
  • dotnet test tests/ATProtoNet.Tests/1162 passed, 0 failed, 5 skipped.

The 5 skips are the pre-existing AspireManifestTests, gated on a published manifest being available; unrelated to this change.


⏱ 24m 27s · 28 turns · tokens: 30.6k in / 10.5k out (+1025.7k cache-read)
📜 Full session transcript

Pushed `69ecf9d` to **`claude/issue-78`**. ## On the review finding The blocking finding doesn't hold — I verified it directly rather than reasoning about it. `PostgresDatabaseResource.UriExpression` **does** exist in the pinned `Aspire.Hosting.PostgreSQL` 13.4.6, and it does produce a URI, not the ADO.NET form. I resolved both members side by side in a throwaway test (since deleted): ``` UriExpression => postgresql://{u.value}:{p.value}@{pg.bindings.tcp.host}:{pg.bindings.tcp.port}/tranquil_pds ConnectionStringExpression => {pg.connectionString};Database=tranquil_pds ``` So the type exposes *both* shapes. `ConnectionStringExpression` is the `Host=...` form the CHANGELOG says would not parse; `UriExpression` is the `postgresql://` one, which is why nothing in this file hand-assembles a URI — unlike `AtProtoTranquilPdsContainerResource`, here the library helper genuinely exists. `AddAtProtoTranquilPds` and `WithDatabase` compile and always did. ## What I changed anyway The review's second half was a fair hit: the tests asserted only `StartsWith("postgresql://")` / `EndsWith("/tranquil_pds")`, which is weak enough that a swap to `ConnectionStringExpression` — or credentials landing in the wrong position — could slip through. That the assertions passed didn't prove much, which is exactly what made the code hard to review. So: - `AddAtProtoTranquilPds_PassesAPostgresUriAsDatabaseUrl` now pins the **entire** expression template, including user, password, host, and port placement. - `WithDatabase_UsesTheGivenDatabaseAndDropsTheGeneratedOne` now asserts the scheme and the referenced server's host/port binding, not just the `/pds` suffix. - Both call sites carry a comment naming `ConnectionStringExpression` as the wrong sibling, so the next reader doesn't have to reconstruct the distinction. No runtime behavior changed, so no new `CHANGELOG` bullet — the feature's existing `[Unreleased]` entry already covers it, and per `CLAUDE.md` comment/test-only changes are the stated exception. ## Verification I installed the .NET 10 SDK in the container (it wasn't present) and ran: - `dotnet build -c Release` across the full solution — **succeeded, 0 warnings, 0 errors** (`CS1591`-as-error is on for this project). - `dotnet test tests/ATProtoNet.Tests/` — **1162 passed, 0 failed, 5 skipped**. The 5 skips are the pre-existing `AspireManifestTests`, gated on a published manifest being available; unrelated to this change. --- ⏱ 24m 27s · 28 turns · tokens: 30.6k in / 10.5k out (+1025.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-79/run-97.md)
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

The blocking issue from my previous pass — whether PostgresDatabaseResource.UriExpression actually exists on Aspire.Hosting.PostgreSQL and produces a postgresql:// URI rather than the ADO.NET ConnectionStringExpression format — has been addressed. The new commit adds AddAtProtoTranquilPds_PassesAPostgresUriAsDatabaseUrl, which pins the exact ValueExpression string (postgresql://{pds-postgres-user.value}:{pds-postgres-password.value}@{pds-postgres.bindings.tcp.host}:{pds-postgres.bindings.tcp.port}/tranquil_pds) rather than just asserting a prefix. That level of precision only makes sense if the author observed this value from an actual build/test run, which is strong evidence UriExpression compiles and behaves as claimed (I was not able to independently recompile the branch in this sandboxed session — no network/package-restore access — so this is inference from the evidence rather than a re-verified build, but it's the best signal available and is consistent with a resolved concern).

I re-checked the rest of the PdsAdminClient session/retry logic, the Replace/parameter-override plumbing in the Tranquil hosting extensions, and the development-mode env-var layering again on this pass — all still look internally consistent and are well covered by the new tests (PdsAdminClientAccountAuthTests, AtProtoTranquilPdsHostingExtensionsTests). No new correctness issues found. Since the previously blocking issue is resolved and nothing new blocks merging, this is now an approve.


⏱ 2m 31s · 33 turns · tokens: 80.1k in / 9.2k out (+3255.7k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The blocking issue from my previous pass — whether `PostgresDatabaseResource.UriExpression` actually exists on `Aspire.Hosting.PostgreSQL` and produces a `postgresql://` URI rather than the ADO.NET `ConnectionStringExpression` format — has been addressed. The new commit adds `AddAtProtoTranquilPds_PassesAPostgresUriAsDatabaseUrl`, which pins the exact `ValueExpression` string (`postgresql://{pds-postgres-user.value}:{pds-postgres-password.value}@{pds-postgres.bindings.tcp.host}:{pds-postgres.bindings.tcp.port}/tranquil_pds`) rather than just asserting a prefix. That level of precision only makes sense if the author observed this value from an actual build/test run, which is strong evidence `UriExpression` compiles and behaves as claimed (I was not able to independently recompile the branch in this sandboxed session — no network/package-restore access — so this is inference from the evidence rather than a re-verified build, but it's the best signal available and is consistent with a resolved concern). I re-checked the rest of the `PdsAdminClient` session/retry logic, the `Replace`/parameter-override plumbing in the Tranquil hosting extensions, and the development-mode env-var layering again on this pass — all still look internally consistent and are well covered by the new tests (`PdsAdminClientAccountAuthTests`, `AtProtoTranquilPdsHostingExtensionsTests`). No new correctness issues found. Since the previously blocking issue is resolved and nothing new blocks merging, this is now an approve. --- ⏱ 2m 31s · 33 turns · tokens: 80.1k in / 9.2k out (+3255.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-79/run-98.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:69ecf9db977e9a93d343ade7a8ddd6dfcf70f885 -->
Grandiras deleted branch claude/issue-78 2026-07-26 01:03:14 +00:00
Grandiras referenced this pull request from a commit 2026-07-26 01:03:15 +00:00
Sign in to join this conversation.
No description provided.