perf(#2351): batch path-existence checks via Git Trees API#72
Open
guyoron1 wants to merge 52 commits into
Open
perf(#2351): batch path-existence checks via Git Trees API#72guyoron1 wants to merge 52 commits into
guyoron1 wants to merge 52 commits into
Conversation
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>
…ullsend-ai#401) Replace blocked-action test cases with prerequisites-action equivalents and update the expected property list (blocked_by → prerequisites). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…d-ai#401) - Replace stale blocked-* schema validation tests with prerequisites equivalents (missing field, both arrays empty, malformed URL) - Fix validateCreateIssues to reject malformed repo formats like "/", "/repo", "owner/" - Align triage.md section 2c terminology from "blocker" to "prerequisite" consistently - Update bugfix-workflow.md and architecture.md to document upstream issue creation capability - Emit ::warning:: when yq is unavailable so silent degradation of cross-repo issue creation is diagnosable Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Adds a skill that summarizes recent E2E Tests workflow runs on main, presents them in a table with clickable links, and diagnoses failures by grepping failed step logs for signal lines. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
The markdown link linter was parsing `[run-id](url)` as a real file reference. Wrapping it in backticks marks it as a code example. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Move list-runs.sh to scripts/ subdirectory to match convention - Add bash command prefix to allowed-tools declaration - Clarify status vs conclusion field handling for in-progress runs - Use case-insensitive grep to catch Timeout/timeout variants - Tighten frontmatter description Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
When multiple runners exhaust the GraphQL rate limit simultaneously, they all sleep until the same reset timestamp and wake up together. The existing slot jitter (250-750ms) is too narrow to desynchronize them, causing collisions that surface as "unknown owner type" errors from gh project view. Add a post-reset spread of up to 60s (configurable via GITHUB_CSMA_SPREAD_MAX_SEC) so runners fan out over a wide window after waking from a rate-limit sleep. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…0045 Phase 3 PR 2) Add DiscoverRemoteAgents() that discovers agent identity (role, slug) from harness files in a remote config repo via the forge API. Extract parseRaw() from LoadRaw() so callers with raw YAML bytes (e.g. from forge API responses) can parse without filesystem I/O. Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
…ss schema Add end-to-end integration tests covering the full Phase 2 pipeline (PR 6 of 6 in the ADR-0045 forge-portable harness schema adoption): - LoadWithBase wrapper→scaffold merge with field inheritance and override - All scaffold templates forge resolution (pre/post scripts, runner_env) - Backward compatibility via Load() (no forge platform) - DiscoverAgents scaffold directory scanning with correct role/slug pairs - HarnessContentHash integrity verification against embedded content - LoadRaw generated wrapper format validation - ResolveForge scaffold runner_env merge with per-template key assertions Resolves fullsend-ai#2328 Signed-off-by: Greg Allen <greg@fullsend.ai> Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
…nings (ADR-0045 Phase 3 PR 1) Part of fullsend-ai#2326 Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
Status comments on PRs/issues get stuck in "Started" when the pre-minted agent token expires before PostCompletion runs. Instead of relying on a static token, have the fullsend binary mint its own fresh short-lived token via mintclient.MintToken() before each status comment API call. Key changes: - Add ClientFactory pattern to statuscomment.Notifier so each API operation gets a freshly minted forge.Client - Add --mint-url flag to fullsend run and reconcile-status commands - Add mint-url input to action.yml and all reusable workflows - Deprecate --status-token (run) and --token (reconcile-status) with runtime warnings; hidden from help output - Deprecate status-token input in action.yml; mask unconditionally - Validate token format before ::add-mask:: to prevent workflow command injection - Move refreshClient below commentEnabled guard in PostCompletion - Make refreshClient failure in cleanup path fail-open (warning) - Add "code" -> "coder" role alias for agent name resolution Closes fullsend-ai#2130 Signed-off-by: Greg Allen <gallen@redhat.com> Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
…window fix: widen CSMA post-reset jitter to prevent thundering herd
The previous backtick-escaping attempt (7c40a70) did not prevent lychee from resolving `url` as a relative file path. Remove the markdown link syntax entirely so the link checker has nothing to chase. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…kill feat(skills): add e2e-health skill
…ter_rate_limit PR fullsend-ai#2304 added post-reset spread to github_csma_sense to prevent thundering herd when runners wake after a rate-limit reset. The structurally parallel _github_csma_sleep_after_rate_limit function was missing the same treatment — multiple runners hitting a 429 would all wake at the same reset timestamp and fire simultaneously. Extract the spread logic into a shared _github_csma_post_reset_spread helper and call it from both github_csma_sense (replacing the inline code) and _github_csma_sleep_after_rate_limit (added after the backoff sleep). Both paths now use GITHUB_CSMA_SPREAD_MAX_SEC to stagger runner wake times. Note: pre-commit and make lint could not run due to shellcheck-py network restriction in sandbox. Scaffold Go tests pass. Closes fullsend-ai#2343
…spread-rate-limit fix(fullsend-ai#2343): add post-reset spread to _github_csma_sleep_after_rate_limit
The NewOrgConfig call gained a 6th parameter (org string) on this branch. Main didn't have it yet, causing a conflict. Keep the 6-parameter version to match the current function signature. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…atus-token fix(fullsend-ai#2130): mint fresh tokens for status comments
test: Phase 2 integration tests for ADR-0045 forge-portable harness
…-decompose-issues feat(triage): add prerequisites action for upstream issue creation (fullsend-ai#401)
|
🤖 Finished Review · ✅ Success · Started 3:16 PM UTC · Completed 3:35 PM UTC |
Verdict: APPROVED_WITH_FINDINGS (score: 94/100) - 0 critical, 1 major, 3 minor findings - Major: Performance Testing misclassified (functional guard test, not perf test)
Refined STP from APPROVED_WITH_FINDINGS to APPROVED in 1 iteration. Resolved 4 findings (1 major, 3 minor): - Unchecked Performance Testing (architectural validation, not perf testing) - Simplified Testing Tools section to remove standard tool listings - Updated Owning SIG to match component/install label - Added Vendor Analyze prefix to STP title for cross-artifact consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 3:43 PM UTC · Completed 3:56 PM UTC |
|
/fs-review |
…iew findings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replaces intermediate pipeline artifacts with organized test files. Total: 5 test files → qf-tests/fullsend-aiGH-2351/ Jira: fullsend-aiGH-2351 [skip ci]
QualityFlow Pipeline Summary
Test Output
Issue: GH-72 Generated by QualityFlow |
Owner
Author
|
/fs-qf |
|
🤖 Finished Review · ✅ Success · Started 6:59 AM UTC · Completed 7:19 AM UTC |
Refined STP from APPROVED_WITH_FINDINGS to APPROVED in 1 iteration. Resolved 7 findings (4 major, 3 minor): - Fixed personal fork links to upstream fullsend-ai/fullsend - Reclassified internal deps from cross-team to internal module - Reclassified CI workflow validation from E2E to Functional - Clarified Performance Testing guard test justification - Simplified Testing Tools section - Added QE kickoff timing - Aligned Dependencies risk with strategy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 7:22 AM UTC · Completed 7:42 AM UTC |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| @@ -393,10 +406,12 @@ runs: | |||
| "${STATUS_FLAGS[@]+"${STATUS_FLAGS[@]}"}" | |||
|
|
|||
| - name: Finalize orphaned status comment | |||
There was a problem hiding this comment.
[medium] API contract
Finalize orphaned status comment step condition tightened to require mint-url or status-token. The old GITHUB_TOKEN fallback was removed. Callers relying on GITHUB_TOKEN will silently skip orphan reconciliation.
Suggested fix: If backward compatibility with GITHUB_TOKEN fallback is intended, keep the step condition as it was and emit a warning if neither mint-url nor status-token is available.
| if err != nil { | ||
| return fmt.Errorf("minting status token: %w", err) | ||
| } | ||
| if os.Getenv("GITHUB_ACTIONS") == "true" && mintTokenPattern.MatchString(result.Token) { |
There was a problem hiding this comment.
[low] secret-exposure
mintTokenPattern may not match all token formats, causing masking to silently skip for non-standard tokens.
| @@ -52,15 +53,41 @@ finalized, this is a no-op.`, | |||
| } | |||
There was a problem hiding this comment.
[low] consumer-completeness
Passes just repo name (not owner/repo) to mintclient.MintToken Repos field. Verify the expected format matches.
| if err := c.retryOnTransient(ctx, "get branch ref", func() error { | ||
| refResp, refErr := c.get(ctx, fmt.Sprintf("/repos/%s/%s/git/ref/heads/%s", owner, repo, repoInfo.DefaultBranch)) | ||
| if refErr != nil { | ||
| return fmt.Errorf("get branch ref: %w", refErr) |
There was a problem hiding this comment.
[low] error-handling
ListRepositoryFiles only wraps branch ref lookup in retryOnTransient. Commit and tree fetches lack retry, inconsistent with other methods in the file.
| CREATED_URLS="" | ||
| FAILED_CREATES="" | ||
|
|
||
| for i in $(seq 0 $((CREATE_COUNT - 1))); do |
There was a problem hiding this comment.
[low] edge-case
gh issue create --body may have shell escaping issues with special characters in agent-controlled content. Consider using --body-file.
| if mintURL != "" { | ||
| if role == "" { | ||
| return fmt.Errorf("--role is required when using --mint-url") | ||
| } |
There was a problem hiding this comment.
[low] race-condition
Package-level newForgeClient overridden in tests without synchronization. Currently safe (no t.Parallel) but fragile pattern.
Generated 51 test cases across 9 suites from STD YAML: - scaffold: ComparePathPresence batch path checking (6 tests) - forge: FakeClient ListRepositoryFiles (2 tests) - statuscomment: ClientFactory pattern (10 tests) - harness: Lint diagnostics + DiscoverRemoteAgents (18 tests) - config: CreateIssues validation (3 tests) - cli: reconcile-status + run mint-url integration (11 tests) - forge/github: Git Trees API truncation handling (1 test) All tests compile and pass.
Removes intermediate pipeline artifacts (STP, STD, reviews). Test files (9) are co-located in source tree with qf_ prefix. Jira: GH-72 [skip ci]
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 · ❌ Failure · Started 8:37 AM UTC · Completed 9:01 AM UTC |
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
3 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#2360
Performance optimization: batches path-existence checks using the Git Trees API instead of individual requests.