fix(#2247): compare decoded text in shim drift detection#7
Closed
guyoron1 wants to merge 1220 commits into
Closed
Conversation
- Split actions/cache into actions/cache/restore + actions/cache/save so stale cache entries get replaced when the upstream image changes - Include image reference in cache key to avoid cross-image collisions when FULLSEND_SANDBOX_IMAGE is overridden - Add restore-keys prefix matching for partial cache hits - Track digest before/after pull to detect image changes and re-save - Guard podman save with podman image exists to avoid hard failure when pull soft-fails - Add -- before image arg in podman save for consistency with pull - Add error handling on podman load for corrupted cache graceful degradation Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…token-scope-logging chore(mint): log requested and granted token scope
Replace the strict "immutable once accepted" policy with a nuanced one: cross-references, short notes linking to newer ADRs, and typo fixes are welcome on accepted ADRs. Substantial rewrites still require a new superseding ADR. This stops agents from refusing to add navigational links between related ADRs while still preventing old ADRs from becoming evolving design documents (that role belongs to docs/architecture.md). Updates both the ADR template and the writing-adrs skill. 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> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
fullsend-ai/.fullsend now references reusable workflows via @main instead of @v0. This means: - Pre-flight: fullsend-ai runs are now a signal that main is healthy before moving the v0 tag (new step E) - Post-flight: fullsend-ai runs no longer tell us anything about v0 resolution, so step C is replaced with an explanatory note and downstream verification is moved to step D with inline commands Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Rename heading to "Skip fullsend-ai repos" to match action-verb pattern - Add rerun command template and prompt to step D for self-containment Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Add AppID and InstallationID fields to GrantedScope and include them in the "minted:" log line. This helps diagnose cross-org token scope issues by revealing which GitHub App installation was used to create the token. Ref: fullsend-ai#1916 Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…jobs
Relative (./) paths in uses: resolve against the caller's repo, not the
repo that defines the reusable workflow. This breaks per-repo mode where
the caller is an external repo without the reusable-{stage}.yml files.
Restore fully-qualified fullsend-ai/fullsend/...@v0 references and add
a regression test to prevent re-introduction.
Signed-off-by: Wayne Sun <gsun@redhat.com>
…patch-paths fix(workflows): use fully-qualified paths in reusable-dispatch stage jobs
…Phase 2 PR 2) Implements recursive skill dependency resolution for the universal harness access resolver (ADR-0038 Phase 2, PR 2 of 3). Changes: - Add ResolveRelativeURL for RFC 3986 relative URL resolution - Add MaxDepth/MaxResources limits to ResolveOpts with explicit depth parameter threading through resolveTransitiveDeps (no mutable state) - Cycle detection via inProgress DFS stack, diamond dedup via resolved map - Conflicting SHA256 hashes for the same URL are detected and rejected - Skills with dependencies: frontmatter are recursively resolved; policy references are fetched as leaf nodes (runtime semantics deferred) - Rename resolveState.appended to inDeps for semantic clarity - Standardize resolveURL error messages to consistent "%s: " prefix format - Add explicit HTTPS-only scheme check in resolveURL as defense-in-depth for transitive dep URLs (fetch.FetchURL also enforces this) - Wrap ParseFrontmatter errors with parentURL context in resolveTransitiveDeps - 14 new transitive resolution tests: chain, diamond, cycle, depth limit, resource limit, allowlist, hash mismatch, relative URL, conflicting hashes, policy leaf, MaxDepth=0 disabled, default, overlap dedup, non-HTTPS scheme - 4 new relurl tests: invalid percent-encoding, empty relRef, empty parentURL, parent URL with no path component - Update stale pseudocode in design docs to reflect Phase 2 implementation Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
…ork-check-token fix(fullsend-ai#1532): use github.token for fork-check in dispatch
Allow overriding the number of prioritize jobs dispatched when triggering the scheduler manually via workflow_dispatch. Falls back to the PRIORITIZE_WIP_LIMIT repo variable, then to 5. 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>
…playwright-mcp chore: add .playwright-mcp to .gitignore
Update AGENTS.md, CONTRIBUTING.md, docs/architecture.md, skills/renumber-adr/SKILL.md, and skills/writing-adrs/SKILL.md to distinguish minor annotations from substantial rewrites on accepted ADRs, consistent with the updated template and writing-adrs skill. Fix parallel structure in writing-adrs mistake table entries. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…w template The shim-workflow-call.yaml template was missing the `---` YAML document start marker, causing yamllint failures in enrolled repos that enforce the `document-start` rule. Prepend `---` as the first line of the template. Also add a test assertion in TestShimWorkflowCallTemplateContent to verify the marker is present. Note: Go tests and pre-commit could not run in the sandbox due to Go version mismatch (sandbox has 1.24.13, repo requires 1.26.0). Manual verification of Go tests is required. Closes fullsend-ai#1946
Prepend `---` to all 25 remaining scaffold YAML files (workflows, actions, templates, policies, harness configs) that were missing the yamllint document-start marker. Add TestAllScaffoldYAMLDocumentStartMarker to prevent future regressions. Addresses review feedback on fullsend-ai#1947 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
Refactor TestAllScaffoldYAMLDocumentStartMarker to use WalkFullsendRepoAll instead of filepath.WalkDir + os.ReadFile. This matches the established test pattern and validates the embedded content that production code actually serves. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Extract saveWorkflowRunDebugInfo from the triage phase so both triage and unenrollment use the same code to fetch workflow logs, download artifacts, and emit GitHub Actions annotations on failure. In the unenrollment phase, find the most recent repo-maintenance run after the CLI returns and capture its logs if it failed. This gives us visibility into why the removal PR never appears. Closes fullsend-ai#1960 Signed-off-by: Ralph Bean <rbean@redhat.com> Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…en scope The minted token only covered enabled repos, so the reconcile script couldn't access disabled repos during unenrollment. API calls failed silently (2>/dev/null), causing the script to treat them as "already unenrolled" and skip removal PR creation. Also capture repo-maintenance workflow logs unconditionally in the e2e test so silent-skip problems leave a debug trail even when the workflow reports success. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…nostics - Replace two yq queries + bash concatenation with a single query that selects both enabled and disabled repos - Return debugDir from saveWorkflowRunDebugInfo so callers can reference it in failure messages - Consolidate the two removal-PR failure paths into one with conditional repo-maintenance run context - Update test assertion to match the simplified yq query Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…eters Split annotation prefix into annotationMsg (::notice::) for plain messages and annotationFile (::notice ) for file-parameter annotations. The previous code reused ::notice:: for both, producing malformed annotations like ::notice::file=path::message instead of the correct ::notice file=path::message. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…debug-logs fix(fullsend-ai#1960): include disabled repos in repo-maintenance token scope
The added --- line pushed dispatch.yml to 392 lines, exceeding the previous 391 cap. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…aml-doc-start-marker fix(fullsend-ai#1946): add YAML document start marker to shim workflow template
The fallback when merge-base fails silently reverted to CHANGED_FILES, which re-introduces false positives after rebase. Align with the post-code.sh fallback chain: warn, try origin/TARGET..HEAD, then HEAD~1..HEAD. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Remove path filters from pull_request trigger and add merge_group trigger so the e2e job always reports a status. When no e2e-relevant files changed, the job short-circuits after checking the PR file list via the GitHub API. This lets the e2e check be made a required status check without blocking docs-only or other non-code PRs from the merge queue. Push-to-main keeps its path filters since it doesn't gate the merge queue. Closes fullsend-ai#1987 Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
The sandbox image pinned Go 1.24.13 while go.mod requires 1.26.0. This version mismatch prevented the code agent from compiling or testing Go code, causing it to submit untested PRs (e.g. PR fullsend-ai#1016 shipped a duplicate map key that go build would have caught). Update GO_VERSION from 1.24.13 to 1.26.0 and replace the SHA256 checksums for linux-amd64 and linux-arm64 archives. Checksums sourced from docker-library/golang at the go1.26.0 release commit. The sandbox image will rebuild automatically via the sandbox-images.yml workflow on push to main. Note: pre-commit could not run in-sandbox (exit 3) because the gitleaks hook tried to auto-download a newer Go toolchain, which is blocked by sandbox network policy. This is the same version mismatch this commit fixes. Closes fullsend-ai#1344
gopls 0.18.1 depends on golang.org/x/tools v0.30.x which fails to compile under Go 1.26.0 (invalid array length in tokeninternal.go). Bump to gopls 0.22.0, the latest release, which supports Go 1.26. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
git %(contents:subject) skips leading blank lines, so a blank first
line still picks up the first category header as .TagSubject. Using
the tag name itself ensures .TagSubject == .Tag, which the goreleaser
guard suppresses, producing a clean release title.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
Opens /organizations/{org}/settings/installations/{id} instead of
/organizations/{org}/settings/apps/{slug}/advanced. The /advanced page
requires app owner access; org admins installing a third-party app need
the installation settings page to uninstall.
Falls back to /settings/installations list when the installation ID is
unavailable (e.g. ListOrgInstallations failed).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
…ract - Distinguish what ADR 0017/0025 originally established (request bodies, response transformation) from the broader scope this ADR introduces - Add implementation model Options subsection: language-agnostic process contract vs. Go interface - Clarify the process contract is the runner's enforcement boundary; the Go interface is the recommended internal pattern for fullsend-maintained servers, not a runner requirement - Scope experiment reference — experiment validated the flow, Go interface decision came from review discussion - Update Consequences for Go interface pattern and process contract as enforcement boundary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Marta Anon <manon@redhat.com>
End-to-end guide for creating new custom agents from scratch on per-repo fullsend installations. Covers agent prompts, harness config, sandbox policies, output schemas, pre/post scripts, GitHub Actions workflows, slash-command dispatch, and troubleshooting. Improvements over the source material: - Fixed nested code block rendering (4-backtick outer fences) - Added post-script security section (treat agent output as untrusted) - Added glossary cross-references and Reference section - Deduplicated with customizing-agents.md via cross-links Addressed review feedback (PR fullsend-ai#1179): - Removed explore/refine agent references (not upstream) - Fixed install command (fullsend admin install <owner/repo>) - Removed WIF secrets from prerequisites (part of installation) - Replaced security model diagram with inline summary + link - Removed Step 1 (config.yaml roles not yet extensible) - Added target-repo checkout step and --target-repo flag - Added MY_VAR to workflow env block - Reduced permissions to least-privilege (contents:read, id-token:write) - Added trigger instructions (UI and CLI) before dispatch workflow - Extracted agent chaining to future guide - Condensed debugging tips into troubleshooting table Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
docs(guides): add building custom agents guide
…ned-status-cleanup fix(fullsend-ai#2149): finalize orphaned status comments on hard process termination
…d-cancel-context fix(fullsend-ai#2148): thread signal-aware context into sandbox exec
feat(fetchsvc): runner-side runtime fetch service (Phase 4, PR 1)
…view feat(harness): add base composition for harness inheritance (ADR-0045 PR 4/7)
…erver docs: ADR 0046 — host-side API server design for sandboxed agents
…lgtm-no-findings feat(review): use "Looks good to me" when no findings
Users must request mint enrollment before configuring fullsend, but the getting started guide did not mention this step. Add it as step 1 with a note that ADR 0043 (public mint mode) will remove the manual enrollment requirement once implemented. Signed-off-by: Wayne Sun <gsun@redhat.com>
…tches remote Use skopeo inspect to check the remote image digest (~1s) before deciding whether to pull. When the locally loaded image (from cache) already matches the remote, skip the pull, save, and cache upload entirely. This avoids ~4.5 min of overhead on every run when the image hasn't changed. On cold cache or digest mismatch, behavior is unchanged: pull the new image and update the cache. When skopeo cannot fetch the remote digest, fall through to pull with a clear log message. Relates-to: fullsend-ai#2222 Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
…e-cache-digest-check perf(fullsend-ai#2222): skip sandbox image pull when cached digest matches remote
…impl-questions fix(fullsend-ai#2024): triage agent must not ask reporters implementation questions
…-prereq docs: add mint enrollment prerequisite to getting started guide
…-claude-agents-md chore(fullsend-ai#2088): consolidate agent instructions into AGENTS.md
Agents sometimes produce Signed-off-by trailers via `git commit -s`, causing gitlint body-max-line-length failures because the bot noreply email makes the trailer ~90 characters. DCO sign-off is a human attestation of personhood — agents are not people, and the DCO app already waives the check for bot authors (fullsend-ai#1449). Changes: - SKILL.md: add prominent prohibition against `git commit -s` and Signed-off-by trailers in the commit section (step 10b) - post-code.sh: add section 3b that scans agent commits for Signed-off-by trailers and exits non-zero if found - post-fix.sh: add section 2b with the same trailer check - post-code-test.sh: add 5 test cases covering trailer detection (present, absent, empty body, mid-line mention, mixed trailers) Note: `make lint` could not run due to network restrictions preventing shellcheck installation. All scripts pass `bash -n` syntax checking and the test suite (51 tests). Closes fullsend-ai#2236
…acing questions The HARD CONSTRAINT on line 146 required `action: "insufficient"` for "ANY open questions or information gaps," which contradicted the question classification rule (line 124) allowing implementation-facing questions to proceed without `insufficient` unless they materially block triage. Scope the HARD CONSTRAINT to user-facing questions and add an explicit carve-out referencing the question classification rules, so the two sections give consistent guidance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
…cile-insufficient-rules fix(fullsend-ai#2237): scope anti-premature-resolution rule to user-facing questions
…tall-cleanup-enrollment fix(fullsend-ai#2017): unenroll repos during uninstall via repo-maintenance workflow
Remove stale "design exploration" framing from the "How to work in this repo" section — the project now ships production Go code with CI and deployments. Scope the options-with-trade-offs and target audience bullets to problem documents specifically, where they still apply. Also fix stale ADR status guidance (related to fullsend-ai#1745): replace references to removed Proposed/Undecided statuses with the current three-status model (Accepted, Deprecated, Superseded). Note: make lint could not run in the sandbox due to shellcheck installation failure (network restriction). The post-script runs an authoritative pre-commit check on the runner. Closes fullsend-ai#2242
…-outdated-agents-md docs(fullsend-ai#2242): prune outdated guidance from AGENTS.md
- Add Signed-off-by prohibition to fix-review/SKILL.md (incomplete-scope) - Add variant-casing test to confirm case-sensitive matching (test-adequacy) - Align success messages to em-dash pattern (success-message-consistency) Addresses review feedback on fullsend-ai#2240
…gned-off-by fix(fullsend-ai#2236): reject Signed-off-by trailers in agent commits
The stale-shim comparison in reconcile-repos.sh used managed_content_b64() which decoded base64, extracted managed content, then re-encoded to base64. Bash command substitution strips trailing newlines, so the re-encoded base64 could differ from the original even when the decoded text was identical. This caused false-positive drift detection, leading to bogus update PRs (e.g. PR fullsend-ai#2101) that removed the sentinel lines. Replace the base64-to-base64 comparison with decoded text comparison: decode both sides, strip carriage returns, extract managed content via extract_managed_ content, and compare the resulting strings directly. For pre-sentinel shims (no sentinel found), fall back to comparing full decoded content. Add a regression test that verifies logically identical content with different trailing newlines is not flagged as stale. Note: pre-commit could not run in sandbox (shellcheck download blocked by network policy). The post-script runs pre-commit authoritatively on the runner. Closes fullsend-ai#2247
Owner
Author
|
/fs quality |
|
🤖 Finished Review · ✅ Success · Started 6:04 AM UTC · Completed 6:06 AM UTC |
|
Review skipped — this PR is already closed. The Posted by fullsend post-review check |
QualityFlow Pipeline Summary
Test Output
Issue: GH-2247 Generated by QualityFlow |
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
11 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.
The stale-shim comparison in reconcile-repos.sh used managed_content_b64() which decoded base64, extracted managed content, then re-encoded to base64. Bash command substitution strips trailing newlines, so the re-encoded base64 could differ from the original even when the decoded text was identical. This caused false-positive drift detection, leading to bogus update PRs (e.g. PR fullsend-ai#2101) that removed the sentinel lines.
Replace the base64-to-base64 comparison with decoded text comparison: decode both sides, strip carriage returns, extract managed content via extract_managed_ content, and compare the resulting strings directly. For pre-sentinel shims (no sentinel found), fall back to comparing full decoded content.
Add a regression test that verifies logically identical content with different trailing newlines is not flagged as stale.
Note: pre-commit could not run in sandbox (shellcheck download blocked by network policy). The post-script runs pre-commit authoritatively on the runner.
Closes fullsend-ai#2247
Post-script verification
agent/2247-fix-shim-stale-comparison)cda8e5712200b8cf6e19be99722d6d33c4caabb7..HEAD)Mirrored from upstream fullsend-ai#2254 for QualityFlow demo.