fix(#1230): run OutputPipeline on post-review before posting to forge#69
Open
guyoron1 wants to merge 195 commits into
Open
fix(#1230): run OutputPipeline on post-review before posting to forge#69guyoron1 wants to merge 195 commits into
guyoron1 wants to merge 195 commits into
Conversation
Add a new subsection under "CI pipeline for agent configurations" elaborating on Step 1 (static analysis). Covers component-level checks (structural integrity, security patterns, token budget), setup-level analysis (redundancy detection, dependency validation, token budget distribution, trigger overlap, dimension scoring), and optional LLM-based rubric scoring. Presents similarity techniques as options (TF-IDF, embeddings, LLM-based) rather than prescribing a single approach. Adds three open questions on thresholds, lint rule universality, and token budgets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
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>
|
/fs-review |
STD generation complete: - STD YAML: GH-1230_test_description.yaml (26 scenarios) - Go test stubs: 7 files, 26 t.Run() blocks with PSE comments - Pipeline state: std phase completed
|
🤖 Finished Review · ✅ Success · Started 3:35 PM UTC · Completed 3:50 PM UTC |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 3:53 PM UTC · Completed 4:10 PM UTC |
Generated 26 Go tests (7 files) for sanitizeReviewResult from STD YAML. All tests verified passing against internal/cli package. Co-Authored-By: QualityFlow[bot] <qualityflow[bot]@users.noreply.github.com>
Replaces intermediate pipeline artifacts with organized test files. Total: 7 test files → qf-tests/fullsend-aiGH-1230/ Jira: fullsend-aiGH-1230 [skip ci]
QualityFlow Pipeline Summary
Test Output
Issue: GH-69 Generated by QualityFlow |
|
/fs-review |
Owner
Author
|
/fs-qf |
|
🤖 Finished Review · ✅ Success · Started 4:07 AM UTC · Completed 4:25 AM UTC |
Refined STP from NEEDS_REVISION to APPROVED_WITH_FINDINGS. Fixed 3 critical, 8 major, and 3 minor findings in 1 iteration. Coverage rate improved from 60% to 100%, weighted score from 52 to 96. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 4:28 AM UTC · Completed 4:45 AM UTC |
STD generation: 12 scenarios (11 unit, 1 functional) - STD YAML (v2.1-enhanced internal format) - 6 Go test stub files with PSE comments (12 t.Run blocks) - Pipeline state tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 4:48 AM UTC · Completed 5:06 AM UTC |
Removes intermediate pipeline artifacts (STP, STD, reviews). Test files (5) are co-located in source tree with qf_ prefix. Jira: GH-69 [skip ci]
|
/fs-review |
Co-located tests (qf_* prefix) are now in source package directories. The qf-tests/ directory contained non-compiling tests from the old pipeline.
|
🤖 Finished Review · ✅ Success · Started 8:32 AM UTC · Completed 9:13 AM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| @@ -506,6 +534,43 @@ func minimizeStaleReviews(ctx context.Context, client forge.Client, user string, | |||
| printer.StepDone("Stale reviews minimized") | |||
There was a problem hiding this comment.
[medium] incomplete-sanitization
sanitizeReviewResult sanitizes Body, Description, and Remediation fields but does not sanitize Category, File, or Severity fields of findings. These unsanitized fields are interpolated into formatFindingComment and posted as GitHub review comments. While these fields are structured data (less likely to contain secrets), this is a defense-in-depth gap in the stated security fix. See also: [logic-error] finding at this location.
Suggested fix: Either sanitize Category field as well, or run the output pipeline on the final formatted comment body in formatFindingComment rather than on individual fields before composition.
| @@ -506,6 +534,43 @@ func minimizeStaleReviews(ctx context.Context, client forge.Client, user string, | |||
| printer.StepDone("Stale reviews minimized") | |||
There was a problem hiding this comment.
[medium] logic-error
sanitizeReviewResult warning message says 'Redacted %d secret(s) in review body' using len(result.Findings), but OutputPipeline chains UnicodeNormalizer then SecretRedactor — result.Findings aggregates findings from both scanners, overcounting secrets. See also: [incomplete-sanitization] finding at this location.
Suggested fix: Filter result.Findings to count only secret_redactor findings, or change message to 'Sanitized %d finding(s) in review body'.
| default: "" | ||
| status-token: | ||
| description: Token for status comments (defaults to GH_TOKEN env var). | ||
| mint-url: |
There was a problem hiding this comment.
[medium] breaking-change
The status-token input is removed and replaced with mint-url. Enrolled repositories using this action with status-token will get an 'unexpected input' warning from GitHub Actions. The finalize orphaned status comment step now requires mint-url to be non-empty, so status reconciliation silently stops working for users who haven't migrated.
| source "${SCRIPT_DIR}/openshell-version.sh" | ||
|
|
||
| echo "Installing OpenShell ${OPENSHELL_VERSION} (${OPENSHELL_SHA})" | ||
| curl -LsSf "https://raw.githubusercontent.com/NVIDIA/OpenShell/${OPENSHELL_SHA}/install.sh" \ |
There was a problem hiding this comment.
[low] supply-chain
curl | sh pattern for installing OpenShell. While the URL is pinned to a specific commit SHA, the downloaded content's integrity is not verified. This matches the existing pattern in action.yml but represents a defense-in-depth gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of upstream fullsend-ai#2444
Security fix: calls
security.OutputPipeline().Scan()on review body and finding fields before posting to the GitHub API — preventing credential/PII leaks in public PR comments.