docs(#6554): document scaffold-sync bot identity and write path - #7086
docs(#6554): document scaffold-sync bot identity and write path#7086fullsend-ai-coder[bot] wants to merge 6 commits into
Conversation
Add fullsend-ai-sync[bot] to the bot-identities table and document its three load-bearing properties: ruleset bypass (bypass_mode: always on main), workflow-write scope, and App-token push recursion (GITHUB_TOKEN suppression does not apply to App installation tokens). Explicitly disambiguate from the coder token statement in #6512 — the coder token has no workflows permission, but the sync App does. Both facts are correct; the gap was that the sync path was unwritten. Add a scaffold-sync dispatch recursion section to ci-workflows.md noting that notify-scaffold-sync fires on every push to main and sync commits re-trigger it (≥2 dispatch rounds per scaffold-touching merge). Item 4 from the issue (workflow file header comment) is excluded — the coder token cannot push workflow files. That change will be made separately by a maintainer. Note: pre-commit hooks were not run. pre-commit could not complete (infrastructure failure — network access blocked for remote hook repos). Hooks were run directly where possible: trailing whitespace, end-of-file, docs-links, and markdown link checks all passed. Closes #6554
|
🤖 Finished Review · ✅ Success · Started 12:11 AM UTC · Completed 12:24 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.30 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsBot-authored docs-only PR adding 4 files and 45 lines carries minimal Tier 1 risk; re-review anchoring preserves the prior score of 2 because Tier 1 signals are unchanged and Tier 2 churn on ci-workflows.md remains consistently elevated as in the prior assessment. Previous runRisk Assessment: moderate (2/5) DetailsBot-authored docs-only PR with 3 files and 31 lines carries minimal Tier 1 risk; re-review anchoring preserves the prior score of 2 because Tier 1 signals are unchanged and Tier 2 churn on ci-workflows.md (9 commits/30d, 5 authors/90d, 5 fix-reverts/90d) remains consistently elevated as in the prior assessment. Previous run (2)Risk Assessment: moderate (2/5) DetailsBot-authored docs-only PR with 3 files and 29 lines carries minimal Tier 1 risk; re-review anchoring preserves the prior score of 2 because Tier 1 signals are unchanged and Tier 2 churn on ci-workflows.md (9 commits/30d, 5 authors/90d, 5 fix-reverts/90d) remains consistently elevated as in the prior assessment. Previous run (3)Risk Assessment: moderate (2/5) DetailsBot-authored docs-only PR with 3 files and 27 lines carries minimal Tier 1 risk; re-review anchoring preserves the prior score of 2 because Tier 1 signals are unchanged and Tier 2 churn on ci-workflows.md (9 commits/30d, 5 authors/90d, 7 fix-reverts/90d) remains consistently elevated as described in the prior assessment. Previous run (4)Risk Assessment: moderate (2/5) DetailsSmall docs-only bot-authored PR with no security, CI, or dependency risk; composite score remains at 2 per re-review anchoring — Tier 1 signals are unchanged from the prior assessment and Tier 2 churn and multi-author contention on these documentation files remain similarly elevated, preserving the moderate score. Previous run (5)Risk Assessment: moderate (2/5) DetailsSmall docs-only bot-authored PR with no security, CI, or dependency risk; composite score driven slightly above minimum by high fix-commit churn and multi-author contention on these documentation files in recent history. |
ReviewFindingsLow
Next steps:
Previous runReviewFindingsLow
Next steps:
Previous run (2)ReviewFindingsMedium
Next steps:
Previous run (3)ReviewFindingsMedium
Next steps:
Previous run (4)ReviewFindingsMedium
Next steps:
Previous run (5)ReviewFindingsLow
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 12:27 AM UTC · Completed 12:33 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.14 |
Remove numeric prefixes from ### headings under Scaffold-sync write path in bot-identities.md to match the plain-descriptive heading convention used throughout docs/contributing/. Update the cross- reference anchor in ci-workflows.md accordingly. Addresses #7086
🔧 Fix agent — iteration 1 (bot-triggered)Dropped numeric prefixes from three subsection headings in bot-identities.md and updated the corresponding cross-reference anchor in ci-workflows.md to match the naming convention used across docs/contributing/. Fixed (1):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 12:35 AM UTC · Completed 12:50 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.94 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 12:51 AM UTC · Completed 12:59 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.79 |
Add a note to ADR-0033's Alternative 2 rejection clarifying that GitHub's event suppression applies only to GITHUB_TOKEN pushes, not to GitHub App installation token pushes. The original sentence overstated the suppression scope — the observed scaffold-sync dispatch recursion confirms App tokens do trigger new workflow runs. This is a permitted minor annotation on an Accepted ADR (per docs/contributing/adrs.md), preserving the original text while linking to the corrected documentation in bot-identities.md. Addresses #7086
🔧 Fix agent — iteration 2 (bot-triggered)Addressed the stale-reference finding by adding a correction annotation to ADR-0033's Alternative 2 rejection. The annotation clarifies that GitHub's event suppression applies only to GITHUB_TOKEN pushes (not App installation tokens), cross-references bot-identities.md and platform-nativeness.md, and preserves the original ADR text per immutability rules. Fixed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 1:01 AM UTC · Completed 1:15 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.79 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 1:18 AM UTC · Completed 1:23 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.10 |
Add `## General identity notes` heading before the bold-paragraph notes (`**Shared vendor identity:**`, `**REST vs. GraphQL login format:**`, `**gh pr view --json format:**`) so they are scoped as document-wide conventions rather than falling inside the sync-specific `## Scaffold-sync write path` section. Addresses #7086
🔧 Fix agent — iteration 3 (bot-triggered)Added Fixed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 1:25 AM UTC · Completed 1:39 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.22 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 1:41 AM UTC · Completed 1:48 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.91 |
Reframe the Risks bullet at line 338 to remove the disproven "no review triggered after fix pushes" failure mode, which contradicts the correction note at line 69 (GitHub's event suppression applies only to GITHUB_TOKEN pushes, not App installation tokens). The reframed risk retains the "App identity confusion" header and states the actual remaining failure mode: a single-App setup grants every role the union of all permissions, violating least-privilege (ADR 0007). Strategy change (iteration 4): previous iterations added annotations without modifying existing text. This iteration directly corrects the contradicted text — permitted as a minor factual correction under ADR immutability rules (docs/contributing/adrs.md). Note: pre-commit hooks were not run. pre-commit could not complete (infrastructure failure). ADR linters, link linters, and whitespace checks were run directly and passed. Addresses #7086
🔧 Fix agent — iteration 4 (bot-triggered)Corrected the App identity confusion risk bullet in ADR-0033 to resolve the internal contradiction with the correction note at line 69. The reframed text removes the disproven 'no review triggered' failure mode and states the actual risk (permission over-scoping per ADR 0007). Fixed (1):
Tests: passed
Decision points
Next steps:
|
|
🤖 Review · ❌ Terminated · Started 1:50 AM UTC · Ended 2:09 AM UTC Commit: |
Site previewPreview: https://70de439b-site.fullsend-ai.workers.dev Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 1:50 AM UTC · Completed 2:09 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.72 |
|
🤖 Finished Fix · ✅ Success · Started 2:11 AM UTC · Completed 2:17 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.56 |
…-nativeness cross-reference Promote bold-prefixed paragraphs under "General identity notes" to ### headings for consistency with the "Scaffold-sync write path" section. Update platform-nativeness.md to note that ADR 0033 now contains its own correction note about GitHub event suppression scope. Note: pre-commit hooks were not run. `pre-commit` could not complete (infrastructure failure: HTTP 403 fetching remote hook repos), and the hooks were run directly via local equivalents (trailing-whitespace, end-of-file-fixer, lint-docs-links, lychee link check). Addresses #7086
🔧 Fix agent — iteration 5 (bot-triggered)Addressed both review findings: promoted bold-prefixed paragraphs to ### headings under General identity notes for structural consistency, and updated platform-nativeness.md to note that ADR 0033 now formally contains the suppression scope correction. Fixed (2):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:19 AM UTC · Completed 2:35 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.71 |
| - **Insider — workflow and config modification**: In per-repo mode, `.github/workflows/fullsend.yml` and `.fullsend/` live alongside code. A contributor with write access could modify agent behavior, sandbox policies, or the workflow trigger in a PR. Without CODEOWNERS protection, these changes could be merged by any approver. | ||
| - **`event_payload` size**: Per-org's `dispatch.yml` builds a minimal payload from `$GITHUB_EVENT_PATH` (extracting only `issue`, `pull_request`, and `comment` fields), avoiding the 65KB `workflow_call` input limit. Per-repo's shim forwards `event_action` via `workflow_call` and `reusable-dispatch.yml` reads remaining context from `github.event.*` expressions, following the same pattern. Large PR event payloads are unlikely to be an issue since the shim does not pass the full payload as an input. | ||
| - **App identity confusion**: Users unfamiliar with the fix→review loop requirement may attempt a single-App setup and get silent failures (no review triggered after fix pushes). | ||
| - **App identity confusion**: Users unfamiliar with the multi-App requirement may attempt a single-App setup. App installation token pushes do trigger events regardless of App identity (see [correction note above](#alternative-2-single-github-app-for-all-roles)), so the fix→review loop itself would function, but a single App grants every role the union of all permissions — violating least-privilege ([ADR 0007](0007-per-role-github-apps.md)). |
There was a problem hiding this comment.
[low] adr-immutability
The App identity confusion bullet in the Risks section was substantively rewritten, not merely annotated. The ADR contributing guide (docs/contributing/adrs.md) requires calling out any edits to accepted ADRs in the PR description. The PR body does not mention changes to ADR 0033 or platform-nativeness.md. The edit is factually correct but the disclosure omission violates the guide requirement.
Suggested fix: Add a sentence to the PR description noting the Risks section edit to ADR 0033 (e.g., Also corrects the App identity confusion risk entry in ADR 0033 Risks section, which was based on the now-refuted suppression premise). No ADR text change needed.
|
🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 2:37 AM UTC · Completed 2:37 AM UTC Commit: Effort: high |
1 participant
Summary
Documents the
fullsend-ai-sync[bot]App's write path, which became load-bearing on 2026-08-24 when #6549 addedpush: branches: [main]tonotify-scaffold-syncand the App was granted workflow-write. The three properties documented — ruleset bypass, workflow-write scope, and App-token push recursion — were previously undiscoverable from any file in the repo.Changes
docs/contributing/bot-identities.md: Addedsyncrow to the bot-identities table. Added a "Scaffold-sync write path" section covering the ruleset bypass (bypass_mode: alwaysonmain), workflow-write scope (disambiguated from the coder token statement in release: validate-agents startup failure (caller permissions) + agents gate validates a different tree than tag-agents tags #6512), and App-token push recursion with the observed 2026-08-24 dispatch chain as a concrete example.docs/contributing/ci-workflows.md: Added a "Scaffold-sync dispatch recursion" section noting thatnotify-scaffold-syncfires on every push tomainand sync commits re-trigger it (≥2 dispatch rounds per scaffold-touching merge), with a cross-link to the bot-identities page.Item 4 from the issue (
.github/workflows/notify-scaffold-sync.ymlheader comment) is excluded per maintainer instruction — the coder token cannot push workflow files.Testing
Checklist
!for breaking changes)Closes #6554
Post-script verification
agent/6554-sync-bot-docs)bccd9e815a09ae063447740473df37908efe17ac..HEAD)