Skip to content

fix: load per-repo config.yaml in fullsend run and reusable workflows#3000

Open
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-fix-2970-per-repo-config
Open

fix: load per-repo config.yaml in fullsend run and reusable workflows#3000
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-fix-2970-per-repo-config

Conversation

@ggallen

@ggallen ggallen commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Add OrgConfigFromPerRepo adapter in internal/config/config.go to bridge PerRepoConfig fields into OrgConfig
  • Rewrite tryLoadOrgConfig/requireOrgConfig in internal/cli/orgconfig.go as tryLoadConfig/requireConfig with per-repo YAML fallback (backward-compatible aliases preserved)
  • Update all six reusable workflows (reusable-{triage,code,review,fix,retro,prioritize}.yml) to layer workspace files under .fullsend/ when install_mode == per-repo and pass fullsend-dir: .fullsend to the action invocation
  • Add comprehensive tests: TestOrgConfigFromPerRepo, TestRunAgent_PerRepoConfig, TestTryLoadConfig_* (5 variants), TestRequireConfig_* (4 variants)

Closes #2970

Test plan

  • go test ./internal/config/ ./internal/cli/ — all pass
  • Coverage on tryLoadConfig 92.3%, requireConfig 92.9%, OrgConfigFromPerRepo 100%
  • Pre-commit hooks (gofmt, go vet, actionlint, shellcheck, pinact) all pass
  • E2E: deploy a per-repo installation and verify fullsend run reads .fullsend/config.yaml

🤖 Generated with Claude Code

@ggallen ggallen requested a review from a team as a code owner July 3, 2026 19:57
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:58 PM UTC · Completed 8:13 PM UTC
Commit: 5cd68bd · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix per-repo config.yaml loading for CLI run + reusable workflows

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Teach CLI config loading to accept per-org and per-repo config.yaml formats
• Update reusable workflows to stage scaffold/custom files under .fullsend/ for per-repo installs
• Add tests covering per-repo fallback and config read/parse error handling
Diagram

graph TD
  WF([Reusable workflow]) --> MODE{install_mode per-repo?}
  MODE -- "no" --> ROOT["repo root"] --> ACT[[fullsend action]] --> CLI["fullsend run"] --> LOADER{{"tryLoad/requireConfig"}} --> ORG["OrgConfig (unified)"]
  MODE -- "yes" --> FSDIR[".fullsend/"] --> ACT

  subgraph Legend
    direction LR
    _wf([Workflow]) ~~~ _dec{Decision} ~~~ _dir["Directory"] ~~~ _act[[Action]] ~~~ _proc{{Process}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Unify YAML schema into a single config struct
  • ➕ Avoids adapter function and dual parsing
  • ➕ One canonical documentation surface for config.yaml
  • ➖ Riskier change: could break existing org-level config semantics/validation
  • ➖ Harder to keep backward compatibility if fields diverge over time
2. Introduce a small shared interface/common struct for allowlist fields
  • ➕ More explicit about which fields are required for remote validation
  • ➕ Reduces temptation to treat PerRepoConfig as a full OrgConfig
  • ➖ More refactoring across callers that currently expect OrgConfig
  • ➖ Less ergonomic if most call sites actually want OrgConfig today

Recommendation: The current approach (parse OrgConfig first, then parse PerRepoConfig and adapt via OrgConfigFromPerRepo) is the least disruptive and preserves existing call-site expectations. It keeps the compatibility surface narrow (only shared fields) while enabling per-repo installs to work without broad refactors.

Files changed (11) +260 / -50

Bug fix (2) +51 / -19
orgconfig.goAdd config loader fallback to PerRepoConfig with compatibility aliases +37/-19

Add config loader fallback to PerRepoConfig with compatibility aliases

• Replaces tryLoadOrgConfig/requireOrgConfig with tryLoadConfig/requireConfig that first parses OrgConfig, then falls back to PerRepoConfig and adapts it to OrgConfig. Keeps the old function names as aliases for backward compatibility and updates user-facing error/warn messages accordingly.

internal/cli/orgconfig.go

config.goAdd OrgConfigFromPerRepo adapter +14/-0

Add OrgConfigFromPerRepo adapter

• Implements OrgConfigFromPerRepo to map shared PerRepoConfig fields into an OrgConfig, leaving Org-only fields zero-valued. Enables CLI code paths that expect OrgConfig to operate with per-repo installs.

internal/config/config.go

Tests (3) +155 / -7
lock_test.goUpdate lock command tests for new config error wording +3/-3

Update lock command tests for new config error wording

• Adjusts assertions to match renamed error messages (org config → config) and updated missing-config guidance for URL-referenced resources.

internal/cli/lock_test.go

run_test.goAdd tests for per-repo config fallback and strict/soft loading behaviors +124/-4

Add tests for per-repo config fallback and strict/soft loading behaviors

• Introduces coverage for per-repo config parsing via the unified loader, plus missing/unreadable/malformed cases for both tryLoadConfig and requireConfig. Updates existing URL-ref tests to expect the new error text.

internal/cli/run_test.go

config_test.goTest OrgConfigFromPerRepo field mapping +28/-0

Test OrgConfigFromPerRepo field mapping

• Adds a unit test verifying that shared fields (version, kill switch, agents, allowlist, create-issues config) are copied and that OrgConfig-only fields remain unset/empty.

internal/config/config_test.go

Other (6) +54 / -24
reusable-code.ymlLayer scaffold/custom files into .fullsend/ for per-repo installs +9/-4

Layer scaffold/custom files into .fullsend/ for per-repo installs

• Adds a DEST prefix that switches to .fullsend/ when install_mode is per-repo, so layered directories are copied into the correct workspace location. Passes fullsend-dir to the action to align runtime behavior with the staged files.

.github/workflows/reusable-code.yml

reusable-fix.ymlSupport per-repo workspace layout and pass fullsend-dir +9/-4

Support per-repo workspace layout and pass fullsend-dir

• Copies scaffold/customized directories into .fullsend/ when running in per-repo mode. Adds fullsend-dir input to the action invocation so the action reads config/assets from the same directory.

.github/workflows/reusable-fix.yml

reusable-prioritize.ymlStage layered directories under .fullsend/ for per-repo mode +9/-4

Stage layered directories under .fullsend/ for per-repo mode

• Introduces a DEST prefix to redirect layered file copies into .fullsend/ when install_mode is per-repo. Also passes fullsend-dir to ensure the action uses the per-repo directory.

.github/workflows/reusable-prioritize.yml

reusable-retro.ymlEnable per-repo directory layout for retro workflow +9/-4

Enable per-repo directory layout for retro workflow

• Updates the file layering step to write into .fullsend/ when install_mode is per-repo. Adds fullsend-dir to the action inputs to match the new staging location.

.github/workflows/reusable-retro.yml

reusable-review.ymlRoute layered files into .fullsend/ and set fullsend-dir +9/-4

Route layered files into .fullsend/ and set fullsend-dir

• Makes the scaffold/custom overlay honor per-repo installs by copying into .fullsend/. Passes fullsend-dir so the action/CLI resolves config from the correct directory.

.github/workflows/reusable-review.yml

reusable-triage.ymlSupport per-repo install workspace and action directory +9/-4

Support per-repo install workspace and action directory

• Adds DEST-based copying so layered directories land under .fullsend/ when install_mode is per-repo. Provides fullsend-dir to the action to align config discovery with the workflow layout.

.github/workflows/reusable-triage.yml

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Site preview

Preview: https://400a58af-site.fullsend-ai.workers.dev

Commit: f5cb1490a969b97333285195746132686b655285

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

qodo-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider


Remediation recommended

1. Parse errors misattributed ✓ Resolved 🐞 Bug ◔ Observability
Description
When both OrgConfig and PerRepoConfig parsing fail, tryLoadConfig/requireConfig warn/return
using the OrgConfig parse error (parseErr) and discard perRepoErr, producing confusing messages
like “parsing config: parsing org config: …” and losing the per-repo parse context. This makes
debugging malformed config.yaml harder, especially in per-repo installs.
Code

internal/cli/orgconfig.go[R65-69]

+	perRepo, perRepoErr := config.ParsePerRepoConfig(data)
+	if perRepoErr != nil {
+		printer.StepFail("Failed to parse config")
+		return nil, fmt.Errorf("parsing config: %w", parseErr)
+	}
Relevance

⭐⭐⭐ High

Team often accepts clearer diagnostics; likely to return perRepoErr instead of misleading parseErr.

PR-#586
PR-#1333
PR-#697

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The strict loader/warn path ignores perRepoErr and wraps/prints parseErr from ParseOrgConfig,
while ParseOrgConfig itself prefixes errors with “parsing org config”, yielding misleading and
duplicated messages in the new generic “config” loader.

internal/cli/orgconfig.go[20-41]
internal/cli/orgconfig.go[48-71]
internal/config/config.go[243-250]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
If both parse attempts fail, the code currently reports only `parseErr` (OrgConfig parse failure) and ignores `perRepoErr`, leading to confusing/duplicated error strings and losing the per-repo parse context.

## Issue Context
`ParseOrgConfig` wraps errors with a “parsing org config:” prefix, so wrapping that again as “parsing config:” yields confusing messages and doesn’t reflect the per-repo parse attempt.

## Fix Focus Areas
- internal/cli/orgconfig.go[24-41]
- internal/cli/orgconfig.go[52-70]

### Suggested implementation approach
- When both parses fail:
 - Prefer returning the underlying YAML error once (unwrap) OR
 - Return a combined error that includes both `parseErr` and `perRepoErr` (e.g., `fmt.Errorf("parsing config as org: %v; as per-repo: %v", parseErr, perRepoErr)`), and use the same combined message for the warning path.
- In the warning path (`tryLoadConfig`), log the combined error (or at least the second attempt’s error) so the message aligns with per-repo usage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Per-repo fallback bypassed ✓ Resolved 🐞 Bug ≡ Correctness
Description
tryLoadConfig/requireConfig return immediately on successful ParseOrgConfig, but
ParseOrgConfig is a plain YAML unmarshal with no structural validation, so per-repo config YAML
(missing org-only fields like dispatch/defaults/repos) will typically parse as OrgConfig and
never reach the PerRepoConfig fallback/adapter. This can silently ignore per-repo-only keys (e.g.,
roles) and makes the new “accepts both formats” behavior unreliable if per-repo fields later need
explicit adaptation.
Code

internal/cli/orgconfig.go[R31-41]

	}
	cfg, parseErr := config.ParseOrgConfig(data)
-	if parseErr != nil {
-		printer.StepWarn("Org config malformed (remote resource allowlist unavailable): " + parseErr.Error())
+	if parseErr == nil {
+		return cfg
+	}
+	perRepo, perRepoErr := config.ParsePerRepoConfig(data)
+	if perRepoErr != nil {
+		printer.StepWarn("Config malformed (remote resource allowlist unavailable): " + parseErr.Error())
		return nil
	}
-	return cfg
+	return config.OrgConfigFromPerRepo(perRepo)
Relevance

⭐⭐ Medium

Some precedent for stricter config validation, but no history on org/per-repo parse ordering
fallback bypass.

PR-#2768
PR-#2769

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
tryLoadConfig short-circuits on ParseOrgConfig success, while ParseOrgConfig does not validate
presence of org-only required fields and thus will accept per-repo YAML, preventing the new per-repo
fallback/adapter from being used and allowing per-repo-only keys like roles to be ignored.

internal/cli/orgconfig.go[20-42]
internal/config/config.go[136-147]
internal/config/config.go[243-250]
internal/config/config.go[405-414]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tryLoadConfig`/`requireConfig` attempt `ParseOrgConfig` first and return on success. Since `ParseOrgConfig` currently does not validate required org-only structure, it will accept per-repo YAML and bypass the intended PerRepoConfig parsing/adapter path, which can silently drop per-repo-only keys and undermines the purpose of `OrgConfigFromPerRepo`.

## Issue Context
- `ParseOrgConfig` is permissive (simple `yaml.Unmarshal`) and does not reject missing org-only fields.
- Per-repo config includes keys (e.g. `roles`) that are not present on `OrgConfig`, so they are ignored when parsed as OrgConfig.

## Fix Focus Areas
- internal/cli/orgconfig.go[20-41]
- internal/config/config.go[136-147]
- internal/config/config.go[243-250]
- internal/config/config.go[405-414]

### Suggested implementation approach
- Add lightweight format detection before choosing a parser, e.g. decode into `map[string]any` (or `yaml.Node`) and treat it as org-config only if org-only keys like `dispatch`/`defaults`/`repos` are present; otherwise parse as per-repo.
- Alternatively, after `ParseOrgConfig`, if org-only required fields are clearly unset (e.g., `Dispatch.Platform == ""` and `Repos == nil`), then fall back to per-repo parsing.
- Keep behavior backward-compatible and avoid turning on strict known-fields decoding globally unless intentionally desired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread internal/cli/orgconfig.go Outdated
Comment thread internal/cli/orgconfig.go Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [config-type-confusion] internal/cli/orgconfig.go:30 — The isPerRepoYAML heuristic defaults to per-repo when no org-specific discriminator keys (dispatch, repos, inference, defaults) are present. A minimal org config that omits all four keys would be parsed as PerRepoConfig. In practice the shared fields parse identically under both parsers, so the functional impact is negligible.

  • [variable-alias-pattern] internal/cli/orgconfig.go:439 — The var alias pattern (var tryLoadOrgConfig = tryLoadFullsendConfig) is used for backwards compatibility but is rare in the codebase. The deprecation plan documents these as temporary aliases to be removed in a future PR. Consider removing them when all call sites are updated.

  • [mutable-function-binding] internal/cli/orgconfig.go:72tryLoadOrgConfig and requireOrgConfig are now package-level var declarations rather than func declarations, making them reassignable at runtime. The risk is minimal since this is an internal package and the aliases are planned for removal.

Previous run

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six files under .github/ are modified. The PR links to issue Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033 #2970 and explains the rationale (aligning per-repo workspace layout with ADR 0033). Human approval is always required for protected-path changes, regardless of context.

  • [logic-error] internal/cli/run.go:2350setupStatusNotifier reads config.yaml via config.ParseOrgConfig(data) directly, bypassing the new isPerRepoYAML/tryLoadFullsendConfig detection. In per-repo mode (where fullsendDir is .fullsend), this path reads a per-repo config file and parses it as OrgConfig. ParseOrgConfig silently succeeds (YAML ignores unknown keys) but produces an OrgConfig where Defaults.StatusNotifications is always nil. This is not a regression (the path was unreachable before), but it is a newly-reachable code path that does not use the new unified config loader.

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for syntactically invalid YAML (because yaml.Unmarshal fails), causing tryLoadFullsendConfig/requireFullsendConfig to fall through to ParseOrgConfig. This produces error messages mentioning "Org config malformed" even when the file was intended to be a per-repo config. The functional behavior (returning nil/error) is correct; only the error message wording is affected.

  • [config-type-confusion] internal/cli/orgconfig.go:30 — The isPerRepoYAML heuristic defaults to per-repo when no org-specific discriminator keys (dispatch, repos, inference, defaults) are present. A minimal org config that omits all four keys would be parsed as PerRepoConfig. In practice the shared fields parse identically under both parsers, so the functional impact is negligible.

  • [code-duplication] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The DEST variable pattern and CUSTOM_BASE duplication is replicated across all 6 workflow files. This extends pre-existing technical debt in the workflow layer.

  • [error-message-consistency] .github/workflows/reusable-code.yml — Error message changed from including the INSTALL_MODE value to a static string, removing diagnostic information that aids debugging. Replicated across all 6 workflow files.

Previous run (2)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, agents, create_issues) but no roles key would be misidentified as OrgConfig and parsed via ParseOrgConfig. In practice NewPerRepoConfig always populates Roles with DefaultAgentRoles(), and the shared fields parse identically under either parser, so the functional impact is negligible for machine-generated configs.

  • [code-duplication] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The DEST variable pattern and CUSTOM_BASE duplication is replicated across all 6 workflow files. This extends pre-existing technical debt in the workflow layer. Consider extracting shared workspace-preparation logic into a composite action in a follow-up.

Previous run (3)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six files under .github/ are modified. The PR links to issue Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033 #2970 and explains the rationale (aligning per-repo workspace layout with ADR 0033). Human approval is always required for protected-path changes, regardless of context.

  • [function-naming] internal/cli/orgconfig.go — The function names tryLoadRuntimeConfig/requireRuntimeConfig introduce "runtime config" as a new term that creates ambiguity with existing uses in the codebase (e.g., internal/runtime/claude.go uses "runtime config" to refer to Claude sandbox directories). Consider a more specific term that distinguishes from existing "runtime" references.

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields but no roles key would be parsed as OrgConfig instead. In practice NewPerRepoConfig always populates Roles with DefaultAgentRoles(), and the shared fields parse identically under either parser, so the functional impact is negligible.

  • [GHA workflow command injection] .github/workflows/reusable-code.yml:91INSTALL_MODE is interpolated into a ::error:: workflow command without sanitizing :: sequences or encoded newlines. The same pattern appears in all 6 reusable workflows. Because workflow_call inputs come from trusted callers, exploitation requires a compromised caller.

  • [scope-coherence] docs/plans/deprecate-per-org-install.md — The file orgconfig.go was not renamed to match the new function names (tryLoadRuntimeConfig/requireRuntimeConfig), leaving the codebase in a transitional state. The deprecation plan indicates the file rename is deferred to Phase 2 (PR 8).

  • [naming-conventions] internal/cli/orgconfig.go — Backward-compatibility aliases use var assignment (var tryLoadOrgConfig = tryLoadRuntimeConfig). The codebase uses var assignment for testability seams, not backward compatibility. This is a reasonable interim approach for scope control.

  • [code-duplication] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The DEST variable pattern is replicated across all 6 workflow files. This extends the pre-existing CUSTOM_BASE duplication. Consider extracting shared workspace-preparation logic into a composite action in a follow-up.

Previous run (4)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields still work correctly regardless of which parser runs.

  • [documentation-accuracy] internal/config/config.go:443OrgConfigFromPerRepo doc comment claims "OrgConfig-specific fields (Dispatch, Inference, Repos, Defaults) remain zero-valued" but the function body explicitly sets Defaults to RepoDefaults{Roles: pr.Roles}. The comment should list only Dispatch, Inference, and Repos as zero-valued.

  • [stale-doc] docs/plans/deprecate-per-org-install.md — Implementation plan references old function names tryLoadOrgConfig() and requireOrgConfig() which were renamed to tryLoadRuntimeConfig() and requireRuntimeConfig(). The plan describes Phase 2 work that would rename these functions, but the PR has already performed an intermediate rename.

Previous run (5)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields (AllowedRemoteResources, Agents, CreateIssues) still work correctly regardless of which parser runs.

  • [edge-case] internal/cli/orgconfig.go:69tryLoadOrgConfig and requireOrgConfig are now package-level var aliases rather than declared functions. This is a standard Go pattern for test-seam injection, and the unexported scope limits exposure to the cli package.

  • [naming-consistency] internal/cli/orgconfig.go — The rename to tryLoadInstallConfig/requireInstallConfig uses "install" terminology. These functions load runtime configuration (OrgConfig or PerRepoConfig), not installation metadata. A name like tryLoadRuntimeConfig might more clearly communicate the polymorphic loading intent.

Previous run (6)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields (AllowedRemoteResources, Agents, CreateIssues) still work correctly regardless of which parser runs.

  • [edge-case] internal/cli/orgconfig.go:69tryLoadOrgConfig and requireOrgConfig are now package-level var aliases rather than declared functions. This is a standard Go pattern for test-seam injection, and the unexported scope limits exposure to the cli package.

  • [naming-consistency] internal/cli/orgconfig.go — The rename to tryLoadInstallConfig/requireInstallConfig uses "install" terminology. These functions load runtime configuration (OrgConfig or PerRepoConfig), not installation metadata. A name like tryLoadRuntimeConfig might more clearly communicate the polymorphic loading intent.

Previous run (7)

Review

Findings

Medium

  • [logic-error] internal/cli/orgconfig.go:31 — The fallback from ParseOrgConfig to ParsePerRepoConfig is dead code. Both parsers use plain yaml.Unmarshal without KnownFields(true), so ParseOrgConfig silently ignores unknown keys (like roles) and never returns an error for valid PerRepoConfig YAML. The if parseErr == nil { return cfg } branch always executes, and the ParsePerRepoConfig + OrgConfigFromPerRepo path is never reached. Functionally the result is equivalent for shared fields, but the per-repo Roles field is silently dropped, and any future PerRepoConfig-specific field will also be lost.
    Remediation: Add structural discrimination — check for a PerRepoConfig-only top-level key (roles) or an OrgConfig-only key (dispatch/repos) to choose the right parser, or use yaml.v3 Decoder.KnownFields(true) so unknown keys cause ParseOrgConfig to fail, triggering the intended fallback.

  • [protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six files under .github/ are modified. The PR links to issue Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033 #2970 and explains the rationale (aligning per-repo workspace layout with ADR 0033). Human approval is always required for protected-path changes, regardless of context.

Low

  • [logic-error] internal/config/config.go:449OrgConfigFromPerRepo does not map PerRepoConfig.Roles to OrgConfig.Defaults.Roles. The adapter leaves Defaults.Roles as nil. Currently on the dead-code path (per the medium finding above) and Defaults.Roles is not consumed during fullsend run, but creates an incomplete adaptation if the fallback is fixed.

  • [error-handling-idiom] internal/cli/orgconfig.go — Error messages genericized from "org config" to "config" (e.g., "Config unreadable", "Failed to parse config"). The generic term loses helpful context in a codebase with multiple config types (OrgConfig, PerRepoConfig, harness YAML, etc.).

  • [naming-convention] internal/cli/orgconfig.go:23tryLoadConfig and requireConfig are overly generic names in a package with multiple config types. Existing patterns use specific prefixes (ParseOrgConfig, ParsePerRepoConfig). Consider tryLoadInstallConfig/requireInstallConfig.

  • [documentation-comment-format] internal/cli/orgconfig.go:44 — The var alias comments describe the aliasing relationship rather than purpose, diverging from the established pattern where package-level vars document what they do (e.g., test seams like statusMintToken).


Labels: PR fixes per-repo config loading across reusable dispatch workflows and CLI install paths, matching the linked issue's component labels.

Previous run (8)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, agents, allowed_remote_resources, create_issues) but no roles key would be misidentified as OrgConfig and parsed via ParseOrgConfig. In practice NewPerRepoConfig always populates Roles with DefaultAgentRoles(), and the shared fields parse identically under either parser, so the functional impact is negligible for machine-generated configs.

  • [code-duplication] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The DEST variable pattern and CUSTOM_BASE duplication is replicated across all 6 workflow files. This extends pre-existing technical debt in the workflow layer. Consider extracting shared workspace-preparation logic into a composite action in a follow-up.


Labels: PR fixes a defect in per-repo config loading; type/bug matches the confirmed-defect description.

Previous run (9)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six files under .github/ are modified. The PR links to issue Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033 #2970 and explains the rationale (aligning per-repo workspace layout with ADR 0033). Human approval is always required for protected-path changes, regardless of context.

  • [function-naming] internal/cli/orgconfig.go — The function names tryLoadRuntimeConfig/requireRuntimeConfig introduce "runtime config" as a new term that creates ambiguity with existing uses in the codebase (e.g., internal/runtime/claude.go uses "runtime config" to refer to Claude sandbox directories). Consider a more specific term that distinguishes from existing "runtime" references.

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields but no roles key would be parsed as OrgConfig instead. In practice NewPerRepoConfig always populates Roles with DefaultAgentRoles(), and the shared fields parse identically under either parser, so the functional impact is negligible.

  • [GHA workflow command injection] .github/workflows/reusable-code.yml:91INSTALL_MODE is interpolated into a ::error:: workflow command without sanitizing :: sequences or encoded newlines. The same pattern appears in all 6 reusable workflows. Because workflow_call inputs come from trusted callers, exploitation requires a compromised caller.

  • [scope-coherence] docs/plans/deprecate-per-org-install.md — The file orgconfig.go was not renamed to match the new function names (tryLoadRuntimeConfig/requireRuntimeConfig), leaving the codebase in a transitional state. The deprecation plan indicates the file rename is deferred to Phase 2 (PR 8).

  • [naming-conventions] internal/cli/orgconfig.go — Backward-compatibility aliases use var assignment (var tryLoadOrgConfig = tryLoadRuntimeConfig). The codebase uses var assignment for testability seams, not backward compatibility. This is a reasonable interim approach for scope control.

  • [code-duplication] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — The DEST variable pattern is replicated across all 6 workflow files. This extends the pre-existing CUSTOM_BASE duplication. Consider extracting shared workspace-preparation logic into a composite action in a follow-up.

Previous run (10)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields still work correctly regardless of which parser runs.

  • [documentation-accuracy] internal/config/config.go:443OrgConfigFromPerRepo doc comment claims "OrgConfig-specific fields (Dispatch, Inference, Repos, Defaults) remain zero-valued" but the function body explicitly sets Defaults to RepoDefaults{Roles: pr.Roles}. The comment should list only Dispatch, Inference, and Repos as zero-valued.

  • [stale-doc] docs/plans/deprecate-per-org-install.md — Implementation plan references old function names tryLoadOrgConfig() and requireOrgConfig() which were renamed to tryLoadRuntimeConfig() and requireRuntimeConfig(). The plan describes Phase 2 work that would rename these functions, but the PR has already performed an intermediate rename.

Previous run (11)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields (AllowedRemoteResources, Agents, CreateIssues) still work correctly regardless of which parser runs.

  • [edge-case] internal/cli/orgconfig.go:69tryLoadOrgConfig and requireOrgConfig are now package-level var aliases rather than declared functions. This is a standard Go pattern for test-seam injection, and the unexported scope limits exposure to the cli package.

  • [naming-consistency] internal/cli/orgconfig.go — The rename to tryLoadInstallConfig/requireInstallConfig uses "install" terminology. These functions load runtime configuration (OrgConfig or PerRepoConfig), not installation metadata. A name like tryLoadRuntimeConfig might more clearly communicate the polymorphic loading intent.

Previous run (12)

Review

Findings

Medium

Low

  • [logic-error] internal/cli/orgconfig.go:31isPerRepoYAML returns false for per-repo configs that omit the optional roles key (tagged omitempty). A valid per-repo config.yaml containing only shared fields (version, kill_switch, allowed_remote_resources, agents, create_issues) but no roles key would be parsed as OrgConfig instead. In practice this is unlikely since NewPerRepoConfig always populates Roles with DefaultAgentRoles(), but a manually edited config could trigger this path. The shared fields (AllowedRemoteResources, Agents, CreateIssues) still work correctly regardless of which parser runs.

  • [edge-case] internal/cli/orgconfig.go:69tryLoadOrgConfig and requireOrgConfig are now package-level var aliases rather than declared functions. This is a standard Go pattern for test-seam injection, and the unexported scope limits exposure to the cli package.

  • [naming-consistency] internal/cli/orgconfig.go — The rename to tryLoadInstallConfig/requireInstallConfig uses "install" terminology. These functions load runtime configuration (OrgConfig or PerRepoConfig), not installation metadata. A name like tryLoadRuntimeConfig might more clearly communicate the polymorphic loading intent.

Previous run (13)

Review

Findings

Medium

  • [logic-error] internal/cli/orgconfig.go:31 — The fallback from ParseOrgConfig to ParsePerRepoConfig is dead code. Both parsers use plain yaml.Unmarshal without KnownFields(true), so ParseOrgConfig silently ignores unknown keys (like roles) and never returns an error for valid PerRepoConfig YAML. The if parseErr == nil { return cfg } branch always executes, and the ParsePerRepoConfig + OrgConfigFromPerRepo path is never reached. Functionally the result is equivalent for shared fields, but the per-repo Roles field is silently dropped, and any future PerRepoConfig-specific field will also be lost.
    Remediation: Add structural discrimination — check for a PerRepoConfig-only top-level key (roles) or an OrgConfig-only key (dispatch/repos) to choose the right parser, or use yaml.v3 Decoder.KnownFields(true) so unknown keys cause ParseOrgConfig to fail, triggering the intended fallback.

  • [protected-path] .github/workflows/reusable-{code,fix,prioritize,retro,review,triage}.yml — Six files under .github/ are modified. The PR links to issue Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033 #2970 and explains the rationale (aligning per-repo workspace layout with ADR 0033). Human approval is always required for protected-path changes, regardless of context.

Low

  • [logic-error] internal/config/config.go:449OrgConfigFromPerRepo does not map PerRepoConfig.Roles to OrgConfig.Defaults.Roles. The adapter leaves Defaults.Roles as nil. Currently on the dead-code path (per the medium finding above) and Defaults.Roles is not consumed during fullsend run, but creates an incomplete adaptation if the fallback is fixed.

  • [error-handling-idiom] internal/cli/orgconfig.go — Error messages genericized from "org config" to "config" (e.g., "Config unreadable", "Failed to parse config"). The generic term loses helpful context in a codebase with multiple config types (OrgConfig, PerRepoConfig, harness YAML, etc.).

  • [naming-convention] internal/cli/orgconfig.go:23tryLoadConfig and requireConfig are overly generic names in a package with multiple config types. Existing patterns use specific prefixes (ParseOrgConfig, ParsePerRepoConfig). Consider tryLoadInstallConfig/requireInstallConfig.

  • [documentation-comment-format] internal/cli/orgconfig.go:44 — The var alias comments describe the aliasing relationship rather than purpose, diverging from the established pattern where package-level vars document what they do (e.g., test seams like statusMintToken).


Labels: PR fixes per-repo config loading across reusable dispatch workflows and CLI install paths, matching the linked issue's component labels.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/dispatch Workflow dispatch and triggers component/install CLI install and app setup component/harness Agent harness, config, and skills loading labels Jul 3, 2026
@ggallen ggallen force-pushed the worktree-fix-2970-per-repo-config branch from 5cd68bd to 1f23a4e Compare July 3, 2026 20:20
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:21 PM UTC · Completed 8:41 PM UTC
Commit: 1f23a4e · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 3, 2026
@ggallen ggallen force-pushed the worktree-fix-2970-per-repo-config branch from 1f23a4e to ed2a5dc Compare July 3, 2026 20:44
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:45 PM UTC · Completed 8:58 PM UTC
Commit: ed2a5dc · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 3, 2026
@ggallen ggallen force-pushed the worktree-fix-2970-per-repo-config branch from ed2a5dc to 11642cc Compare July 3, 2026 21:02
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 9:03 PM UTC · Completed 9:17 PM UTC
Commit: 11642cc · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:02 PM UTC · Completed 10:14 PM UTC
Commit: d93770d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment type/bug Confirmed defect in existing behavior and removed requires-manual-review Review requires human judgment labels Jul 3, 2026
@ggallen ggallen force-pushed the worktree-fix-2970-per-repo-config branch from d93770d to b22e5f9 Compare July 3, 2026 22:17
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:18 PM UTC · Completed 10:44 PM UTC
Commit: b22e5f9 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 3, 2026
Per-repo installations store config.yaml in .fullsend/ but the CLI only
parsed OrgConfig format. Add OrgConfigFromPerRepo adapter and teach
tryLoadFullsendConfig/requireFullsendConfig to fall back to PerRepoConfig
parsing using structural discrimination (isPerRepoYAML). Update all six
reusable workflows to layer workspace files under .fullsend/ when
install_mode is per-repo, and pass fullsend-dir to the action
invocation.

Closes fullsend-ai#2970

Signed-off-by: Greg Allen <greg@fullsend.ai>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@ggallen ggallen force-pushed the worktree-fix-2970-per-repo-config branch from b22e5f9 to f5cb149 Compare July 3, 2026 22:49
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:50 PM UTC · Completed 11:02 PM UTC
Commit: f5cb149 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/dispatch Workflow dispatch and triggers component/harness Agent harness, config, and skills loading component/install CLI install and app setup requires-manual-review Review requires human judgment type/bug Confirmed defect in existing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-repo config.yaml not read by fullsend run; workspace layout diverges from ADR 0033

1 participant