Skip to content

fix: restore workflow consolidation reverted by PR #788#831

Merged
Gkrumbach07 merged 1 commit intomainfrom
fix/restore-workflow-consolidation
Mar 6, 2026
Merged

fix: restore workflow consolidation reverted by PR #788#831
Gkrumbach07 merged 1 commit intomainfrom
fix/restore-workflow-consolidation

Conversation

@Gkrumbach07
Copy link
Copy Markdown
Contributor

Summary

PR #788 (worktree-gemini-runner) was branched from a worktree that independently re-created 12 individual workflow files that PR #781 had consolidated. When #788 was squash-merged, it silently reverted #781's entire consolidation. No subsequent PR caught or fixed this — the repo has been running 27 workflows instead of 16 since March 4.

This PR re-applies #781's consolidation:

  • Delete 12 individual workflow files: ambient-api-server-tests, backend-unit-tests, claude-code-review, claude, cli-tests, frontend-lint, makefile-quality, mermaid-lint, runner-tests, test-e2e-docker-cache, test-go-module-cache, test-registry-cache
  • Restore unit-tests.yml: Consolidated backend, api-server, runner, CLI tests with dorny/paths-filter
  • Rename go-lint.ymllint.yml: Consolidated Go + frontend lint
  • Restore concise workflows README: Removes per-workflow inventory that references deleted files

Changes from original #781

  • Runner test paths updated from claude-code-runnerambient-runner to match current directory structure

Test plan

  • Verify unit-tests.yml triggers on backend/runner/cli/api-server changes
  • Verify lint.yml triggers on frontend and Go component changes
  • Verify path filtering skips unrelated jobs
  • Confirm no branch protection rules reference deleted workflow names

🤖 Generated with Claude Code

PR #788's worktree branch independently re-created 12 individual
workflow files that PR #781 had consolidated, and deleted the two
consolidated replacements (unit-tests.yml, lint.yml). This was never
caught or fixed by subsequent PRs.

Re-applies #781's consolidation (27 → 16 workflows):
- Delete 12 individual workflow files (ambient-api-server-tests,
  backend-unit-tests, claude-code-review, claude, cli-tests,
  frontend-lint, makefile-quality, mermaid-lint, runner-tests,
  test-e2e-docker-cache, test-go-module-cache, test-registry-cache)
- Restore unit-tests.yml (consolidated backend, api-server, runner,
  CLI tests with dorny/paths-filter)
- Rename go-lint.yml → lint.yml (consolidated Go + frontend lint)
- Restore concise workflows README

Updated runner paths from claude-code-runner to ambient-runner to
match the current directory structure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

PR REVIEW PLACEHOLDER - BEING REPLACED

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2026

Claude Code Review

Summary

PR 831 re-applies the workflow consolidation from PR 781 that was silently reverted by PR 788. Deletes 12 workflow files and introduces two consolidated ones (unit-tests.yml and the renamed lint.yml), reducing active workflow count from 27 to 16. The overall approach is sound, but one behavior change in the runner test job could silently break CI.

Issues by Severity — Blocker / Critical: None.

Major Issue: Runner test scope expanded to include all tests

unit-tests.yml, runner job now runs: pytest tests/ -v --tb=short --color=yes -x

The deleted runner-tests.yml scoped to 3 specific test files with the comment: 'Only run standalone unit tests that do not require runner_shell runtime (test_model_mapping.py and test_wrapper_vertex.py require full runtime environment).'

If those two test files still require a runtime environment unavailable in standard CI, the -x flag will cause the runner job to fail on every PR touching the runner. Verify pytest tests/ passes in a vanilla Ubuntu runner, or restore the exclusions: --ignore=tests/test_model_mapping.py --ignore=tests/test_wrapper_vertex.py

Minor Issues

  1. Missing !/*.md exclusion in pull_request paths for unit-tests.yml (push trigger has it, PR trigger does not). Markdown-only PRs will trigger all unit test jobs. Fix: add - '!/*.md' to the pull_request.paths block.

  2. workflow_dispatch inputs (inputs.test_label, inputs.default_namespace) are interpolated directly in shell run: blocks. README security guidance says to use env vars instead. Inherited from deleted backend-unit-tests.yml; low risk but inconsistent with documented standards.

Positive Highlights

  • Clean consolidation: 27 workflows down to 16, well-motivated with clear PR description explaining the root cause.
  • Correct dorny/paths-filter@v3 usage with fine-grained filters matching the actual directory layout.
  • Robust summary job pattern using if: always() across all needs results.
  • Frontend lint (ESLint, tsc --noEmit, npm run build) now integrated into lint.yml alongside Go lint.
  • Runner path correctly updated to components/runners/ambient-runner/**.
  • Concurrency groups correctly renamed.

Recommendations (priority order)

  1. Verify pytest tests/ completes in standard CI before merging; if test_model_mapping.py or test_wrapper_vertex.py fail, restore exclusions.
  2. Add !**/*.md to pull_request.paths in unit-tests.yml.
  3. Wrap workflow_dispatch inputs in env vars in backend configure step.

Reviewed against .claude/context/, .claude/patterns/, and CLAUDE.md standards.

@Gkrumbach07 Gkrumbach07 merged commit 4f0b0a0 into main Mar 6, 2026
17 checks passed
@Gkrumbach07 Gkrumbach07 deleted the fix/restore-workflow-consolidation branch March 6, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant