Add Sharkey (Misskey-family) interoperability smoke tests#659
Open
sij411 wants to merge 2 commits intofedify-dev:mainfrom
Open
Add Sharkey (Misskey-family) interoperability smoke tests#659sij411 wants to merge 2 commits intofedify-dev:mainfrom
sij411 wants to merge 2 commits intofedify-dev:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a Sharkey (Misskey-family) interoperability smoke-test suite: new GitHub Actions workflow, Docker Compose test stack with Caddy TLS proxies, cert generation and provisioning scripts, Deno harness/orchestrator updates for async recipient resolution and robust account lookup, and related test artifacts. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions\n(smoke-sharkey)"
participant Runner as "Ubuntu Runner"
participant Docker as "Docker Compose"
participant DB as "PostgreSQL"
participant Redis as "Redis"
participant Caddy as "Caddy TLS Proxies"
participant Sharkey as "Sharkey web backend"
participant Harness as "Fedify harness (Deno)"
participant Orchestrator as "orchestrator.ts"
GH->>Runner: trigger workflow (push/cron/manual)
Runner->>Docker: generate certs, pull images, start stack
Docker->>DB: start & healthcheck
Docker->>Redis: start & healthcheck
Docker->>Sharkey: start web backend
Docker->>Caddy: start TLS proxies (harness & sharkey)
Runner->>Sharkey: run provision.sh (create users/tokens/follows)
Runner->>Harness: run Deno harness tests (deno run)
Orchestrator->>Sharkey: lookup accounts, poll inboxes, validate activities
Orchestrator->>Harness: verify harness-side expectations
Runner->>Docker: on failure collect compose logs
Runner->>Docker: teardown stack (down -v)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces smoke tests for Sharkey, including a Docker Compose environment with Caddy for TLS termination, provisioning scripts, and configuration files. The test harness has been updated to support WebFinger resolution with caching and improved account lookup logic that falls back to Sharkey-specific endpoints. Additionally, error handling was added to harness inbox fetches, and the unfollow test assertion order was refined. Feedback was provided regarding the lack of timeouts in the new fetch calls within the WebFinger resolution logic, which could cause tests to hang.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/smoke-sharkey.yml:
- Around line 19-26: Add an explicit minimal GITHUB_TOKEN permissions block to
the workflow so it doesn't inherit repo defaults: under the top-level workflow
definition (near the existing concurrency / jobs: smoke block), add a
permissions section that grants only read access to the repository contents
(e.g., permissions: contents: read) to limit the token to the minimal scope
required by the smoke job.
In `@test/smoke/harness/backdoor.ts`:
- Around line 62-64: The fallback constructs an inboxId with the variable scheme
but hardcodes https for actorId, causing mismatched http/https URIs; update the
actorId construction (actorId) to use the same scheme variable (scheme) as
inboxId (e.g., `${scheme}://${domain}/users/${user}`) so both inboxId and
actorId share the same scheme.
In `@test/smoke/orchestrator.ts`:
- Around line 159-205: The empty catch blocks swallow all errors; change them to
catch the error (e.g., catch (err)) and only suppress expected "endpoint
unsupported" cases (HTTP 404 from serverGet or fetch) while rethrowing
everything else; specifically update the serverGet try/catch blocks around
`/api/v1/accounts/search` and `/api/v1/accounts/lookup` to inspect the caught
error (check for err.status === 404 or err instanceof Response with status 404
or the presence of "404" in err.message) and rethrow non-404 errors, and do the
same for the POST to `/api/users/show` (catch errors from fetch and only swallow
a 404/not-found network response, otherwise rethrow).
In `@test/smoke/sharkey/default.yml`:
- Around line 19-22: The allowedPrivateNetworks block currently uses overly
broad catch-alls (0.0.0.0/0 and ::/0) which effectively disables the
restriction; update the allowedPrivateNetworks entry to list only the specific
private CIDR ranges the smoke stack actually uses (replace 0.0.0.0/0 and ::/0 in
allowedPrivateNetworks) — e.g. the project’s Docker/compose subnets (like the
specific 172.x.x.x/16 or 192.168.x.x/16 ranges your smoke stack uses) and any
required internal IPv6 prefix, or better yet reference the smoke stack’s
configured subnet variables instead of wildcarding everything. Ensure loopback
(::1/128, 127.0.0.0/8) is not inadvertently allowed unless explicitly needed.
In `@test/smoke/sharkey/docker-compose.yml`:
- Line 87: Replace the inline mapping "fedify-harness-backend: { condition:
service_healthy }" with a normalized block mapping so YAML linter accepts it;
locate the depends_on entries (the "fedify-harness-backend" mapping) and change
each inline form to a two-line mapping with "fedify-harness-backend:" on one
line and an indented "condition: service_healthy" on the next, preserving
surrounding indentation and aligning with the existing depends_on structure
(also update the other occurrences noted around the same stanza).
In `@test/smoke/sharkey/provision.sh`:
- Line 19: Remove printing of raw credential-bearing responses and tokens in
provision.sh: stop echoing variables like ADMIN_RAW (and similar echoes at the
other mentioned locations) that may contain bearer tokens, passwords, or reset
links; instead log only non-sensitive status messages (e.g., "admin creation
succeeded") or a sanitized summary (no tokens, no passwords, no reset URLs), and
if you must indicate truncated content, ensure you redact token-like patterns
rather than printing the raw body. Update the echo statements that reference
ADMIN_RAW and the other echoed response variables to either omit the variable
entirely or replace it with a safe, redacted summary so no secret material
appears in CI logs.
- Around line 124-132: The follow request currently silences failures with "||
true" and stores output in FOLLOW_RAW, so failures are ignored; remove the "||
true" and instead check curl's exit/status and fail fast: run the POST to
"$SHARKEY_URL/api/following/create" into FOLLOW_RAW (keep TEST_TOKEN and
FEDIFY_USER_ID), capture curl's exit code or use curl -f, and if non-zero print
a clear error including FOLLOW_RAW and exit non-zero so the bootstrap stops on
failed follow attempts.
- Around line 134-141: The .env.test heredoc writes unescaped assignments (e.g.,
SERVER_ACCESS_TOKEN=$TOKEN) which can allow unwanted shell expansion when later
sourced; instead emit shell-safe assignments by escaping values with printf '%q'
(or similar) for each variable before writing—replace the heredoc with lines
like printf "SERVER_ACCESS_TOKEN=%q\n" "$TOKEN" (and do the same for
SERVER_BASE_URL, SERVER_INTERNAL_HOST, HARNESS_BASE_URL, HARNESS_ORIGIN) into
test/smoke/.env.test so the file contains safe, quoted/escaped assignments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6954926-bbe5-4084-8c3c-1907704b872d
📒 Files selected for processing (11)
.github/workflows/smoke-sharkey.yml.gitignoretest/smoke/harness/backdoor.tstest/smoke/orchestrator.tstest/smoke/sharkey/Caddyfile.fedify-harnesstest/smoke/sharkey/Caddyfile.sharkeytest/smoke/sharkey/default.ymltest/smoke/sharkey/docker-compose.ymltest/smoke/sharkey/docker.envtest/smoke/sharkey/generate-certs.shtest/smoke/sharkey/provision.sh
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/smoke/harness/backdoor.ts`:
- Around line 65-70: The fallback Mastodon-convention result (inboxId, actorId
stored in result) should not be cached indefinitely: remove the
recipientCache.set(handle, result) call in the fallback branch so transient
failures don't persistently poison the cache, or alternatively implement a TTL
for fallback entries by storing a timestamp with the fallback result and
updating recipientCache lookup logic to reject/refresh entries older than the
TTL; update the code around recipientCache, handle, inboxId, actorId, and result
accordingly.
- Around line 61-63: The empty catch in the WebFinger/actor fetch block swallows
all errors; update the catch to accept an error parameter and handle expected
"not found" cases while surfacing unexpected failures (e.g., log error details
or re-throw). Specifically, in the try that performs WebFinger lookup (the block
surrounding the current empty catch in backdoor.ts), inspect the caught error
(HTTP status, network/DNS error, or JSON parse error) and only suppress when the
response indicates WebFinger not supported or 404; otherwise call the existing
logger (or console.error) with the error details or re-throw so TLS/DNS/JSON
failures are visible during smoke tests.
In `@test/smoke/orchestrator.ts`:
- Around line 158-203: The fallback chain currently ignores non-OK HTTP
responses (e.g., 5xx) and silently proceeds, so add logging for unexpected
non-OK responses in the poll handler: when checking searchRes, lookupRes and
misskeyRes, if !res.ok and res.status !== 404 log the endpoint, status and
response body (or statusText) before falling through; update the blocks around
searchRes, lookupRes and misskeyRes (the variables named searchRes, lookupRes,
misskeyRes inside the poll("Fedify user resolvable", async () => { ... })
function) to await and log response.text() or statusText for non-404 errors so
server-side errors are visible in test output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 38b521ef-c224-4475-abf4-cf6692c1d7f8
📒 Files selected for processing (2)
test/smoke/harness/backdoor.tstest/smoke/orchestrator.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
test/smoke/sharkey/provision.sh (3)
135-141:⚠️ Potential issue | 🟠 MajorWrite
.env.testusing shell-escaped assignments.This file is sourced later; raw assignments at Line 138 can execute unintended shell expansion if values contain metacharacters. Emit escaped values (
printf %q) for each variable.Suggested patch
-cat > test/smoke/.env.test <<EOF -SERVER_BASE_URL=http://localhost:3000 -SERVER_INTERNAL_HOST=sharkey -SERVER_ACCESS_TOKEN=$TOKEN -HARNESS_BASE_URL=http://localhost:3001 -HARNESS_ORIGIN=https://fedify-harness -EOF +{ + printf 'SERVER_BASE_URL=%q\n' "http://localhost:3000" + printf 'SERVER_INTERNAL_HOST=%q\n' "sharkey" + printf 'SERVER_ACCESS_TOKEN=%q\n' "$TOKEN" + printf 'HARNESS_BASE_URL=%q\n' "http://localhost:3001" + printf 'HARNESS_ORIGIN=%q\n' "https://fedify-harness" +} > test/smoke/.env.test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/provision.sh` around lines 135 - 141, The here-doc that writes test/smoke/.env.test uses raw assignments which can be subject to shell expansion for values like SERVER_ACCESS_TOKEN; update the block that creates the file (the cat > test/smoke/.env.test <<EOF ... EOF fragment) to emit shell-escaped assignments by using printf '%q' for each value (e.g., printf "SERVER_BASE_URL=%q\n" "$SERVER_BASE_URL" and similarly for SERVER_INTERNAL_HOST, SERVER_ACCESS_TOKEN, HARNESS_BASE_URL, HARNESS_ORIGIN) so the file is safe to source later without unintended expansion.
34-34:⚠️ Potential issue | 🟠 MajorStop logging token material, even truncated.
Line 34, Line 86, and Line 143 still expose token prefixes in CI logs. Please log only success/failure status without any token bytes.
Suggested patch
-echo " admin token: ${ADMIN_TOKEN:0:8}..." +echo " admin token acquired" ... -echo " testuser token: ${TEST_TOKEN:0:8}..." +echo " testuser token acquired" ... -echo "✓ Provisioning complete (token: ${TOKEN:0:8}...)" +echo "✓ Provisioning complete"Also applies to: 86-86, 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/provision.sh` at line 34, Remove all printing of token material (even truncated) in the provision script: stop echoing ADMIN_TOKEN and any other TOKEN variables; instead log only a non-sensitive status message like "admin token created" or "admin token set" on success/failure. Update the occurrences that echo "${ADMIN_TOKEN:0:8}" (and similar token-prefix echoes at the other occurrences) to emit a generic success/failure message and ensure no token substring is written to stdout/stderr or CI logs.
52-77:⚠️ Potential issue | 🟠 MajorDon’t swallow transport failures in signin/reset fallback calls.
Line 52, Line 61, Line 67, and Line 73 use
|| true, which masks network/runtime failures and turns them into generic auth flow failures. Keep fallback for expected auth outcomes, but let transport errors fail fast.Suggested direction
-SIGN_IN_RAW=$(curl -sf -X POST "$SHARKEY_URL/api/signin" \ +SIGN_IN_RAW=$(curl -sS -X POST "$SHARKEY_URL/api/signin" \ -H "Content-Type: application/json" \ - -d '{"username": "testuser", "password": "testpassword123"}' 2>&1) || true + -d '{"username": "testuser", "password": "testpassword123"}')Use HTTP/body checks to decide fallback, but do not blanket-ignore curl execution errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/provision.sh` around lines 52 - 77, The curl invocations that populate SIGN_IN_RAW, TESTUSER_INFO, RESET_RAW and the second SIGN_IN_RAW are swallowing transport failures via "|| true"; change them to preserve curl's exit status (remove "|| true") and explicitly check curl's exit code after each call: if curl returns non-zero (network/transport/runtime error) exit the script/report error immediately, but if curl succeeded and the response body lacks the expected fields (e.g., jq checks on '.i', '.id', '.password'), continue with the auth fallback logic; apply this to the SIGN_IN_RAW initial call, TESTUSER_INFO, RESET_RAW, and the signin-after-reset block so only real transport failures fail fast while expected auth failures still trigger fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/smoke/sharkey/provision.sh`:
- Around line 135-141: The here-doc that writes test/smoke/.env.test uses raw
assignments which can be subject to shell expansion for values like
SERVER_ACCESS_TOKEN; update the block that creates the file (the cat >
test/smoke/.env.test <<EOF ... EOF fragment) to emit shell-escaped assignments
by using printf '%q' for each value (e.g., printf "SERVER_BASE_URL=%q\n"
"$SERVER_BASE_URL" and similarly for SERVER_INTERNAL_HOST, SERVER_ACCESS_TOKEN,
HARNESS_BASE_URL, HARNESS_ORIGIN) so the file is safe to source later without
unintended expansion.
- Line 34: Remove all printing of token material (even truncated) in the
provision script: stop echoing ADMIN_TOKEN and any other TOKEN variables;
instead log only a non-sensitive status message like "admin token created" or
"admin token set" on success/failure. Update the occurrences that echo
"${ADMIN_TOKEN:0:8}" (and similar token-prefix echoes at the other occurrences)
to emit a generic success/failure message and ensure no token substring is
written to stdout/stderr or CI logs.
- Around line 52-77: The curl invocations that populate SIGN_IN_RAW,
TESTUSER_INFO, RESET_RAW and the second SIGN_IN_RAW are swallowing transport
failures via "|| true"; change them to preserve curl's exit status (remove "||
true") and explicitly check curl's exit code after each call: if curl returns
non-zero (network/transport/runtime error) exit the script/report error
immediately, but if curl succeeded and the response body lacks the expected
fields (e.g., jq checks on '.i', '.id', '.password'), continue with the auth
fallback logic; apply this to the SIGN_IN_RAW initial call, TESTUSER_INFO,
RESET_RAW, and the signin-after-reset block so only real transport failures fail
fast while expected auth failures still trigger fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6e1c0cad-49cf-450c-b674-ed26329f6e41
📒 Files selected for processing (1)
test/smoke/sharkey/provision.sh
Adds interoperability smoke tests for Sharkey, a Misskey fork. Sharkey
requires HTTPS for all ActivityPub federation lookups (hard-coded in
checkHttps() and WebFingerService), so the setup uses Caddy TLS proxies
with self-signed certificates.
Key changes:
- New GitHub Actions workflow (smoke-sharkey) that boots a Sharkey
stack with Caddy TLS proxies, provisions users, and runs E2E
scenarios via the shared orchestrator
- Test harness backdoor now resolves recipients via WebFinger + actor
document fetch instead of hardcoding Mastodon URL patterns, with a
cache that is cleared on /_test/reset
- Orchestrator account lookup falls back through Mastodon-compat
search, lookup, and Misskey-native /api/users/show endpoints
- res.ok checks before JSON parsing in inbox poll helpers
- poll() catches transient errors from callbacks and retries until
timeout
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Replace raw JSON response echoes with safe ok/failed status messages to avoid logging unnecessary API internals. Make the follow request fail the script immediately on error instead of silently continuing with `|| true`, which would cause confusing downstream test failures. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
♻️ Duplicate comments (5)
test/smoke/harness/backdoor.ts (1)
63-64:⚠️ Potential issue | 🟠 MajorScheme mismatch between
inboxIdandactorIdin fallback path.Line 63 constructs
inboxIdusing theschemevariable, but line 64 hardcodeshttps://foractorId. WhenSTRICT_MODEis off, this produces anhttpinbox URL paired with anhttpsactor URI, breaking non-TLS smoke stacks.Suggested fix
const inboxId = new URL(`${scheme}://${domain}/users/${user}/inbox`); - const actorId = new URL(`https://${domain}/users/${user}`); + const actorId = new URL(`${scheme}://${domain}/users/${user}`); const result = { inboxId, actorId };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/harness/backdoor.ts` around lines 63 - 64, The fallback constructs create inboxId with the variable scheme but build actorId with a hardcoded "https://", causing mismatched schemes when STRICT_MODE is off; update the actorId construction to use the same scheme variable (replace the `https://` literal with `scheme + '://'`) so both inboxId and actorId are created consistently (refer to the inboxId and actorId variables and the scheme value in backdoor.ts).test/smoke/sharkey/provision.sh (2)
135-141:⚠️ Potential issue | 🟠 MajorWrite
.env.testwith shell-escaped assignments before sourcing.
SERVER_ACCESS_TOKEN=$TOKENis emitted raw, then sourced by the workflow. Escape each value when writing to prevent unintended shell expansion.🛡️ Suggested fix
-echo "→ Writing test env..." -cat > test/smoke/.env.test <<EOF -SERVER_BASE_URL=http://localhost:3000 -SERVER_INTERNAL_HOST=sharkey -SERVER_ACCESS_TOKEN=$TOKEN -HARNESS_BASE_URL=http://localhost:3001 -HARNESS_ORIGIN=https://fedify-harness -EOF +echo "→ Writing test env..." +{ + printf 'SERVER_BASE_URL=%q\n' "http://localhost:3000" + printf 'SERVER_INTERNAL_HOST=%q\n' "sharkey" + printf 'SERVER_ACCESS_TOKEN=%q\n' "$TOKEN" + printf 'HARNESS_BASE_URL=%q\n' "http://localhost:3001" + printf 'HARNESS_ORIGIN=%q\n' "https://fedify-harness" +} > test/smoke/.env.test#!/bin/bash # Verify sourcing path and unsafe raw env emission pattern. rg -n -C2 'source test/smoke/.env.test|\. test/smoke/.env.test' .github/workflows rg -n -C3 'cat > test/smoke/.env.test|SERVER_ACCESS_TOKEN=\$TOKEN|printf .*%q' test/smoke/sharkey/provision.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/provision.sh` around lines 135 - 141, The here-doc in provision.sh writes SERVER_ACCESS_TOKEN=$TOKEN raw into test/smoke/.env.test which will be expanded when sourced; update the block that writes the .env.test (the cat > test/smoke/.env.test <<EOF ... EOF section) to shell-escape or quote each value before emitting (e.g., use printf '%s=%q\n' or wrap values in single quotes) so SERVER_ACCESS_TOKEN and other variables are written safely (refer to the here-doc writing of SERVER_BASE_URL, SERVER_INTERNAL_HOST, SERVER_ACCESS_TOKEN, HARNESS_BASE_URL, HARNESS_ORIGIN).
34-34:⚠️ Potential issue | 🟠 MajorStop logging token material (even prefixes) in CI output.
These lines still expose token fragments in logs. Replace with non-sensitive status messages only.
🧯 Suggested fix
-echo " admin token: ${ADMIN_TOKEN:0:8}..." +echo " admin token acquired" @@ -echo " testuser token: ${TEST_TOKEN:0:8}..." +echo " testuser token acquired" @@ -echo "✓ Provisioning complete (token: ${TOKEN:0:8}...)" +echo "✓ Provisioning complete"Also applies to: 86-86, 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/provision.sh` at line 34, Remove token material from CI logs by replacing any echo that prints ADMIN_TOKEN (e.g., the echo using ${ADMIN_TOKEN:0:8} at the lines shown) with a non-sensitive status message such as "admin token created" or "admin token set" and ensure the same change is applied to the other occurrences referenced (the echo statements that currently print token fragments at the other locations). Do not print any prefix or substring of secrets; if a debug mode is required, output that tokens are hidden and write the actual token only to a secure file or environment variable instead..github/workflows/smoke-sharkey.yml (1)
19-23:⚠️ Potential issue | 🟠 MajorDeclare least-privilege
GITHUB_TOKENpermissions explicitly.This workflow currently inherits default token permissions. Pin it to read-only repo contents at workflow level.
🔐 Suggested change
concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +permissions: + contents: read + jobs: smoke:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/smoke-sharkey.yml around lines 19 - 23, The workflow currently uses default token permissions; explicitly scope GITHUB_TOKEN to least privilege by adding a top-level permissions mapping with "contents: read" (i.e., set permissions: contents: read) so the workflow (including jobs and steps) only has read-only access to repo contents rather than broader default permissions; update the workflow YAML near the top-level (alongside concurrency/group and before jobs) to include this permissions entry referencing GITHUB_TOKEN.test/smoke/sharkey/docker-compose.yml (1)
87-87:⚠️ Potential issue | 🟡 MinorNormalize inline
depends_onmappings to pass YAMLlint.These mappings still include spaces inside
{}and will keep the lint gate failing.🧹 Suggested fix
- fedify-harness-backend: { condition: service_healthy } + fedify-harness-backend: {condition: service_healthy} @@ - db: { condition: service_healthy } - redis: { condition: service_healthy } + db: {condition: service_healthy} + redis: {condition: service_healthy} @@ - sharkey-web-backend: { condition: service_healthy } + sharkey-web-backend: {condition: service_healthy}Also applies to: 110-111, 134-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/smoke/sharkey/docker-compose.yml` at line 87, Normalize the inline depends_on YAML mappings by removing spaces inside the curly braces so they conform to YAMLlint; specifically update the entries like the "fedify-harness-backend: { condition: service_healthy }" (and the other duplicate occurrences) to use a compact inline mapping (e.g., {condition: service_healthy}) or convert them to the equivalent block mapping under depends_on, ensuring the key "fedify-harness-backend" and its mapping use the corrected format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/smoke-sharkey.yml:
- Around line 19-23: The workflow currently uses default token permissions;
explicitly scope GITHUB_TOKEN to least privilege by adding a top-level
permissions mapping with "contents: read" (i.e., set permissions: contents:
read) so the workflow (including jobs and steps) only has read-only access to
repo contents rather than broader default permissions; update the workflow YAML
near the top-level (alongside concurrency/group and before jobs) to include this
permissions entry referencing GITHUB_TOKEN.
In `@test/smoke/harness/backdoor.ts`:
- Around line 63-64: The fallback constructs create inboxId with the variable
scheme but build actorId with a hardcoded "https://", causing mismatched schemes
when STRICT_MODE is off; update the actorId construction to use the same scheme
variable (replace the `https://` literal with `scheme + '://'`) so both inboxId
and actorId are created consistently (refer to the inboxId and actorId variables
and the scheme value in backdoor.ts).
In `@test/smoke/sharkey/docker-compose.yml`:
- Line 87: Normalize the inline depends_on YAML mappings by removing spaces
inside the curly braces so they conform to YAMLlint; specifically update the
entries like the "fedify-harness-backend: { condition: service_healthy }" (and
the other duplicate occurrences) to use a compact inline mapping (e.g.,
{condition: service_healthy}) or convert them to the equivalent block mapping
under depends_on, ensuring the key "fedify-harness-backend" and its mapping use
the corrected format.
In `@test/smoke/sharkey/provision.sh`:
- Around line 135-141: The here-doc in provision.sh writes
SERVER_ACCESS_TOKEN=$TOKEN raw into test/smoke/.env.test which will be expanded
when sourced; update the block that writes the .env.test (the cat >
test/smoke/.env.test <<EOF ... EOF section) to shell-escape or quote each value
before emitting (e.g., use printf '%s=%q\n' or wrap values in single quotes) so
SERVER_ACCESS_TOKEN and other variables are written safely (refer to the
here-doc writing of SERVER_BASE_URL, SERVER_INTERNAL_HOST, SERVER_ACCESS_TOKEN,
HARNESS_BASE_URL, HARNESS_ORIGIN).
- Line 34: Remove token material from CI logs by replacing any echo that prints
ADMIN_TOKEN (e.g., the echo using ${ADMIN_TOKEN:0:8} at the lines shown) with a
non-sensitive status message such as "admin token created" or "admin token set"
and ensure the same change is applied to the other occurrences referenced (the
echo statements that currently print token fragments at the other locations). Do
not print any prefix or substring of secrets; if a debug mode is required,
output that tokens are hidden and write the actual token only to a secure file
or environment variable instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5291212d-9711-4f3b-ad4d-defdce64f284
📒 Files selected for processing (11)
.github/workflows/smoke-sharkey.yml.gitignoretest/smoke/harness/backdoor.tstest/smoke/orchestrator.tstest/smoke/sharkey/Caddyfile.fedify-harnesstest/smoke/sharkey/Caddyfile.sharkeytest/smoke/sharkey/default.ymltest/smoke/sharkey/docker-compose.ymltest/smoke/sharkey/docker.envtest/smoke/sharkey/generate-certs.shtest/smoke/sharkey/provision.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Fedify currently has smoke tests for Mastodon, but lacks coverage for Misskey-family servers which use different API conventions and URL patterns. Sharkey is a popular Misskey fork that represents this server family well.
A key challenge is that Sharkey requires HTTPS for all ActivityPub federation lookups —
checkHttps()inApPersonServicerejectshttp://URIs outright, andWebFingerService.genUrl()hardcodeshttps://(see misskey-dev/misskey#10716). This means the test infrastructure needs TLS termination, unlike the plain HTTP setup used for Mastodon non-strict tests.Additionally, Sharkey's Mastodon-compatible API layer has gaps (e.g.
/api/v1/accounts/searchreturns 404 for remote users,/api/v1/accounts/lookupthrowsTypeError), so the orchestrator needs Misskey-native API fallbacks to work reliably.Related: #654
Summary
smoke-sharkey) that boots a Sharkey stack with Caddy TLS proxies and self-signed certificates, provisions users, and runs E2E scenarios via the shared orchestratorsharkey-web-backend) to avoid DNS collision with the TLS proxy aliases (sharkey,fedify-harness)/_test/reset/api/users/showendpointsres.okchecks before JSON parsing in inbox poll helpers🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 [email protected]
Co-Authored-By: Claude Sonnet 4.6 [email protected]