chore: update fullsend shim workflow#2101
Conversation
Update the shim workflow to match the current template in the .fullsend config repo.
Site previewPreview: https://f6b3b120-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 10:42 PM UTC · Completed 10:50 PM UTC |
ReviewFindingsHigh
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| --- | ||
| # --- fullsend managed below - do not edit --- | ||
| # lint-workflow-size: max-lines=280 | ||
| # fullsend shim workflow (workflow_call mode) |
There was a problem hiding this comment.
[high] runtime mechanism failure
The PR removes the sentinel comment # --- fullsend managed below - do not edit --- from this repo's shim workflow. The reconcile-repos.sh script (line 29) depends on this sentinel to separate user-owned headers from managed content via extract_managed_content() and extract_user_header(). When the sentinel is absent, managed_content_b64() treats the entire file as managed, while the template retains the sentinel. This causes a content mismatch on the next reconciliation run, creating unnecessary churn. The template at internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml still contains both lines, so this PR creates divergence rather than syncing.
Suggested fix: Do not remove the sentinel comment. If the intent is to remove it from all enrolled repos, update the template and the sentinel-dependent logic in reconcile-repos.sh in the same PR.
There was a problem hiding this comment.
The review bot is right on this one. Verified that:
- The template at
internal/scaffold/fullsend-repo/templates/shim-workflow-call.yamlstill has the---and sentinel comment reconcile-repos.shusesSENTINEL="# --- fullsend managed below - do not edit ---"to split user-owned headers from managed content viaextract_managed_content()/extract_user_header()- The test suite (
reconcile-repos-test.sh) also asserts the sentinel is present in generated blobs
Removing these two lines from the enrolled repo while the template keeps them will cause managed_content_b64() to compute a different hash on the next reconciliation run, triggering another onboard PR to restore them — creating an infinite loop of churn.
This PR should not be merged as-is. If the intent is to drop the sentinel from shim workflows, it needs to be done atomically: update the template, update reconcile-repos.sh sentinel logic, and update the test assertions in the same change.
|
🤖 Finished Retro · ✅ Success · Started 10:53 AM UTC · Completed 10:59 AM UTC |
Retro: PR #2101 — chore: update fullsend shim workflowWhat happened
AssessmentReview quality: Excellent. The review bot caught a real, consequential bug on the first pass. Its explanation was detailed, accurate, and actionable. A human reviewer independently confirmed the findings. This is the review system working as intended. The failure was in the human override. A PR with confirmed high-severity findings and no subsequent fixes was approved and merged by the same person who initially flagged others for input. The merge queue did not block it because the bot's Existing issue coverageAll identified improvement opportunities are already tracked by open issues:
No new proposals filed — both core issues are already tracked. Resolving #2247 would prevent the bad PR from being generated, and resolving #1201 would add guardrails against merging when high-severity findings are unaddressed. |
2 participants
This PR updates the fullsend shim workflow to match the current template in the
.fullsendconfig repo.The shim content has drifted from the template — this brings it back in sync.