Fix/academy review findings - #19
Merged
Merged
Conversation
The published API built successfully and then failed to start. `../../server` resolves from the workspace root, where Replit runs these commands, to a path outside the repository — so uv had nothing to run. The production build had been using `--project server` all along, which is why the build half worked and only the start half did not. The twelve other artifacts in this repo all run `pnpm --filter @workspace/<name>`, which likewise only resolves from the root. This one was the odd one out, and the only one that would not start. test_replit_deployment.py was pinning the broken value, so a sync would have reintroduced the outage on every merge. It now asserts the rule instead of a string: no command may contain `..`, and each must point at the server project from the root. Checked by mutation — putting `../../server` back fails it with the reason rather than a diff of two literals. I had recommended taking the repository's version over the deployment's during the merge conflict, on the grounds that the test was authoritative. It was not: it encoded an assumption no deployment had ever confirmed. 147 pytest pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On Android, tapping the Google button opened an "Open with application" chooser and the reader came back not signed in. The popup flow navigates to accounts.google.com, and that host is a verified Android App Link for the Google app: Chrome hands the navigation to the OS rather than completing it. It reproduces in ordinary Chrome and not in Incognito, which is what identified it — Incognito does not do the app handoff. FedCM has the browser draw the sign-in dialog itself, so there is no navigation for the OS to intercept. `use_fedcm_for_button` is opt-in and browsers without FedCM fall back to the popup flow unchanged, so asking for it costs nothing where it is not available. None of the methods the FedCM migration requires removing are used here, and the CSP already allows accounts.google.com in both connect-src and frame-src. Two hypotheses were tested and discarded first: an in-app WebView (the report was ordinary Chrome) and a mis-hit in the mobile drawer (measured on the live site — 127px of clear space below the community links, and elementFromPoint at the button's centre returns Google's own iframe). 174 component tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The résumé tool showed "API error (429): Too many AI requests. Please wait
before trying again." — an English sentence naming an HTTP status, inside a
Hebrew page, with no way forward in it.
Two things were wrong on that line. It built the message by hand instead of
using S.errApiPrefix, which the two direct-call paths already use, so a Hebrew
reader got "API error (" from the proxy and "שגיאת API (" from everywhere else.
And it passed the wire's answer straight through for the two statuses a visitor
actually meets.
429 and 503 now say what happened and what to do — connect a provider key of
your own, which is what lets them carry on either way. Anything else keeps the
status and the server's text, which is the useful thing to paste into a bug
report and is exactly what the other two are not.
This is not the cause of the 429 on the live site: that is still
SharedRateLimiter failing closed for want of RATE_LIMIT_SALT, which
/api/readyz reports. It is what the reader sees when a genuine allowance runs
out, and with a 10-request anonymous daily quota, they will.
246 unit and 174 component tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
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.
No description provided.