fix(#2433): restore data consistency guard in EnsureOrgInMint#63
fix(#2433): restore data consistency guard in EnsureOrgInMint#63guyoron1 wants to merge 170 commits into
Conversation
Introduce --vendor to install vendored binaries, reusable workflows, actions, and agent content. Vendored upstream mirror content is committed under .defaults/ (same layout as runtime sparse checkout); layered installs fetch fullsend-ai/fullsend@v0 into .defaults when the marker file is absent. Reusable workflows use inline workspace preparation and reference infra from ./.defaults/, matching the pre-vendor layered design. Thin callers render local reusable paths when --vendor is set. --fullsend-source pins the source tree for both content and binary cross-compile; --fullsend-binary remains an explicit ELF override. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Write vendor-manifest.yaml on --vendor installs so cleanup and analyze work without a local fullsend checkout. Workflows analyze stays embed-only; vendor layer reports presence, manifest alignment, and optional source alignment via admin analyze --fullsend-source. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate thin-stage caller registry, reuse resolved source root for binary vendoring, reject oversized tar members during extraction, restore workflows scope comment, fix testing-workflows prose, and introduce InstallFiles as the canonical collector return type. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Re-add the full download_test.go suite and append extractSourceTree size limit coverage. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Delete vendored paths atomically via forge.DeleteFiles, reuse resolved source root for cross-compile, preserve extracted file modes, and tighten WouldFix deduplication to exact path matches. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Document intentional breaking change: old flag callers should use --vendor; only known usage was e2e, already updated in this branch. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Document VendorBinaryLayer legacy naming, restore Uninstall/Analyze comments, and use Title Case for stale-cleanup progress messages. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Batch binary, content, and manifest in one CommitFiles call; validate manifest version on read; trim leading slash in extractSourceTree; wrap DeleteFiles ref PATCH in retryOnTransient. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Use the existing blob mode from the recursive tree and set type blob so deletion entries match GitHub Trees API expectations. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Guard against regressions in delete-entry construction per review. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # internal/forge/fake.go # internal/forge/forge.go Signed-off-by: Barak Korren <bkorren@redhat.com>
Encode CommitFiles tree entries as base64 to preserve ELF binaries, add tar extract containment check, consolidate stale cleanup with a manifest/binary quick-check, and deduplicate cleanup between CLI and layer. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # action.yml # docs/guides/dev/testing-workflows.md Signed-off-by: Barak Korren <bkorren@redhat.com>
Clarify removed distribution-mode artifacts, drop e2e vendor line, and document action.yml source-build fallback. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Empty commit to re-dispatch review; prior synchronize dispatch was cancelled. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Keep enumerateVendoredPaths aligned with CollectVendoredAssets after main added the composite action (fullsend-ai#2106); fixes CI parity test. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…t dispatch GitHub Actions may return 422 when repo-maintenance is dispatched immediately after a separate vendor CommitFiles on a fresh .fullsend repo. Merge scaffold and vendored assets into one atomic commit and retry dispatch on indexing lag. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…nance Poll GitHub until repo-maintenance.yml is active before dispatch, re-touch config.yaml after scaffold so the push trigger can run enrollment when dispatch is still rejected, and fall back to awaiting a push-triggered run. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…nary Tree entries with encoding:base64 stored base64 text literally on GitHub, corrupting YAML workflows and vendor-manifest.yaml. Restore UTF-8 inline content for text and upload binary via the Git Blob API instead. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Design for a new `prerequisites` triage action that replaces `blocked`. The agent can now express both existing blockers and new issues that need to be created upstream before progress can happen. Includes allowlist configuration for cross-repo issue creation and a degraded path when targets are not authorized. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…nd-ai#401) Seven-task plan covering config structs, JSON schema, agent prompt, post-script, user docs, and caller updates. TDD approach with exact file paths and code blocks. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Add CreateIssuesConfig and AllowTargets types to both OrgConfig and PerRepoConfig. NewOrgConfig populates defaults with the org and fullsend-ai/fullsend. NewPerRepoConfig populates with the target repo and fullsend-ai/fullsend. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…ues (fullsend-ai#401) Pass org name and target repo to config constructors so create_issues defaults are populated at install time. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…pt (fullsend-ai#401) The triage agent can now recommend creating upstream issues via the prerequisites action's create array, in addition to referencing existing blockers. Adds hard constraint against emitting sufficient when prerequisites exist. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…d-ai#401) Update triage agent docs to explain the new prerequisites action and the create_issues.allow_targets configuration surface. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…#401) Replace the blocked handler with prerequisites. The post-script reads the create_issues allowlist from config.yaml, creates permitted upstream issues via gh, and includes collapsed draft bodies for disallowed or failed creates so humans can file them manually. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…ullsend-ai#401) The agent prompt referenced a nonexistent `prerequisites` label when checking for prior blockers — the post-script actually applies the `blocked` label. Also removed unused SOURCE_ORG variable from post-triage.sh. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…end-ai#401) Replace the four blocked-action test cases with five prerequisites-action test cases that exercise the new schema (existing[], create[], allowlist validation). Set up GITHUB_WORKSPACE with a config.yaml fixture and add a mock gh issue-create handler that returns a fake URL. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…-ai#1706) Validate agent-recommended labels against a control-label guard list, check label existence, append reason to review body, and apply mutations via the GitHub labels API after posting. Mirrors the label_actions processing in post-triage.sh. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…1706) Add issue-labels to the harness skills list and agent definition. Document when and how to invoke the skill during review, and add label_actions to the pipeline mode output docs. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Add issue-labels skill section to review agent docs, update the built-in skills table, and align triage docs example with the generalized skill language. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
) - Revert triage.md example wording to stay issue-specific (triage agent doesn't process PRs) - Add trap for LABEL_MODIFIED_RESULT temp file cleanup in post-review.sh - Add integration tests for label_actions processing in post-review-test.sh (10 cases covering: applied, control-label refused, nonexistent skipped, invalid chars refused, remove, multiple add, all-refused no body append, absent, request-changes) Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
The test helpers intentionally export variables inside subshells for isolation. Shellcheck flags these as accidental — disable the warnings. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Sanitize LA_LABEL and LA_ACTION after jq -r extraction by stripping newlines, carriage returns, and GHA workflow command delimiters (::). This prevents command injection via crafted label names that embed GHA workflow commands after a JSON-decoded newline. Replace per-tempfile trap EXIT handlers with a CLEANUP_FILES array and a single composed trap. Bash traps don't compose — the second trap was silently replacing the first, leaking MODIFIED_RESULT when both protected-path downgrade and label_actions processing fired. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…-agent-labels feat: review agent applies contextual labels via issue-labels skill (fullsend-ai#1706)
…script-diagnostic-errors fix(fullsend-ai#2393): add diagnostic stderr output to post-script failure paths
…-await-and-enqueue feat(merge-queue): add await-and-enqueue script
PR fullsend-ai#2331 removed the defense-in-depth cross-check that prevented silent clobbering of ALLOWED_ORGS on stale reads because it relied on org-scoped ROLE_APP_IDS keys which no longer exist in the role-only model. No replacement was added. Restore the guard adapted for the role-only model: if ALLOWED_ORGS is empty but mintcore.RoleOnlyAppIDs() finds role-only entries in ROLE_APP_IDS, the mint has been bootstrapped and empty ALLOWED_ORGS indicates env var data loss. Return an error instead of silently writing only the new org (which would unenroll all existing orgs). First-enrollment (both empty) and legacy-only keys (no role-only entries) proceed normally. Updated TestEnsureOrgInMint_DerivesAllowedRolesWhenEmpty to set ALLOWED_ORGS to a non-empty value, since the test's purpose is verifying ALLOWED_ROLES derivation, not the empty-ALLOWED_ORGS path which is now correctly guarded. Note: pre-commit could not run in sandbox (shellcheck download blocked by network policy). Post-script runs it authoritatively. Closes fullsend-ai#2433
|
/fs-qf |
|
🤖 Finished Review · ✅ Success · Started 10:56 AM UTC · Completed 11:11 AM UTC |
ReviewFindingsHigh
Medium
Low / Info
Previous runReviewReason: stale-head The review agent reviewed commit Previous run (2)ReviewReason: stale-head The review agent reviewed commit |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 11:14 AM UTC · Completed 11:52 AM UTC |
Generated Software Test Description (STD) with: - 16 scenarios (13 Functional, 3 End-to-End) - Comprehensive STD YAML (v2.1-enhanced) - 6 Go test stub files covering all 13 Functional scenarios - Python stubs skipped (tier2_tests disabled) Co-Authored-By: QualityFlow[bot] <qualityflow[bot]@users.noreply.github.com>
Replaces intermediate pipeline artifacts with organized test files. Total: 6 test files → qf-tests/fullsend-aiGH-2433/ Jira: fullsend-aiGH-2433 [skip ci]
QualityFlow Pipeline Summary
Test Output
Issue: GH-2433 Generated by QualityFlow |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 11:55 AM UTC · Completed 12:16 PM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| // DeleteFiles atomically removes multiple paths in a single commit via the | ||
| // Git Trees API. Missing paths are skipped. Returns the number of paths | ||
| // removed, or (0, nil) when none of the paths exist. | ||
| DeleteFiles(ctx context.Context, owner, repo, message string, paths []string) (deleted int, err error) |
There was a problem hiding this comment.
[high] breaking-api
New required method DeleteFiles added to forge.Client interface. All in-repo implementations must implement this method.
Suggested fix: Implement DeleteFiles in all forge.Client implementations (github, fake).
| GetOrgVariableRepos(ctx context.Context, org, name string) ([]int64, error) | ||
|
|
||
| // CI/Workflow operations | ||
| GetWorkflow(ctx context.Context, owner, repo, workflowFile string) (*Workflow, error) |
There was a problem hiding this comment.
[high] breaking-api
New required method GetWorkflow added to forge.Client interface. All in-repo implementations must be updated.
Suggested fix: Implement GetWorkflow in all forge.Client implementations.
| default: "" | ||
| status-token: | ||
| description: Token for status comments (defaults to GH_TOKEN env var). | ||
| mint-url: |
There was a problem hiding this comment.
[high] breaking-cli
status-token input replaced with mint-url. External workflows using status-token will silently have it ignored. Breaking change to the GitHub Action's public interface.
Suggested fix: Update all consuming workflows to use mint-url instead of status-token.
| "action": { | ||
| "type": "string", | ||
| "enum": ["insufficient", "duplicate", "sufficient", "blocked", "question"] | ||
| "enum": ["insufficient", "duplicate", "sufficient", "prerequisites", "question"] |
There was a problem hiding this comment.
[high] breaking-schema
action enum value 'blocked' replaced with 'prerequisites'. blocked_by field replaced with prerequisites object. Agent output using 'blocked' will fail validation.
Suggested fix: Update triage agent prompts and post-scripts to use action: 'prerequisites'.
| @@ -24,19 +25,6 @@ on: | |||
| - 'scripts/check-e2e-authorization.sh' | |||
| pull_request_target: | |||
There was a problem hiding this comment.
[medium] workflow-permission
pull_request_target paths filter removed. Every PR now triggers e2e. Runtime check fails open (relevant=true on API failure).
Suggested fix: Consider keeping a broad paths filter as defense-in-depth.
| // AllowTargets defines which orgs and repos agents may create issues in. | ||
| type AllowTargets struct { | ||
| Orgs []string `yaml:"orgs,omitempty"` | ||
| Repos []string `yaml:"repos,omitempty"` |
There was a problem hiding this comment.
[medium] breaking-api
New create_issues field with validation. Optional but malformed entries will fail Validate().
| // translate Line==0 into the appropriate API representation (e.g., | ||
| // GitHub's subject_type: "file"). | ||
| type ReviewComment struct { | ||
| Path string // relative file path in the repository |
There was a problem hiding this comment.
[medium] breaking-api
ReviewComment.Line==0 semantics changed to mean file-level comment instead of error case.
| @@ -58,7 +60,7 @@ fullsend | |||
| │ ├── --run-url <url> # CI/CD run URL for status comments | |||
| │ ├── --status-repo <owner/repo> # Repository for status comments | |||
There was a problem hiding this comment.
[medium] stale-doc
References deprecated --status-token flag replaced with --mint-url.
Suggested fix: Update to --mint-url and add --role parameter.
| | `--status-repo` | Repository (`owner/repo`) to post status comments on | | ||
| | `--status-number` | Issue or PR number for status comments | | ||
| | `--status-token` | Token for posting comments (defaults to `GH_TOKEN`) | | ||
| | `--mint-url` | Mint service URL for on-demand status comment tokens (default: `$FULLSEND_MINT_URL`) | |
There was a problem hiding this comment.
[medium] stale-doc
References deprecated --status-token flag.
Suggested fix: Replace with --mint-url and --role.
| @@ -133,7 +136,8 @@ Both per-org and per-repo modes share the same core pipeline. The code follows t | |||
| │ │ a. Discover mint --mint-url / --mint-project / default │ │ | |||
There was a problem hiding this comment.
[medium] stale-doc
ROLE_APP_IDS discovery logic doesn't reflect role-only model.
Suggested fix: Update to clarify role-only model.
8 participants
Mirror of upstream fullsend-ai#2436
Restore the defense-in-depth cross-check that prevents silent clobbering of ALLOWED_ORGS on stale reads, adapted for the role-only model.