Harden the Check workflow; thumbnails at the OG size - #2
Conversation
…e OG size - The changed files come from the PR's file list, renames included, capped at 3000, and any outside projects/<id>/ is refused. - The author is matched by numeric account id, so a renamed or re-registered login cannot take a project over; a non-author must already be in contributors or be added by the same PR. - Every login named must exist; any non-zero sp pack --check fails. - thumbnail.png is 2400x1260, an Open Graph image twice over, 5 MB at most. - The index job builds from main, and sorts by parsed timestamps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a42b236003
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| listed = {e["id"]: e["author"]["id"] for e in read(Path("index.json"))["projects"]} | ||
|
|
||
| def author_id(name): | ||
| return listed.get(name) or user_id(read(base[name] / "project.json")["author"]) |
There was a problem hiding this comment.
When an author hands a project to another user, the project commit reaches main before the separate Index workflow commits the regenerated index.json. A Check workflow started in that interval reads the new project.json but this expression still returns the former author's ID from the stale index, so the former author can remove or hand off the project while the actual new author is denied those operations. Compare the indexed login with the base project's author before trusting its ID, or otherwise make the identity source update atomically with the handoff.
Useful? React with 👍 / 👎.
| found = user(f"user/{known[login.lower()]}" if login.lower() in known else f"users/{login}") | ||
| return {"login": found["login"], "id": found["id"]} if found else {"login": login, "id": None} |
There was a problem hiding this comment.
If a previously indexed author deletes their account, known still contains the immutable numeric ID, but a 404 now rewrites that entry with id: None. The checker consequently falls back to resolving the author string by login; if GitHub later assigns that login to someone else, the new account is treated as the project's author and gains removal and handoff privileges. Keep the known ID on lookup failure rather than discarding the identity that this change is intended to protect.
Useful? React with 👍 / 👎.
| # A rename lists its old path too, which is a change to the folder it left. | ||
| changed = gh("--paginate", f"repos/{os.environ['GITHUB_REPOSITORY']}/pulls/" | ||
| f"{os.environ['PR']}/files", "--jq", ".[] | .filename, .previous_filename // empty") | ||
| found = check(Path(sys.argv[2]), sys.argv[3], int(sys.argv[4]), changed.split()) |
There was a problem hiding this comment.
For a package containing a valid filename with spaces or other whitespace, gh --jq emits the path on one line but changed.split() breaks it into multiple alleged paths. The first fragment may select the project while the remaining fragments are reported as changes outside projects/<id>/, causing the Check workflow to reject the PR. Parse the command output without whitespace tokenization, such as by returning JSON and decoding the filename array.
Useful? React with 👍 / 👎.
1 participant
Fixes from a three-way review of the Check workflow, plus the new thumbnail size.
projects/<id>/fails.index.json, not the login. A non-author must already be incontributors, or be added by this PR.author/contributorslogin must exist. Any non-zerosp pack --checkfails, even with no problem line.thumbnail.pngmust be 2400 × 1260 (an Open Graph image's 1200 × 630, twice over) and at most 5 MB.index.ymlchecks outmainexplicitly; the sort parses timestamps.--checkreports a missing id/author that-ofills in) and the thumbnail rule.Needs ReScienceLab/super-prototyping#183's
spfor the new thumbnail size.🤖 Generated with Claude Code