fix(identity-check): resolve goreleaser_config repo-root-relative for monorepos

[rianjs]

What

Makes identity-check resolve goreleaser_config relative to the repo root instead of --working-dir, unblocking the monorepo release migration (atlassian-cli#397). Implements the architect-recommended Option B.

Closes #14.

Why

For a monorepo, each tool's packaging lives at tools/<tool>/packaging/, so identity-check must run with --working-dir tools/<tool>. But the goreleaser configs live at the repo root (.goreleaser-cfl.yml, with per-build dir: tools/cfl), and the reusable release.yml runs goreleaser from the repo root. Previously goreleaser_config resolved relative to --working-dir, so no single value satisfied both the working-dir packaging checks and the root-run release goreleaser. Flat repos were unaffected, which is why slck/nrq/codereview migrated fine.

Change (Option B)

Resolution is now asymmetric (distribution.md §8.3):

• goreleaser_config → relative to a new --repo-root (default .) — goreleaser is a repo-root release operation even in a monorepo (go.work, shared modules, root tags/dist).
• packaging/*, manifest-path, version_file → stay --working-dir-relative (tool-local identity).

Flat repos are unchanged (working-dir . == repo root). A monorepo caller passes working-directory: tools/cfl and leaves repo-root at the default ..

• identity.py: validate() gains repo_root; --repo-root CLI arg (default .); docstring documents the split.
• action.yml: new repo-root input (default .).
• test_identity.py: 2 new monorepo tests (root-relative goreleaser passes; a wrong working-dir-relative resolution is caught); existing flat-repo tests thread repo_root.
• New tests/fixtures/identity/monorepo/ + identity-monorepo CI job exercising the action's repo-root plumbing end-to-end. Golden fixtures (slck/gro/nrq/jtk) pass repo-root == working-directory (they're flat/self-contained).
• cli-common distribution.md §8.3 documents the asymmetry (companion doc PR).

Proof

• pytest -q: 21 passed (19 existing + 2 monorepo).
• Smoke-tested against atlassian-cli's real .goreleaser-cfl.yml + tools/cfl/packaging: --working-dir tools/cfl --repo-root . → identity-check ok.

Follow-up

After merge: cut v1.1.0 and re-point the moving @v1 tag (backward-compatible — flat-repo behavior unchanged), then atlassian-cli#397 can migrate.

[rianjs]

Findings

No blocking findings.

The shape is correct and minimal:

• goreleaser_config now resolves from repo-root, matching reusable release.yml’s root-run GoReleaser behavior.
• Manifest, packaging/*, and version_file remain tool-local through working-directory.
• Flat repo consumers remain compatible with defaults because working-directory=. and repo-root=..
• Self-contained fixtures correctly opt into repo-root == working-directory.
• The monorepo fixture covers the new action-level plumbing, and the unit regression catches the old working-dir-relative failure mode.

I would not require repo-root on export-json; export does not validate the GoReleaser file and the release path uses validate-and-export, where validation now receives repo-root.

After merge, moving @v1 to the fixed ref is the required unblock for atlassian#397.

[rianjs]

TDD coverage assessment

Overall: strong. The core behavior change is well-exercised at both the unit and integration layers. Here is the breakdown:

What is covered

Asymmetric resolution — the central change is guarded by two dedicated unit tests:

• test_monorepo_root_relative_goreleaser_passes — the happy path: working_dir=tools/cfl, repo_root=<tmp_path>, goreleaser config found at the root, packaging found under the tool dir → no errors.
• test_monorepo_goreleaser_not_found_under_working_dir — the regression guard: passing repo_root=wd (the wrong directory) causes a "goreleaser_config not found" error, directly proving that the old incorrect resolution would be caught.

Flat-repo backward compatibility is proven implicitly by the 19 existing tests, all of which now pass wd for both working_dir and repo_root (equivalent to the default . behaviour). No flat-repo test was deleted or weakened.

Action-level plumbing (action.yml → REPO_ROOT env var → --repo-root CLI arg) is exercised by the new identity-monorepo CI job, which runs the action with a real working-directory/repo-root split and uses a concrete fixture (tests/fixtures/identity/monorepo/tools/cfl) with all four packaging types present. The existing identity-interface job is also updated to pass repo-root: tests/fixtures/identity/slck, keeping the validate-and-export path honest.

export-json / validate-and-export paths are not touched by this change (they don't forward repo_root to validate(), and cmd_export_json doesn't accept --repo-root), but this is by design: the PR documents that scope as deliberate, and the existing test_export_json_* tests continue to exercise those paths.

Gaps worth naming

1. cmd_export_json does not accept --repo-root — the diff adds --repo-root only to the validate subcommand. If a monorepo ever calls export-json mode, goreleaser_config will still resolve relative to working_dir, silently producing a spurious "not found" error. There is no unit test asserting that export-json mode behaves correctly in a monorepo layout (nor a failing one documenting the omission as a known limitation). This is the only meaningful untested failure mode introduced by the asymmetry.

2. Absolute --repo-root path — all tests and CI jobs use relative or tmp_path-derived paths. An absolute path (e.g., /workspace) is the normal value in a real CI runner and is not exercised. This is low-risk given os.path.join handles it correctly, but a one-line parametrize would close the gap.

3. Default behaviour (repo_root=".") via the CLI entry point — the unit tests call identity.validate() directly with explicit repo_root values; there is no test that invokes main(["validate", "--working-dir", wd, "--manifest", ...]) without --repo-root to confirm the default wires through correctly. This is a thin gap given the code is straightforward, but it is unexercised at the CLI layer.

None of these gaps block the immediate use case (monorepo validate mode), and the export-json omission is at least consistent with the current scope. Flagging (1) as something to revisit before wider rollout.

[monit-reviewer]

Automated PR Review

Reviewed commit: d49249e

Approved with 2 non-blocking suggestions below. Address at your discretion.

Summary

Reviewer	Findings
harness-engineering:harness-architecture-reviewer	1
harness-engineering:harness-enforcement-reviewer	1

harness-engineering:harness-architecture-reviewer (1 findings)

💡 Suggestion - actions/identity-check/identity.py:257

--repo-root is registered only under the validate subparser. If dispatch logic ever changes, cmd_validate will raise AttributeError on args.repo_root; and callers who try to pass --repo-root to export-json will receive an unrecognized-argument error rather than a clear diagnostic. A brief comment at the subparser registration explaining why export-json intentionally omits --repo-root (goreleaser is a validate-only concern) would prevent both surprises without changing the interface.

harness-engineering:harness-enforcement-reviewer (1 findings)

💡 Suggestion - tests/fixtures/identity/monorepo/tools/cfl/packaging/winget/OpenCLICollective.cfl.installer.yaml:1

Winget installer manifest is a minimal stub (PackageIdentifier only). This is sufficient for identity-check's current validation scope, but if identity-check ever validates additional required winget fields (PackageVersion, Installers, ManifestType, ManifestVersion), the fixture will generate false negatives. Worth noting for future fixture maintainers.

3 info-level observations excluded. Run with --verbose to include.

2 PR discussion threads considered.

---

Completed in 1m 42s | $0.46 | sonnet | daemon 0.2.121 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, database:database-reviewer, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	1m 42s wall · 2m 29s compute (Reviewers: 39s · Synthesis: 1m 01s)
Cost	$0.46
Tokens	118.0k in / 10.2k out
Turns	7

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	31.7k	3.9k	18.6k	13.1k (1h)	$0.12
database:database-reviewer	sonnet	10.7k	111	2.1k	8.6k (1h)	$0.04
harness-engineering:harness-architecture-reviewer	sonnet	16.8k	536	2.1k	14.6k (1h)	$0.07
harness-engineering:harness-enforcement-reviewer	sonnet	16.8k	363	2.1k	14.6k (1h)	$0.07
harness-engineering:harness-knowledge-reviewer	sonnet	16.8k	601	2.1k	14.6k (1h)	$0.07
harness-engineering:harness-self-documenting-code-reviewer	sonnet	10.7k	471	2.1k	8.6k (1h)	$0.04
security:security-code-auditor	haiku	14.6k	4.2k	0	14.6k (1h)	$0.05

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

[monit-reviewer]

🔵 Low (harness-engineering:harness-architecture-reviewer): --repo-root is registered only under the validate subparser. If dispatch logic ever changes, cmd_validate will raise AttributeError on args.repo_root; and callers who try to pass --repo-root to export-json will receive an unrecognized-argument error rather than a clear diagnostic. A brief comment at the subparser registration explaining why export-json intentionally omits --repo-root (goreleaser is a validate-only concern) would prevent both surprises without changing the interface.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-enforcement-reviewer): Winget installer manifest is a minimal stub (PackageIdentifier only). This is sufficient for identity-check's current validation scope, but if identity-check ever validates additional required winget fields (PackageVersion, Installers, ManifestType, ManifestVersion), the fixture will generate false negatives. Worth noting for future fixture maintainers.

Reply to this thread when addressed.