feat(dispatch): synchronous workflow_call event dispatch (ADR 41) - #14
Closed
ifireball wants to merge 878 commits into
Closed
feat(dispatch): synchronous workflow_call event dispatch (ADR 41)#14ifireball wants to merge 878 commits into
ifireball wants to merge 878 commits into
Conversation
…-slugs-filter fix: filter config knownSlugs by requested app-set prefix
…bber ProvisionWIF() overwrote the WIF provider's attributeCondition with only the installing org, removing all other orgs from the condition. This caused STS 400 errors for any org not included in the latest install run. Add the same merge logic that Provision() already has: read the existing provider's condition, parse out its orgs, union with the new orgs, then write the merged condition back. Signed-off-by: Wayne Sun <gsun@redhat.com>
…ror logging Add double-hyphen rejection to org name validation, normalize parsed org names to lowercase for consistent dedup, and log warnings when existing WIF provider lookup fails instead of silently proceeding. Apply the same error-logging improvement to the Provision() merge path. Signed-off-by: Wayne Sun <gsun@redhat.com>
…ogic provisionSelfManaged() and ProvisionWIF() had identical 30-line blocks for reading the existing WIF provider, merging org lists, and creating the updated provider. Extract into a shared helper so the merge logic lives in one place, preventing future divergence like the clobber bug. Signed-off-by: Wayne Sun <gsun@redhat.com>
…ly, add parseConditionOrgs tests Add owner/repo format and quote validation to the repo-scoped WIF path to prevent panics on malformed input. Move sort.Strings(allOrgs) outside the merge branch so conditions are deterministic on first install. Add table-driven TestParseConditionOrgs covering single org, multi-org, legacy repo format, mixed-case normalization, and empty input. Signed-off-by: Wayne Sun <gsun@redhat.com>
…ordering - Fail fast on GetWIFProvider errors instead of silently degrading to installing-orgs-only, which could re-introduce the condition clobber bug on transient GCP API failures. GetWIFProvider returns (nil, nil) for 404 so a non-nil error always indicates a real failure. - Move repo format and quote validation before GCP API calls in the repo-scoped ProvisionWIF path to avoid wasted API calls and orphaned infrastructure on invalid input. - Document the TOCTOU race on ensureWIFPoolAndProvider matching the existing EnsureOrgInMint warning pattern. - Document parseConditionOrgs assumption that only org names appear as quoted values in WIF conditions. - Add test for repo-scoped validation ordering (rejects invalid repos before any GCP API calls). Signed-off-by: Wayne Sun <gsun@redhat.com>
Use a local providerID variable instead of overwriting the shared config field so callers can safely reuse the Provisioner after ProvisionWIF without seeing a corrupted WIFProvider value. Signed-off-by: Wayne Sun <gsun@redhat.com>
- Replace quote-only denylist with githubRepoSlugPattern allowlist regex for repo-scoped ProvisionWIF input validation (prevents backslash, control chars, spaces from reaching CEL conditions) - Add defensive strings.ToLower in ensureWIFPoolAndProvider so the helper is self-contained regardless of caller normalization - Add TestProvisioner_Provision_GetWIFProviderError_FailsFast to cover the Provision() path behavior change (fail-fast on transient GetWIFProvider errors) - Expand repo validation test cases (backslash, spaces) Signed-off-by: Wayne Sun <gsun@redhat.com>
…nd detect public apps without --public - Apply filterSlugsByAppSet to shared slugs so cross-app-set installs don't shadow each other (e.g. nonflux-triage no longer blocks fullsend-ai-triage). - After recoverCreatedApp + ensureInstalled, resolve AppID from the installation so ROLE_APP_IDS is updated correctly. - Always check GetAppClientID before the manifest flow regardless of --public flag. When a public app exists and --public was not passed, prompt the user to confirm installation instead of silently falling through to create a duplicate via the manifest flow. Signed-off-by: Wayne Sun <gsun@redhat.com>
Address review findings: - Propagate findExistingInstallation errors in recover path instead of silently swallowing them, matching the public-app path's behavior. - Replace misleading "public app" terminology with "existing app" in prompts shown when --public is not set. - Use StepInfo instead of StepDone before user confirmation to avoid showing a success checkmark before the user has decided. - Add tests: recoverCreatedApp AppID resolution through Run(), prompter error propagation, and --public flag skips prompt assertion. Signed-off-by: Wayne Sun <gsun@redhat.com>
githubRepoSlugPattern now rejects leading/trailing dots and hyphens and enforces a 100-char max, matching GitHub's actual repo name rules. Repo-scoped ProvisionWIF lowercases p.cfg.Repo before using it in the CEL attribute condition and IAM principal, preventing silent auth failures when callers pass mixed-case repo names. Signed-off-by: Wayne Sun <gsun@redhat.com>
GitHub allows repo names starting with dots (e.g., .fullsend, .github) but rejects "." and ".." as standalone names. Relax the regex to permit leading dots while adding an explicit check for the two invalid cases. Signed-off-by: Wayne Sun <gsun@redhat.com>
…etection fix: detect public apps without --public flag and filter shared slugs by app-set
Use a local `repo` variable instead of mutating p.cfg.Repo in-place, matching the pattern already used for providerID. Validate the owner component of repo-scoped installs with githubOrgPattern (rejects dots, underscores, >39 chars) instead of the looser githubRepoSlugPattern, since the owner is always a GitHub org/user name. Signed-off-by: Wayne Sun <gsun@redhat.com>
Error messages in repo-scoped ProvisionWIF now show the user's original input casing instead of the lowercased value used for validation. Also reject repo names ending in .git since GitHub strips this suffix from OIDC tokens, making the WIF condition silently unmatchable. Adds test for double-hyphen owner rejection and .git suffix validation. Signed-off-by: Wayne Sun <gsun@redhat.com>
…n-merge fix: merge existing orgs in ProvisionWIF to prevent WIF condition clobber
Replace openshell sandbox upload + RestoreSymlinks with UploadDir, which builds a local tar archive (symlinks preserved by default), uploads it, and extracts it in the sandbox. This also eliminates the separate .git upload step since the tarball includes everything in one shot. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat: add --debug flag for Claude Code debug logging
…inks-after-sandbox-upload fix(sandbox): use tarball upload to preserve symlinks
…e-search-filter feat: add search filter to docs sidebar file tree
Agents fall back to verbose python3 one-liners when jq is absent, wasting ~500-1000 tokens per recovery sequence. Closes fullsend-ai#1126
…andbox-image build(sandbox): add jq to base image
The log line "WARNING: skill X has N injection finding(s)" gave no indication of whether the skill was blocked or uploaded. A high-severity finding looks alarming but does not block the upload; only critical findings do (when fail_mode is closed). This made post-hoc debugging ambiguous. Changes: - Non-critical findings: append "— not blocked (only critical findings block); uploading" so the disposition is unambiguous - Critical findings with fail_mode open: append "— uploading anyway" - After every successful upload: log "Skill X: uploaded to sandbox" so the upload completion is visible in job logs independently of the scan Assisted-by: Claude claude-sonnet-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
sanitizeDownload removed all symlinks unconditionally. This caused lint-broken-symlinks to fail post-run because tracked relative symlinks (e.g. .claude/skills) were stripped from the working tree. Keep symlinks whose relative target resolves inside the repo root. Remove absolute targets and relative targets that escape via ../. Fixes fullsend-ai#1149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SafeDownload now keeps relative in-repo symlinks; update all four doc sites (sandbox.go, run.go, cli-internals.md ×2) to say "dangerous symlinks (absolute or repo-escaping)" instead of "symlinks". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The official GitHub Apps maintained by the fullsend-ai organization use the "fullsend-ai" prefix (fullsend-ai-fullsend, fullsend-ai-coder, etc.). Update the default so installs use the official apps without requiring an explicit --app-set flag. Orgs with existing "fullsend" app sets must now pass --app-set fullsend explicitly. Closes fullsend-ai#1142 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix reuses the coder app per ADR 0031; reusable-fix.yml already mints a coder token. The stage-to-role mapping was missing a fix→coder case, so the role gate checked for a "fix" entry that never appears in default configs, silently blocking the fix agent on all new installations. Closes fullsend-ai#1186 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TestInstallCmd_Flags and TestUninstallCmd_Flags still asserted the old "fullsend" default value for --app-set after the constant was changed to "fullsend-ai". Signed-off-by: Wayne Sun <gsun@redhat.com>
filepath.Clean is textual: "sub/link/../../etc/passwd" resolves to inside the repo, but if link -> ../real the filesystem follows it to /etc/passwd. EvalSymlinks follows intermediate symlink components, so the resolved path correctly lands outside the repo and is removed. Fall back to remove on any EvalSymlinks error (dangling/looping link). Adds TestSanitizeDownload_RemovesSymlinkChainEscape to verify the fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Closes fullsend-ai#1188. Covers public vs. private repo differences, agent safety defaults, AGENTS.md examples for preventing sensitive content reproduction, guardrail testing procedures, and deployment guidance based on data sensitivity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…on-integrity-hash feat: add URL detection and integrity hash parsing to harness package
…-logging-v2 feat: add fetch audit logging for remote resource tracking
…e-length fix(ci): raise gitlint title-max-length from 72 to 100
- Separate --vendor-fullsend-binary from enrollment flags in docs; it uploads a dev binary to the config repo, not enrollment-related - Add --skip-mint-check and --vendor-fullsend-binary to the flags table in installation.md - Clarify per-org-only flag descriptions in ADR 0033 Signed-off-by: Wayne Sun <gsun@redhat.com>
The flag was restricted to per-org mode only, but vendoring a development binary is useful in per-repo installs too. Parameterize VendorBinary, VendorBinaryLayer, and vendorFullsendBinary to accept an explicit repo target instead of hardcoding forge.ConfigRepoName. Signed-off-by: Wayne Sun <gsun@redhat.com>
Update VendorBinary call in e2e/admin/admin_test.go to pass owner and repo as separate arguments, matching the signature change from this PR. Addresses review feedback on fullsend-ai#1011 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
Per-repo mode was uploading the vendored binary to bin/fullsend at the repo root, polluting the target repo with a top-level bin/ directory. Now per-repo uploads to .fullsend/bin/fullsend (matching per-repo config conventions), while per-org continues to use bin/fullsend in the .fullsend config repo. Also adds stale binary cleanup for per-repo mode — reinstalling without --vendor-fullsend-binary now removes any previously vendored binary, matching the cleanup behavior that VendorBinaryLayer provides for per-org mode. Updates action.yml to check both .fullsend/bin/fullsend (per-repo) and bin/fullsend (per-org) when detecting a vendored binary. Signed-off-by: Wayne Sun <gsun@redhat.com>
The layer's cleanup and analyze methods hardcoded VendoredBinaryPath (bin/fullsend) instead of selecting the path based on the target repo. Add a binaryPath() helper that returns the correct path for per-org vs per-repo mode, and add per-repo unit tests for cleanup, analyze, and VendorFunc delegation. Signed-off-by: Wayne Sun <gsun@redhat.com>
- Make --vendor-fullsend-binary flag and doc description mode-neutral (removes path that was only correct for per-repo) - Update VendorBinaryLayer type and Uninstall comments for dual-mode - Add cross-reference comment in per-repo cleanup noting the parallel with VendorBinaryLayer.Install - Standardize StepStart/StepFail/StepDone to lowercase in per-repo cleanup to match the layer's style - Fix e2e test log to use VendoredBinaryPath constant Signed-off-by: Wayne Sun <gsun@redhat.com>
Phase 5 diagram, mode differences table, and layer stack section described vendor binary as per-org only. Updated to show per-repo mode also supports optional binary vendoring via runPerRepoInstall(). Signed-off-by: Wayne Sun <gsun@redhat.com>
Add SSRF-hardened HTTP fetcher for remote resource retrieval with domain allowlist, internal IP rejection, DNS rebinding protection, size limiting, port restriction, and no-redirect policy. Extract shared IP classification logic to internal/netutil so both internal/fetch and internal/security use a single implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
…ed-fetcher-v2 feat: add internal/fetch package with SSRF-protected HTTP client
…y-flag feat: support --vendor-fullsend-binary in per-repo mode
- FakeClient GetRepo/DeleteRepo: remove Name-only fallback so multi-org lock tests match by FullName only (prevents cross-org collisions) - Add tryReclaimStaleLock to the polling loop so locks that age past staleLockTimeout during polling are reclaimed - Protect main branch from cancel-in-progress to avoid orphaned locks and ensure full e2e coverage on merge commits - Update ADR 0040 to reflect current values: 10-min lock timeout, 15-min stale threshold, 30s poll interval, pool shuffling, and round-robin polling with stale reclaim Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Signed-off-by: Ralph Bean <rbean@redhat.com> # Conflicts: # e2e/admin/admin_test.go
…l-orgs feat(e2e): parallel test runs via org pool
…edup fix(fullsend-ai#1467): add dedup research step to retro agent before proposing
Replace dispatch.yml gh workflow run fan-out with workflow_call for UI traceability, and drop ADR 26 dynamic agent discovery without adding a compile-time sync substitute. Discovery may be revisited after ADR 38. Signed-off-by: Barak Korren <bkorren@redhat.com>
…hronous-workflow-call docs(adr): synchronous workflow_call for event-driven agent dispatch
Create ADR 0040 documenting the decision to namespace all fullsend slash commands with the /fs- prefix. This decision was previously captured only as a one-liner in the glossary, causing two separate issues (fullsend-ai#1473, fullsend-ai#1477) where agents proposed changes that contradicted the undocumented choice. The ADR records context (multiple AI tools claim bare slash command verbs), the decision (/fs- prefix for all commands), rejected alternatives (bare commands, /fullsend- prefix), and consequences. Also adds a note to ADR 0002 clarifying that its bare command examples (/triage, /implement, /review) are illustrative and superseded by the /fs- prefix convention. Note: make lint could not run (Go toolchain download permission denied in sandbox). This is a docs-only change with no Go code affected. Closes fullsend-ai#1544 Signed-off-by: fullsend-code <fullsend-code@users.noreply.github.com>
ADR 0040 (org-pool-for-parallel-e2e-tests) and ADR 0041 already exist on main. Renumber the /fs- prefix ADR to 0042 to avoid ambiguity. Updates the filename, title, heading, and cross-reference in ADR 0002. Addresses review feedback on fullsend-ai#1549 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
…s-prefix-convention docs(fullsend-ai#1544): add ADR 0040 for /fs- slash command prefix
Add a stdlib Python script and portable slash command that builds a merged markdown/JSON RICE table from the org GitHub Project board (same fields as post-prioritize.sh): top unassigned backlog issues plus the caller's assigned work. Resolve FULLSEND_PROJECT_NUMBER via org variable with friendly fallbacks, skip items with open PRs or blockedBy links, and wire unit tests into make script-test. Introduce a canonical commands/ tree with .cursor/commands and .claude/commands symlinks mirroring the skills layout. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
feat(skills): add /topissues slash command for RICE backlog
Replace per-org dispatch marker scan and gh workflow run with static workflow_call jobs to upstream reusable workflows. Remove thin stage workflows from scaffold; add workflow_call to prioritize.yml. Port per-stage concurrency from removed thin callers into reusable workflows so per-org behavior is preserved and per-repo mode gains cancel-in-progress per stage. Update unit tests and e2e smoke test to poll dispatch.yml completion. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…_call actionlint resolves ./.github/workflows/prioritize.yml from repo root; the file exists only under internal/scaffold after install to .fullsend. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ifireball
force-pushed
the
feat/adr-41-sync-dispatch
branch
from
May 27, 2026 11:08
c6e7948 to
10b23ea
Compare
2 tasks
Owner
Author
|
Superseded by upstream PR: fullsend-ai#1586 (opened against ifireball:feat/adr-41-sync-dispatch → fullsend-ai/main). |
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
5 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.
Summary
dispatch.yml→reusable-*.yml@v0(synchronousworkflow_call), replacing# fullsend-stage:scanning andgh workflow run.triage.yml,code.yml,review.yml,fix.yml,retro.yml) from the scaffold;prioritize.ymlgainsworkflow_callfor dispatch routing.concurrency(cancel-in-progress) from the removed thin callers into upstream reusable workflows — preserves per-org behavior and improves per-repo mode.dispatch.ymlcompletion instead of orphantriage.ymlruns.Migration
fullsend admin install <org>to refresh.fullsendscaffold. No enrolled-repo shim changes.# fullsend-stage:workflowsworkflow_calljob indispatch.yml; remove markers.Test plan
go test ./internal/scaffold/... ./internal/layers/...Made with Cursor