Fix SF /_atomic mock for ?waitForIndex=true; rename test fixture username#4707
Merged
Fix SF /_atomic mock for ?waitForIndex=true; rename test fixture username#4707
Conversation
The recent ?waitForIndex=true opt-in adds a query string to the /_atomic POST URL when the factory entrypoint syncs the seed batch. The integration mock's exact-string match against '/hassan/personal/_atomic' no longer matches, so the request fell through to the generic POST handler, which returned 204 No Content. The sync client expects 201, surfaced "Atomic upload failed: 204 No Content", and the factory:go subprocess exited 1 — failing every shard 1 run of pnpm test:node since 9444b82. Switch to a startsWith check so the handler accepts both the plain URL and the ?waitForIndex=true form, matching the pattern used by the other /hassan/personal/* handlers in this mock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The integration mock and a few sibling tests hardcoded 'hassan' as the Matrix user / realm owner across user IDs, account-data paths, and realm URLs. Swap to the neutral 'testuser' so the fixture doesn't read like it belongs to a real teammate. No behavior change — the username is just the test fixture. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
habdelra
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Software Factory Node integration test mock to correctly handle /_atomic requests that include query strings (e.g. ?waitForIndex=true), and renames the SF test fixture username to a neutral testuser to avoid embedding a real teammate’s name in test data.
Changes:
- Update the integration test realm mock to match
POST /_atomicviastartsWith(...)so query strings don’t fall through to the generic handler. - Rename fixture username usages from
hassan→testuseracross Software Factory test files. - Update expected owner usernames / realm URLs in unit and integration tests to reflect the renamed fixture.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/software-factory/tests/factory-target-realm.test.ts | Updates test profile + expected realm URLs/owner username to testuser. |
| packages/software-factory/tests/factory-entrypoint.test.ts | Updates fixture realm/owner username assertions to testuser. |
| packages/software-factory/tests/factory-entrypoint.integration.test.ts | Fixes /_atomic mock matching to allow query strings; renames all mocked URLs/user IDs to testuser. |
| packages/software-factory/tests/factory-agent-schema-boundary.test.ts | Updates agent context fixture targetRealm to testuser. |
| packages/software-factory/tests/factory-agent-claude-code.test.ts | Updates agent context fixture targetRealm to testuser. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Summary
204 No Contentfailure on shard 1/3 of SF Node tests. The?waitForIndex=trueopt-in landed in 9444b82 makes the factory entrypoint append a query string to the/_atomicPOST URL when syncing the seed batch. The integration mock checkedrequest.url === '/testuser/personal/_atomic'(exact match), so the new URL fell through to the generic POST handler and got204 No Content. The sync client expects201→ reportedAtomic upload failed: 204 No Content, factory:go exited 1, andpnpm test:nodefailed atfactory-entrypoint integration > factory:go --debug prints a structured JSON summary. Switched the handler tostartsWith('/testuser/personal/_atomic'), consistent with the other handlers already in this mock.hassantotestuseracross 5 SF test files (34 occurrences). The fixture had a real teammate's name baked into Matrix user IDs, account-data paths, and realm URLs;testuseris neutral.mainsince the merge — not flake. Failure example: https://github.com/cardstack/boxel/actions/runs/25493597169/job/74808011882Test plan
pnpm test:nodeinpackages/software-factorypasses locally (440 pass, 0 fail) before and after the rename