Reset smoke harness idempotence state#750
Conversation
Clear the Fedify harness inbox idempotence keys when the smoke test reset endpoint is called. This keeps repeated Sharkey follow/unfollow scenarios from being filtered as duplicate activities after the visible test inbox has already been cleared. Fixes fedify-dev#749 Assisted-by: Codex:gpt-5.5
dahlia
added
type/test
|
@codex review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds federation test-state cleanup: the federation harness reuses a single in-memory KV store and exposes Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 6/8 reviews remaining, refill in 10 minutes and 31 seconds.Comment |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/federation.ts`:
- Around line 19-25: resetFederationTestState currently only clears the
["_fedify","activityIdempotence"] prefix so leftover federation caches remain;
update resetFederationTestState to also list and delete keys for the other
Federation KV prefixes used by the harness (["_fedify","remoteDocument"],
["_fedify","publicKey"], ["_fedify","httpMessageSignaturesSpec"],
["_fedify","acceptSignatureNonce"]) by iterating over each prefix with
kv.list(...) and adding their keys to the deletion batch (then await Promise.all
on kv.delete calls) so /_test/reset produces a full clean slate.
🪄 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: 21710b6c-fd9e-4c71-b1b9-8a2c08b0d782
📒 Files selected for processing (2)
test/smoke/harness/backdoor.tstest/smoke/harness/federation.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a state reset mechanism for the smoke test harness by adding a resetFederationTestState function and integrating it into the /_test/reset backdoor endpoint. The feedback suggests making the reset more robust by clearing all entries in the KV store rather than just specific idempotence keys, which helps prevent potential test flakiness from stale cached data.
Clear the smoke harness KV store completely when /_test/reset is called because the harness owns its MemoryKvStore exclusively. This covers idempotence and cached federation documents, keys, signature state, and nonces so each smoke scenario starts from clean federation state. Resolves review comments: fedify-dev#750 (comment) fedify-dev#750 (comment) Assisted-by: Codex:gpt-5.5
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request adds a resetFederationTestState function to clear the KV store and integrates it into the test reset endpoint. Feedback indicates that the current implementation clears more than intended and suggests using a prefix to target specific states, while also recommending a more type-safe approach for deleting keys to align with repository standards.
Delete each smoke harness KV entry as it is listed instead of collecting keys first. The harness still clears the full dedicated MemoryKvStore so /_test/reset remains a clean federation-state reset. Resolves review comment: fedify-dev#750 (comment) Assisted-by: Codex:gpt-5.5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces a state reset mechanism for the federation test harness by clearing the key-value store during the /_test/reset operation. A technical issue was identified in the resetFederationTestState function where the kv.list() method is called without the required selector argument, which could lead to runtime exceptions.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request adds a way to reset the federation test state by clearing the KV store. It introduces the resetFederationTestState function in the test harness and updates the backdoor reset endpoint to call it. Feedback was provided regarding the scope of the KV store clearing, suggesting that a targeted deletion using specific key prefixes might be more appropriate than a full wipe.
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request introduces a state reset mechanism for the smoke test harness by adding a resetFederationTestState function that clears the MemoryKvStore. Feedback suggests refining this function to specifically target idempotency-related keys using the ["_fedify", "activityIdempotence"] prefix, ensuring the reset is more precise and avoids clearing unrelated state.
2 participants
Fixes #749.
Summary
This fixes the
smoke-sharkeyfailure in #749 by resetting Fedify's inbox idempotence state when the smoke harness reset endpoint is called.The test harness already cleared its visible inbox state in test/smoke/harness/backdoor.ts, but it kept the underlying
MemoryKvStoreused by the Fedify federation instance in test/smoke/harness/federation.ts. That meant repeated activities with the same ActivityPub id could be accepted at the HTTP delivery layer but skipped before reaching the inbox listeners.What we found
After reproducing the failure locally, Sharkey 2025.4.6 was not failing to deliver
Undo(Follow)outright. Its deliver queue contained successfulUndojobs tohttps://fedify-harness/users/testuser/inbox.The important detail was that Sharkey reused the same
Undoactivity id across repeated follow/unfollow cycles for the same pair of actors:https://sharkey/follows/<followerId>/<followeeId>/undoFedify correctly treats already-seen inbox activities as idempotent for one day. Since
/_test/resetonly cleared the harness' visible inbox array, the next SharkeyUndowith the same id was filtered as already processed and never reached the test store. The smoke assertion then timed out even though Sharkey's delivery job reported success.Changes
MemoryKvStorein test/smoke/harness/federation.ts.["_fedify", "activityIdempotence"]entries./_test/resetin test/smoke/harness/backdoor.ts.Verification
deno fmt test/smoke/harness/federation.ts test/smoke/harness/backdoor.tsdeno cache test/smoke/harness/main.tssmoke-sharkeyreproduction test passed: