Conversation
- Create requirements-dev.txt with pinned direct dev/CI dependencies
(anthropic, mcp, pyright, pytest, PyYAML, ruff). Every CI job now
installs via pip install -r requirements-dev.txt followed by pip check,
which validates the resolved dependency graph is internally consistent
and satisfies FEA-107 AC-004.1 (install must succeed without warnings
or missing packages). File header documents the regeneration procedure
for manual bumps.
- Add [tool.pytest.ini_options] to pyproject.toml registering two markers:
contract (repo-level contract tests) and quarantine (flaky, non-blocking).
Registering the markers also silences pytest's PytestUnknownMarkWarning
so future runs are warning-free. Also sets testpaths = ["plugins"].
- Rewrite .github/workflows/ci.yml with six jobs:
- Lint: ruff check .
- Type Check: pyright
- Tests: pytest plugins/ -m 'not quarantine' (quarantined tests are
now excluded from the blocking run)
- Contract tests: pytest plugins/ -m contract, wrapped to treat exit
code 5 (no tests collected) as success -- passes vacuously until
tests are marked
- Quarantined tests (non-blocking): pytest plugins/ -m quarantine with
continue-on-error: true, also handling exit 5 gracefully
- PR gates: aggregator with needs: [lint, typecheck, test, contract-tests]
and if: always(); lets branch protection require one check long-term
instead of four.
- Add a concurrency group so stacked PR updates cancel in-flight runs.
- Add docs/flaky-test-quarantine.md documenting the @pytest.mark.contract
and @pytest.mark.quarantine conventions, how to quarantine/un-quarantine,
and the policy for rolling review.
- Add docs/ci-gates.md documenting the gate matrix and the gh api commands
to update ruleset 13555155 once the new check contexts have been observed
on a real PR run. The ruleset currently has zero required status checks.
Testing: Ran the full suite locally against requirements-dev.txt in a
Python 3.14 venv: pip check clean, ruff check . clean, pyright clean
(0 errors, 0 warnings), pytest plugins/ -m 'not quarantine' = 577 passed,
pytest plugins/ -m contract = exit 5 (wrapper treats as success), same
for -m quarantine.
Risks: The new check contexts (Lint, Type Check, Tests, Contract tests,
Quarantined tests, PR gates) will not appear on existing PRs until the
branch rebuilds. Branch protection ruleset 13555155 currently has zero
required status checks -- none of the CI jobs are gating merges today.
The ruleset must be updated to require the new contexts only after they
have been observed on a real PR run; see docs/ci-gates.md. Pinned
requirements-dev.txt is the minimum viable lockfile; it is bumped manually
rather than via Dependabot.
The quarantine job's run script ended with an unconditional exit 0, forcing the step to succeed even when pytest reported a real failure (rc != 0 and rc != 5). Combined with continue-on-error: true at the job level, real flaky-test breakages would have rendered as silent green checks instead of yellow non-blocking warnings, defeating the gate. The exit 5 (no tests collected) special case is preserved -- it still exits 0 vacuously until quarantined tests exist. All other exit codes now propagate so pytest failures are visible. - Move the unconditional exit 0 inside the rc==5 branch. - Add explicit `exit $rc` for the general case so pytest's exit code flows through. - continue-on-error: true at the job level keeps the gate non-blocking. Note: PIPESTATUS[0] is already used to capture pytest's exit code across the tee pipeline, so this fix only needed to stop masking that captured rc. Testing: - The rc==5 branch is unchanged and still vacuously passes (no quarantined tests yet). - Same fix applied in parallel to closedloop-electron and symphony-alpha. Risks: - None. continue-on-error keeps merges unblocked; the change only restores the failure signal that was being silently dropped.
peterulsteen
reviewed
Apr 9, 2026
wongk
requested changes
Apr 9, 2026
wongk
left a comment
wongk
left a comment
Collaborator
There was a problem hiding this comment.
same comments here regarding contract tests and quarantine.
Aligning with the decision in closedloop-electron PR #94 where reviewers rejected the contract-tests and quarantine mechanisms. Cross-repo standardization was the strongest remaining argument for keeping this scaffolding in claude-plugins; with the reference repo (closedloop- electron) reverting, that argument no longer holds. Reverts: - Remove contract-tests, quarantine, and pr-gates jobs from ci.yml. - Revert the Tests job's -m 'not quarantine' filter back to pytest plugins/. - Remove the contract and quarantine markers from pyproject.toml (the entire [tool.pytest.ini_options] block is removed since testpaths is redundant with the explicit pytest plugins/ in CI). - Remove docs/ci-gates.md and docs/flaky-test-quarantine.md. Kept from the original FEA-107 commit: - requirements-dev.txt pinned lockfile (real improvement, independent of the gate shape). - pip install -r requirements-dev.txt && pip check in each job (install consistency check, satisfies the original AC-004.1 motivation). - The concurrency block that cancels superseded PR runs. Risks: - None. All removed pieces had no current users -- no tests were marked @pytest.mark.contract or @pytest.mark.quarantine, and neither job context was in the branch-protection ruleset.
wongk
approved these changes
Apr 9, 2026
wongk
left a comment
wongk
left a comment
Collaborator
There was a problem hiding this comment.
i believe this repo also does not have code review?
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
3 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.
Testing: Ran the full suite locally against requirements-dev.txt in a Python 3.14 venv: pip check clean, ruff check . clean, pyright clean (0 errors, 0 warnings), pytest plugins/ -m 'not quarantine' = 577 passed, pytest plugins/ -m contract = exit 5 (wrapper treats as success), same for -m quarantine.