ci: add reviewed upstream sync workflow - #6
Conversation
🔎 Lint report:
|
![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: 041d5162c2
ℹ️ 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".
| git checkout -B "$BRANCH" origin/main | ||
| git merge --no-ff --no-edit upstream/main |
There was a problem hiding this comment.
git checkout -B "$BRANCH" origin/main resets automation/upstream-sync to origin/main on each execution, and the subsequent git merge --no-ff --no-edit upstream/main then creates a fresh merge commit even if upstream has not moved since the last run. In the common case where an upstream-sync PR is still open, this causes branch rewrites every 4 hours (retriggering CI repeatedly) and can overwrite manual conflict-resolution commits pushed to that branch. The workflow should preserve/reuse the existing sync branch (or detect that it already contains upstream/main) instead of forcibly recreating it each run.
Useful? React with 👍 / 👎.
| fi | ||
|
|
||
| git checkout -B "$BRANCH" origin/main | ||
| git merge --no-ff --no-edit upstream/main |
There was a problem hiding this comment.
With set -euo pipefail, a conflict in git merge --no-ff --no-edit upstream/main aborts the step before any push or PR update happens, so maintainers get no refreshable sync branch to resolve (despite the PR policy text saying conflicts should be resolved on this branch). In any upstream-vs-fork conflict scenario, this workflow fails hard and leaves the existing PR/branch stale instead of publishing a branch that can be fixed and reviewed.
Useful? React with 👍 / 👎.
| --head "$BRANCH" \ | ||
| --base main \ |
There was a problem hiding this comment.
gh pr list --head "$BRANCH" filters only by branch name, not by <owner>:<branch>, so this query can match open PRs from other forks that happen to use the same head ref name. In that case PR_NUMBER may target an unrelated PR and gh pr edit will update the wrong pull request instead of the automation PR. The lookup should also constrain head repository owner (e.g., via JSON fields like headRepositoryOwner) before selecting a PR number.
Useful? React with 👍 / 👎.
| git checkout -B "$BRANCH" origin/main | ||
| git merge --no-ff --no-edit upstream/main | ||
|
|
||
| git push --force-with-lease origin "HEAD:$BRANCH" |
There was a problem hiding this comment.
The workflow force-pushes with --force-with-lease, but it only fetches origin main and never fetches origin/$BRANCH, so the lease check for automation/upstream-sync is not anchored to the latest remote-tracking value for that branch. As a result, updates pushed to the sync branch by reviewers/maintainers between runs can still be overwritten by this forced push. Fetch the branch first (or use --force-with-lease=<ref>:<expected_sha>) so the lease actually protects that ref.
Useful? React with 👍 / 👎.
…te + cell_size_check + synchronous=FULL) Production corruption #6 left b-tree pages with zeroed headers but intact old cell content — the Bug E pattern. This fix applies three pragma calls on every connect(): - synchronous=FULL (was NORMAL): closes the WAL-checkpoint reordering window where a crash between WAL commit and main-DB write leaves a partially-written b-tree page header. Cost is <1ms per commit on local SSD; negligible at kanban write volume. - secure_delete=ON: forces SQLite to zero freed page bytes on disk. If a torn write or hardware fault later corrupts a page, the underlying cell content is zero, so corruption is detectable and no stale rows can resurface as live data. - cell_size_check=ON: adds a read-side guard so corrupt cells surface as errors at read time rather than as silent wrong-data returns. All three are connection-scoped and re-applied on every connect(). secure_delete also writes a persistent flag into the DB header on the first call against a fresh DB, making the protection durable across processes for new DBs. Tests added for all four required cases: each pragma active on a fresh connection, and all three re-applied after close+reopen. Also adds the required negative test (migration path does not reset pragmas).
(cherry picked from commit a75965a)
(cherry picked from commit a75965a)
1 participant
Summary
pai-scaffolde/hermes-agentfork.automation/upstream-syncinstead of auto-merging upstream intomain.Verification
git diff --check.github/workflows/upstream-sync-pr.ymlwith Python YAMLactionlintunavailable locally; skippedPolicy
This keeps NousResearch upstream changes review-gated while preserving Scaffolde fork patches.