Compare commits
20 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| df840855cf | |||
| 53e061bafd | |||
| cfe3ff6755 | |||
| 093945254d | |||
| 625f835aa7 | |||
| be4cd82c37 | |||
| 2d5cb4bb29 | |||
| 1441591e74 | |||
| 848a4294ba | |||
| 496e796cdd | |||
| 642adf4705 | |||
| e842b60ad8 | |||
| 3a246ab553 | |||
| dbfa0fe188 | |||
| b3728c54ce | |||
| 4afada098c | |||
| 6089ec724a | |||
| c6c6e75af6 | |||
| 4e43347b2d | |||
| ec9ddb09a7 |
@@ -381,4 +381,43 @@ python3 -m pytest tests/ -v
|
||||
| `test_python_cli.py` | `close_issue.py` + `mark_issue.py` CLI validation |
|
||||
| `test_mirror_refs.py` | Flags, safety defaults, local integration tests |
|
||||
|
||||
(Core suites — the table is non-exhaustive; see `tests/` for the full set.)
|
||||
|
||||
All tests mock network and keychain access — no real API calls are made.
|
||||
|
||||
For how to write tests — mocking the API/auth safely, testing profile and
|
||||
self-review/self-merge gates, no-secret regression expectations, and unit vs.
|
||||
integration guidance — see
|
||||
[`docs/developer-testing-guidelines.md`](docs/developer-testing-guidelines.md).
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### macOS: `com.apple.provenance` blocks Python execution (#3)
|
||||
|
||||
On macOS Sequoia and later, files written by an agent/IDE terminal receive the
|
||||
`com.apple.provenance` extended attribute, and macOS blocks `Python.app` from
|
||||
**executing** such files. Symptoms: newly created/restored `.py` files fail to
|
||||
run (e.g. `create_issue.py` "vanishing" or refusing to execute), while shell
|
||||
scripts and files created before the session are unaffected. This is a macOS
|
||||
security feature, not a bug in this project's code.
|
||||
|
||||
Workarounds (run from a terminal with **Full Disk Access**, e.g. `Terminal.app`
|
||||
— not the IDE terminal, or the removal itself may be blocked):
|
||||
|
||||
```bash
|
||||
# Preferred: strip only com.apple.provenance under the repo (dry-run first)
|
||||
./scripts/clear-provenance --dry-run
|
||||
./scripts/clear-provenance
|
||||
|
||||
# Or a single file
|
||||
./scripts/clear-provenance /path/to/file.py
|
||||
|
||||
# Manual equivalents
|
||||
xattr -r -d com.apple.provenance /Users/jasonwalker/Development/Gitea-Tools/
|
||||
xattr -cr /Users/jasonwalker/Development/Gitea-Tools/ # clears ALL xattrs
|
||||
```
|
||||
|
||||
Alternatively, grant Full Disk Access to the terminal app in
|
||||
**System Settings → Privacy & Security**. `scripts/clear-provenance` removes only
|
||||
`com.apple.provenance` (leaving other extended attributes intact) and supports
|
||||
`--dry-run`.
|
||||
|
||||
@@ -5,5 +5,5 @@ This document describes how credentials and sensitive environment variables are
|
||||
## Separate Credentials
|
||||
Even though multiple MCP servers share the same monorepo, they **must** have separate credentials and runtimes.
|
||||
|
||||
- **No Shared Environments**: Each MCP server (`gitea-mcp`, `jenkins-mcp`, `ops-mcp`, etc.) must be instantiated as an independent service with its own dedicated `.env` configuration file.
|
||||
- **No Shared Environments**: Each MCP server (`gitea-mcp`, `jenkins-mcp`, `glitchtip-mcp`, `ops-mcp`, etc.) must be instantiated as an independent service with its own dedicated `.env` configuration file.
|
||||
- **Strict Isolation**: A server will only have access to the credentials required for its specific trust boundary. For instance, `gitea-mcp` has no access to Jenkins or Ops authentication tokens.
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
# Developer Testing Guidelines
|
||||
|
||||
How to write and run tests for Gitea-Tools. This guide reflects the current
|
||||
repository behavior and the safety model documented in
|
||||
[`safety-model.md`](safety-model.md),
|
||||
[`credential-isolation.md`](credential-isolation.md), and
|
||||
[`gitea-execution-profiles.md`](gitea-execution-profiles.md).
|
||||
|
||||
Core principle: **tests never make real network calls and never touch real
|
||||
credentials.** Every test mocks the HTTP client and the keychain/auth lookup.
|
||||
|
||||
---
|
||||
|
||||
## 1. Standard test commands
|
||||
|
||||
The test suite needs the project virtualenv (it provides the MCP SDK):
|
||||
|
||||
```bash
|
||||
# From the repository root
|
||||
source venv/bin/activate
|
||||
python3 -m pytest tests/ -q
|
||||
```
|
||||
|
||||
Or invoke the venv interpreter directly without activating:
|
||||
|
||||
```bash
|
||||
./venv/bin/python -m pytest tests/ -q
|
||||
```
|
||||
|
||||
Use `-q` for a compact summary and `-v` to see individual test names.
|
||||
|
||||
### Run the full suite
|
||||
|
||||
```bash
|
||||
./venv/bin/python -m pytest tests/ -q
|
||||
```
|
||||
|
||||
### Run targeted tests
|
||||
|
||||
```bash
|
||||
# One file
|
||||
./venv/bin/python -m pytest tests/test_mcp_server.py -q
|
||||
|
||||
# One class
|
||||
./venv/bin/python -m pytest tests/test_merge_pr.py::TestMergeDisabled -q
|
||||
|
||||
# One test, by node id
|
||||
./venv/bin/python -m pytest tests/test_review_pr.py::TestAPIPayload::test_payload_fields_and_workflow -q
|
||||
|
||||
# By keyword expression
|
||||
./venv/bin/python -m pytest tests/ -q -k "merge and fails_closed"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Syntax and formatting checks
|
||||
|
||||
These are fast and belong in any pre-PR loop:
|
||||
|
||||
```bash
|
||||
# Byte-compile the main modules (catches syntax errors)
|
||||
python3 -m py_compile mcp_server.py
|
||||
python3 -m py_compile manage_labels.py
|
||||
|
||||
# Lint shell scripts without executing them
|
||||
bash -n scripts/clear-provenance
|
||||
|
||||
# Detect stray conflict markers and whitespace errors in the diff
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Run `git diff --check` before every commit; it flags leftover merge-conflict
|
||||
markers and trailing-whitespace/whitespace-error lines.
|
||||
|
||||
---
|
||||
|
||||
## 3. How to add an MCP tool test
|
||||
|
||||
MCP tools live in `mcp_server.py` and are exercised in
|
||||
`tests/test_mcp_server.py`. Tests call the underlying tool function directly
|
||||
with the network layer and auth mocked. The established pattern:
|
||||
|
||||
```python
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
FAKE_AUTH = "Basic ZmFrZTpmYWtl" # not a real credential
|
||||
|
||||
class TestCreateIssue(unittest.TestCase):
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_creates_issue(self, _auth, mock_api):
|
||||
mock_api.return_value = {"number": 1, "html_url": "https://gitea.example.com/issues/1"}
|
||||
|
||||
result = mcp_server.gitea_create_issue(title="Add tests", remote="prgs")
|
||||
|
||||
# Assert on the request the tool would have made
|
||||
mock_api.assert_called_once()
|
||||
method, url = mock_api.call_args[0][0], mock_api.call_args[0][1]
|
||||
self.assertEqual(method, "POST")
|
||||
self.assertIn("/issues", url)
|
||||
```
|
||||
|
||||
Checklist when adding a tool test:
|
||||
|
||||
* Patch `mcp_server.api_request` — never hit the network.
|
||||
* Patch `mcp_server.get_auth_header` to return a fake header — never read the
|
||||
keychain.
|
||||
* Assert on the **method, URL, and payload** the tool builds, and on the shape
|
||||
of the returned payload.
|
||||
* Cover both `dadeschools` and `prgs` remotes when the tool takes `remote`, and
|
||||
confirm the correct host/org/repo are used.
|
||||
* Cover the error path (e.g. `api_request` raising) and confirm the tool
|
||||
surfaces a clear message without leaking secrets.
|
||||
|
||||
---
|
||||
|
||||
## 4. How to mock API requests safely
|
||||
|
||||
* **Always patch `mcp_server.api_request`** (or `gitea_auth.api_request` for the
|
||||
CLI/auth-level tests). No test should open a socket.
|
||||
* **Always patch the auth lookup** (`get_auth_header` / `get_credentials`) and
|
||||
return an obviously fake value. Do not put a real token or password in a test,
|
||||
a fixture, or an environment file.
|
||||
* Prefer asserting on `mock_api.call_args` (method/URL/payload) over asserting on
|
||||
a real response body.
|
||||
* For keychain behavior specifically, see `tests/test_credentials.py`, which
|
||||
mocks the `git credential fill` subprocess (`Popen`) and the environment.
|
||||
|
||||
---
|
||||
|
||||
## 5. How to test profile / allowed-operation failures
|
||||
|
||||
The execution-profile model (see
|
||||
[`gitea-execution-profiles.md`](gitea-execution-profiles.md)) enforces that a
|
||||
tool may only perform operations in its profile's `allowed_operations`, and that
|
||||
`forbidden_operations` always override `allowed_operations`. Mutating tools must
|
||||
**fail closed** when the active profile does not permit the operation.
|
||||
|
||||
When adding or changing a gated tool, add tests that:
|
||||
|
||||
* Configure a profile whose `allowed_operations` does **not** include the
|
||||
requested operation, and assert the tool refuses **without** calling
|
||||
`api_request` (assert `mock_api.assert_not_called()`).
|
||||
* Configure a profile where the operation is in **both** allowed and forbidden,
|
||||
and assert forbidden wins (still refused).
|
||||
* Confirm the refusal message names the missing operation and does not include
|
||||
any secret material.
|
||||
* Confirm the happy path (operation allowed) still reaches `api_request`.
|
||||
|
||||
The guiding assertion is: **no mutation path may reach `api_request` unless the
|
||||
profile/allowed-operation check passed first.**
|
||||
|
||||
---
|
||||
|
||||
## 6. How to test self-review / self-merge gates
|
||||
|
||||
Author-cannot-review and author-cannot-merge are hard safety gates. The merge
|
||||
path is gated (`gitea_merge_pr`), the legacy review wrapper fails closed on
|
||||
`merge=True`, and `gitea_submit_pr_review` never merges. Existing coverage lives
|
||||
in `tests/test_merge_pr.py` and `tests/test_review_pr.py`.
|
||||
|
||||
Patterns to follow (see those files for concrete examples):
|
||||
|
||||
* **Self-merge blocked:** authenticated user == PR author → the tool returns a
|
||||
refusal and **never calls the merge endpoint** (`mock_api.assert_not_called()`
|
||||
or assert no `POST .../merge`).
|
||||
* **Fail-closed inputs:** missing confirmation string, or an unexpected
|
||||
`expected_head_sha`/changed-file set → refuse before any API call.
|
||||
* **Legacy wrapper:** `merge=True` on the review wrapper fails closed and points
|
||||
to the gated workflow, with no API call
|
||||
(`test_merge_flag_fails_closed_without_api_call`).
|
||||
* **Self-approval blocked:** authenticated user == PR author → `approve` /
|
||||
`request_changes` refused.
|
||||
|
||||
Every new gate should have a test proving the mutating endpoint is **not**
|
||||
reached when the gate should block.
|
||||
|
||||
---
|
||||
|
||||
## 7. No-secret / no-token regression expectations
|
||||
|
||||
Secrets must never appear in logs, tool return values, audit records, or test
|
||||
output (see [`safety-model.md`](safety-model.md) §3). The audit module
|
||||
(`gitea_audit.py`) redacts secret-like keys and value prefixes; see
|
||||
`tests/test_audit.py`.
|
||||
|
||||
Expectations for new tests:
|
||||
|
||||
* Assert that token/authorization/password fields are replaced with
|
||||
`gitea_audit.REDACTED` in any structured output or audit record
|
||||
(`test_redacts_secret_keys`, `test_redacts_nested_and_lists`).
|
||||
* Assert that credential-looking substrings in free-text (error messages,
|
||||
reasons) are redacted (`test_redacts_credential_value_prefixes`,
|
||||
`test_metadata_and_reason_redacted`).
|
||||
* Never commit a real token/password, even in a fixture. Use obviously fake
|
||||
values (e.g. `FAKE_AUTH` above).
|
||||
* When a tool returns identity/profile metadata, assert it contains the
|
||||
non-secret fields (username, profile name) and **not** the token.
|
||||
|
||||
There is no third-party secret scanner wired into this repo today; secret safety
|
||||
is enforced by `gitea_audit.redact` plus the regression tests above. A quick
|
||||
manual sweep before a PR:
|
||||
|
||||
```bash
|
||||
# Look for accidentally committed credentials in the diff
|
||||
git diff --cached | grep -nEi "authorization: (basic|bearer)|password|token=[A-Za-z0-9]" || echo "clean"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Unit tests vs. future Docker integration tests
|
||||
|
||||
* **Unit tests (today, default):** fast, fully mocked, no network, no keychain.
|
||||
This is where the vast majority of coverage lives and where new tests should
|
||||
go. They must stay fast and must not require credentials.
|
||||
* **Docker/local-Gitea integration tests (planned, see #66):** opt-in and
|
||||
skipped by default, gated behind an explicit environment variable and run
|
||||
against a pinned, disposable Gitea container. They validate real API behavior
|
||||
(pagination, permissions, label/PR-review endpoints, error payloads) that
|
||||
mocks cannot prove. They must not require production credentials and must not
|
||||
leak tokens.
|
||||
|
||||
Rule of thumb: prove **logic and request-shaping** with unit tests; reserve
|
||||
integration tests for **real-server compatibility**. Do not convert unit tests
|
||||
into network tests.
|
||||
|
||||
---
|
||||
|
||||
## 9. Read-only vs. mutating tool expectations
|
||||
|
||||
* **Read-only tools** (e.g. `gitea_whoami`, `gitea_view_*`, `gitea_list_*`,
|
||||
`gitea_get_profile`): test that they never issue a mutating HTTP method and
|
||||
never require a mutation gate. Assert the request method is `GET`.
|
||||
* **Mutating tools** (create/edit/close/label, review, merge, mirror): test that
|
||||
they (a) pass the profile/allowed-operation gate, (b) honor confirmation and
|
||||
self-action gates, (c) emit an audit record with the authenticated identity
|
||||
and outcome, and (d) fail closed — no `api_request` call — when any gate fails.
|
||||
|
||||
Keep this split explicit in test names and assertions so a reviewer can see, per
|
||||
tool, which category it belongs to and which gates it must respect.
|
||||
@@ -299,14 +299,19 @@ touching anything.
|
||||
- **Prompt:** `Use any eligible merger profile to merge PR #N if checks pass and
|
||||
it is mergeable. Confirm with "MERGE PR N". Do not force-merge.`
|
||||
|
||||
### Close the issue after merge
|
||||
### Close the issue after merge / Reconciliation
|
||||
|
||||
- **Profile:** issue-manager or merger.
|
||||
- **Steps:** verify remote `master` actually contains the merge; close the
|
||||
issue (or rely on a `Closes #N` keyword); release `status:in-progress`;
|
||||
clean up merged branches.
|
||||
- **Prompt:** `After confirming master contains the merge of PR #N, close issue
|
||||
#M and delete the merged branch.`
|
||||
issue; release `status:in-progress` (if it cannot be removed, report why).
|
||||
- **If closed but not merged (`merged=false`):** Stop normal flow. Do not delete worktrees. Compare PR content to remote `master`.
|
||||
- **fully landed:** comment it landed, remove `status:in-progress`, clean up.
|
||||
- **partially landed:** reopen issue, create corrective PR for missing pieces.
|
||||
- **not landed:** reopen issue/PR, do not clean up.
|
||||
- **Direct push to master:** is forbidden except as a documented recovery exception. Final reports must include why, commits, PR metadata, and repaired labels.
|
||||
- **Final reports:** must include both PR metadata (state, merged flag, merge commit) and Git content (remote master hash, expected content present).
|
||||
- **Prompt (normal):** `After confirming master contains the merge of PR #N, close issue #M and delete the merged branch.`
|
||||
- **Prompt (reconcile):** `Reconcile closed-not-merged PR #N by verifying if its content landed on master.`
|
||||
|
||||
### Stop on blocker
|
||||
|
||||
|
||||
@@ -13,3 +13,11 @@ To maintain a secure environment, all secrets, tokens, passwords, and sensitive
|
||||
- System and application logs
|
||||
- Tool return values/outputs
|
||||
- Any form of persistent storage or console output
|
||||
|
||||
## 4. Read-Only First Policy
|
||||
By default, MCP servers (such as `jenkins-mcp` and `ops-mcp`) operate in a **read-only** mode. Mutation capabilities are deny-by-default and fail-closed.
|
||||
|
||||
## 5. Mutation Gating
|
||||
Any mutating action (e.g., Gitea issue creation from GlitchTip, or Jenkins builds) must be explicitly allowed by the execution profile.
|
||||
- **Jenkins build triggers** are explicitly deferred for phase 1.
|
||||
- **GlitchTip to Gitea issue filing** is documented as a gated, orchestrated workflow, not a direct unprompted automatic action.
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
This document defines the strict boundaries between the different MCP server packages within the monorepo.
|
||||
|
||||
The project is named **MCP Control Plane** and lives in the `mcp-control-plane` repository. It groups the following packages: `common`, `gitea-mcp`, `jenkins-mcp`, `ops-mcp`, and `release-mcp`.
|
||||
The project is named **MCP Control Plane** and lives in the `mcp-control-plane` repository. It groups the following packages: `common`, `gitea-mcp`, `jenkins-mcp`, `glitchtip-mcp`, `ops-mcp`, and `release-mcp`.
|
||||
|
||||
## 1. Architectural Philosophy
|
||||
- **One MCP Server per Trust Boundary**: While the packages share a monorepo, their runtime services must remain entirely separate. There is no single "everything" server.
|
||||
@@ -10,4 +10,5 @@ The project is named **MCP Control Plane** and lives in the `mcp-control-plane`
|
||||
## 2. Package-Specific Boundaries
|
||||
- **gitea-mcp**: Restricted to source-control and work-item capabilities (issues, PRs, comments). This package **must not** have Jenkins or Ops credentials, nor can it execute deploy operations.
|
||||
- **jenkins-mcp**: Focused on CI/CD capabilities. This package **must not** have Ops credentials unless explicitly configured for a specific, isolated pipeline later.
|
||||
- **glitchtip-mcp**: Dedicated to observability and error reporting. This package **must not** have Gitea write credentials or Jenkins deploy capabilities.
|
||||
- **ops-mcp**: Dedicated to live environment and host checks. In its initial state, this package starts as strictly read-only (e.g., health checks, status checks, log reading).
|
||||
|
||||
+119
-6
@@ -14,6 +14,7 @@ import datetime
|
||||
import subprocess
|
||||
import urllib.request
|
||||
import urllib.error
|
||||
import urllib.parse
|
||||
from email.utils import parsedate_to_datetime
|
||||
from dotenv import dotenv_values, load_dotenv
|
||||
|
||||
@@ -188,6 +189,39 @@ DEFAULT_MAX_RETRIES = _env_int("GITEA_MAX_RETRIES", 3)
|
||||
DEFAULT_BASE_DELAY = _env_float("GITEA_RETRY_BASE_DELAY", 1.0) # seconds
|
||||
DEFAULT_MAX_DELAY = _env_float("GITEA_RETRY_MAX_DELAY", 60.0) # seconds
|
||||
|
||||
# Per-request socket timeout (seconds). Overridable via environment.
|
||||
DEFAULT_HTTP_TIMEOUT = _env_float("GITEA_HTTP_TIMEOUT", 30.0)
|
||||
|
||||
|
||||
def _redact(text):
|
||||
"""Best-effort strip of credential-like substrings from error text.
|
||||
|
||||
Reuses the audit module's redactor so error messages never surface tokens,
|
||||
Basic/Bearer headers, or password-like values. Falls back to the plain
|
||||
string if the audit helper is unavailable.
|
||||
"""
|
||||
try:
|
||||
from gitea_audit import _redact_str
|
||||
return _redact_str(str(text))
|
||||
except Exception:
|
||||
return str(text)
|
||||
|
||||
|
||||
def _add_query(url, **params):
|
||||
"""Return *url* with the given query parameters added or overridden.
|
||||
|
||||
Preserves any existing query string on *url* (e.g. ``?state=open``) so
|
||||
pagination params can be layered on top of an already-filtered endpoint.
|
||||
"""
|
||||
parts = urllib.parse.urlsplit(url)
|
||||
query = dict(urllib.parse.parse_qsl(parts.query, keep_blank_values=True))
|
||||
for key, value in params.items():
|
||||
query[str(key)] = str(value)
|
||||
new_query = urllib.parse.urlencode(query)
|
||||
return urllib.parse.urlunsplit(
|
||||
(parts.scheme, parts.netloc, parts.path, new_query, parts.fragment)
|
||||
)
|
||||
|
||||
|
||||
def parse_retry_after(value, now=None):
|
||||
"""Parse a ``Retry-After`` header into a non-negative delay in seconds.
|
||||
@@ -239,16 +273,31 @@ def backoff_delay(attempt, base=DEFAULT_BASE_DELAY, cap=DEFAULT_MAX_DELAY, rand=
|
||||
|
||||
def api_request(method, url, auth_header, payload=None, *,
|
||||
max_retries=None, base_delay=None, max_delay=None,
|
||||
timeout=None,
|
||||
sleep_func=time.sleep, rand_func=random.random,
|
||||
now_func=time.time):
|
||||
"""Make an authenticated JSON request to the Gitea API.
|
||||
|
||||
Returns parsed JSON on success, raises ``RuntimeError`` on HTTP errors.
|
||||
Returns parsed JSON on success (or ``None`` for an empty body), and raises
|
||||
``RuntimeError`` on failure.
|
||||
|
||||
On HTTP 429 the request is retried up to *max_retries* times: honoring a
|
||||
valid ``Retry-After`` header (seconds or HTTP-date) when present, otherwise
|
||||
using capped jittered exponential backoff. Non-429 errors and successful
|
||||
responses are unchanged. The ``*_func`` parameters are injection points for
|
||||
using capped jittered exponential backoff. Successful responses are
|
||||
unchanged.
|
||||
|
||||
All failures are converted to a ``RuntimeError`` with a clear, secret
|
||||
-redacted message (no raw stack traces or credential material):
|
||||
|
||||
- Non-429 HTTP errors surface the status code and a redacted response body.
|
||||
502/503/504 upstream errors get an explicit "Gitea upstream unavailable"
|
||||
message.
|
||||
- Timeouts and network/DNS failures (``URLError`` / ``TimeoutError``) surface
|
||||
a generic "network error contacting Gitea" message.
|
||||
- A malformed (non-JSON) success body surfaces a "malformed JSON response"
|
||||
message rather than a raw decode error.
|
||||
|
||||
The ``*_func`` parameters and ``timeout`` are injection points for
|
||||
deterministic testing.
|
||||
"""
|
||||
if max_retries is None:
|
||||
@@ -257,6 +306,8 @@ def api_request(method, url, auth_header, payload=None, *,
|
||||
base_delay = DEFAULT_BASE_DELAY
|
||||
if max_delay is None:
|
||||
max_delay = DEFAULT_MAX_DELAY
|
||||
if timeout is None:
|
||||
timeout = DEFAULT_HTTP_TIMEOUT
|
||||
|
||||
data = json.dumps(payload).encode("utf-8") if payload is not None else None
|
||||
req = urllib.request.Request(url, data=data, method=method)
|
||||
@@ -267,9 +318,8 @@ def api_request(method, url, auth_header, payload=None, *,
|
||||
attempt = 0
|
||||
while True:
|
||||
try:
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
with urllib.request.urlopen(req, timeout=timeout) as resp:
|
||||
body = resp.read().decode("utf-8")
|
||||
return json.loads(body) if body else None
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code == 429 and attempt < max_retries:
|
||||
header = e.headers.get("Retry-After") if e.headers else None
|
||||
@@ -279,8 +329,71 @@ def api_request(method, url, auth_header, payload=None, *,
|
||||
attempt += 1
|
||||
sleep_func(delay)
|
||||
continue
|
||||
try:
|
||||
error_body = e.read().decode("utf-8", errors="replace")
|
||||
raise RuntimeError(f"HTTP {e.code}: {error_body}") from e
|
||||
except Exception:
|
||||
error_body = ""
|
||||
detail = _redact(error_body).strip()
|
||||
if e.code in (502, 503, 504):
|
||||
msg = f"HTTP {e.code}: Gitea upstream unavailable"
|
||||
raise RuntimeError(f"{msg}: {detail}" if detail else msg) from e
|
||||
raise RuntimeError(f"HTTP {e.code}: {detail}") from e
|
||||
except (urllib.error.URLError, TimeoutError) as e:
|
||||
reason = getattr(e, "reason", e)
|
||||
raise RuntimeError(
|
||||
f"network error contacting Gitea: {_redact(reason)}"
|
||||
) from e
|
||||
|
||||
if not body:
|
||||
return None
|
||||
try:
|
||||
return json.loads(body)
|
||||
except ValueError as e:
|
||||
raise RuntimeError("malformed JSON response from Gitea") from e
|
||||
|
||||
|
||||
def api_get_all(url, auth_header, *, limit=None, page_size=50, max_pages=100,
|
||||
**kwargs):
|
||||
"""Fetch a paginated Gitea collection, following page-based pagination.
|
||||
|
||||
Issues successive ``GET`` requests with ``page`` and ``limit`` (per-page)
|
||||
query parameters, accumulating list items until one of:
|
||||
|
||||
- a page returns fewer items than the page size (the last page),
|
||||
- an empty or ``None`` page is returned (also treated as the end — this is
|
||||
how missing/malformed pagination metadata degrades safely),
|
||||
- *limit* total items have been collected, or
|
||||
- *max_pages* pages have been fetched (a safety cap against runaway loops).
|
||||
|
||||
Pagination relies on the *length of each returned page*, not on
|
||||
``X-Total-Count`` / ``Link`` headers, so it tolerates missing or malformed
|
||||
pagination metadata. Returns a list (possibly empty). Raises ``RuntimeError``
|
||||
(via :func:`api_request`) on network/HTTP/malformed failures, or if a page is
|
||||
not a JSON list. Extra ``kwargs`` pass through to :func:`api_request`.
|
||||
"""
|
||||
if page_size < 1:
|
||||
page_size = 1
|
||||
if page_size > 50:
|
||||
page_size = 50 # Gitea caps per-page results at 50
|
||||
if limit is not None and limit < page_size:
|
||||
page_size = max(1, limit)
|
||||
|
||||
results = []
|
||||
for page in range(1, max_pages + 1):
|
||||
page_url = _add_query(url, page=page, limit=page_size)
|
||||
data = api_request("GET", page_url, auth_header, **kwargs)
|
||||
if data is None:
|
||||
break
|
||||
if not isinstance(data, list):
|
||||
raise RuntimeError(
|
||||
f"expected a list page from Gitea, got {type(data).__name__}"
|
||||
)
|
||||
results.extend(data)
|
||||
if limit is not None and len(results) >= limit:
|
||||
return results[:limit]
|
||||
if len(data) < page_size:
|
||||
break
|
||||
return results
|
||||
|
||||
|
||||
def repo_api_url(host, org, repo):
|
||||
|
||||
+78
-17
@@ -4,9 +4,14 @@
|
||||
Auth follows the project convention: credentials are pulled from the macOS
|
||||
keychain via `git credential fill` (HTTPS), then sent as Basic auth.
|
||||
|
||||
Usage:
|
||||
./manage_labels.py # create labels, then apply the mapping below
|
||||
./manage_labels.py --dry # print actions without writing
|
||||
Modes (default = create labels then apply the one-off MAPPING, preserving the
|
||||
original behavior):
|
||||
|
||||
./manage_labels.py # create labels + apply MAPPING
|
||||
./manage_labels.py --create-labels # idempotent label creation only
|
||||
./manage_labels.py --apply-mapping # one-off MAPPING labeling only
|
||||
./manage_labels.py --add-label 42 chore # add one label to one issue
|
||||
./manage_labels.py --dry ... # print actions without writing
|
||||
"""
|
||||
import os
|
||||
import sys
|
||||
@@ -34,7 +39,7 @@ LABELS = [
|
||||
"description": "Issue is being worked on"},
|
||||
]
|
||||
|
||||
# issue number -> label names to apply
|
||||
# issue number -> label names to apply (one-off backfill)
|
||||
MAPPING = {
|
||||
23: ["chore"],
|
||||
22: ["chore"],
|
||||
@@ -56,6 +61,11 @@ MAPPING = {
|
||||
|
||||
BASE_URL = repo_api_url(HOST, ORG, REPO)
|
||||
|
||||
USAGE = (
|
||||
"usage: manage_labels.py [--dry] "
|
||||
"[--create-labels | --apply-mapping | --add-label <issue> <label>]"
|
||||
)
|
||||
|
||||
|
||||
def api(method, path, auth, payload=None):
|
||||
"""Thin wrapper around auth.api_request that prepends BASE_URL and
|
||||
@@ -68,19 +78,15 @@ def api(method, path, auth, payload=None):
|
||||
return None
|
||||
|
||||
|
||||
def main():
|
||||
dry = "--dry" in sys.argv
|
||||
auth = get_auth_header(HOST)
|
||||
if auth is None:
|
||||
print("Could not get credentials from git credential fill",
|
||||
file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
# 1. Existing labels -> name:id
|
||||
def _labels_by_name(auth):
|
||||
"""Return {label name: id} for the repo's existing labels."""
|
||||
existing = api("GET", "/labels?limit=100", auth) or []
|
||||
by_name = {l["name"]: l["id"] for l in existing}
|
||||
return {lb["name"]: lb["id"] for lb in existing}
|
||||
|
||||
# 2. Create missing labels
|
||||
|
||||
def create_labels(auth, dry=False):
|
||||
"""Idempotently create the LABELS set; return the resulting name->id map."""
|
||||
by_name = _labels_by_name(auth)
|
||||
for spec in LABELS:
|
||||
if spec["name"] in by_name:
|
||||
print(f"label exists: {spec['name']}")
|
||||
@@ -92,8 +98,13 @@ def main():
|
||||
if created:
|
||||
by_name[created["name"]] = created["id"]
|
||||
print(f"created label: {created['name']} (id {created['id']})")
|
||||
return by_name
|
||||
|
||||
# 3. Apply mapping
|
||||
|
||||
def apply_mapping(auth, by_name=None, dry=False):
|
||||
"""Apply the one-off MAPPING (PUT replaces each issue's label set)."""
|
||||
if by_name is None:
|
||||
by_name = _labels_by_name(auth)
|
||||
for issue, names in sorted(MAPPING.items(), reverse=True):
|
||||
ids = [by_name[n] for n in names if n in by_name]
|
||||
missing = [n for n in names if n not in by_name]
|
||||
@@ -105,9 +116,59 @@ def main():
|
||||
# PUT replaces the issue's labels with exactly this set (idempotent).
|
||||
res = api("PUT", f"/issues/{issue}/labels", auth, {"labels": ids})
|
||||
if res is not None:
|
||||
applied = [l["name"] for l in res]
|
||||
applied = [lb["name"] for lb in res]
|
||||
print(f"#{issue} labeled: {applied}")
|
||||
|
||||
|
||||
def add_label(auth, issue, label, dry=False):
|
||||
"""Ad-hoc: ADD a single existing label to one issue (append, not replace)."""
|
||||
by_name = _labels_by_name(auth)
|
||||
if label not in by_name:
|
||||
print(f" unknown label '{label}'; create it first (--create-labels)",
|
||||
file=sys.stderr)
|
||||
return False
|
||||
if dry:
|
||||
print(f"[dry] #{issue} += {label}")
|
||||
return True
|
||||
# POST appends to the issue's existing labels (does not replace).
|
||||
res = api("POST", f"/issues/{issue}/labels", auth, {"labels": [by_name[label]]})
|
||||
if res is not None:
|
||||
print(f"#{issue} += {label}")
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def main(argv=None):
|
||||
argv = list(sys.argv[1:] if argv is None else argv)
|
||||
dry = "--dry" in argv or "--dry-run" in argv
|
||||
|
||||
auth = get_auth_header(HOST)
|
||||
if auth is None:
|
||||
print("Could not get credentials from git credential fill",
|
||||
file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
if "--create-labels" in argv:
|
||||
create_labels(auth, dry=dry)
|
||||
elif "--apply-mapping" in argv:
|
||||
apply_mapping(auth, dry=dry)
|
||||
elif "--add-label" in argv:
|
||||
i = argv.index("--add-label")
|
||||
if i + 2 >= len(argv):
|
||||
print(USAGE, file=sys.stderr)
|
||||
sys.exit(2)
|
||||
try:
|
||||
issue = int(argv[i + 1])
|
||||
except ValueError:
|
||||
print(f"--add-label: issue must be a number, got '{argv[i + 1]}'",
|
||||
file=sys.stderr)
|
||||
sys.exit(2)
|
||||
add_label(auth, issue, argv[i + 2], dry=dry)
|
||||
else:
|
||||
# Default (backward compatible): create labels, then apply the mapping.
|
||||
by_name = create_labels(auth, dry=dry)
|
||||
apply_mapping(auth, by_name, dry=dry)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
||||
+116
-6
@@ -14,6 +14,7 @@ Configuration (mcp_config.json):
|
||||
}
|
||||
"""
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import functools
|
||||
import contextlib
|
||||
@@ -37,6 +38,7 @@ from gitea_auth import ( # noqa: E402
|
||||
get_credentials,
|
||||
get_auth_header,
|
||||
api_request,
|
||||
api_get_all,
|
||||
repo_api_url,
|
||||
get_profile,
|
||||
)
|
||||
@@ -48,6 +50,71 @@ mcp = FastMCP("gitea-tools", instructions=(
|
||||
))
|
||||
|
||||
|
||||
def extract_linked_issue_numbers(text: str | None, branch_name: str | None = None) -> list[int]:
|
||||
issues = set()
|
||||
if text:
|
||||
pattern = re.compile(r'(?i)(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|ref[s]?)\s+#(\d+)')
|
||||
issues.update(int(m) for m in pattern.findall(text))
|
||||
if branch_name:
|
||||
pattern = re.compile(r'(?i)issue-(\d+)')
|
||||
issues.update(int(m) for m in pattern.findall(branch_name))
|
||||
return sorted(list(issues))
|
||||
|
||||
def release_in_progress_label(issue_numbers: list[int], remote: str, host: str | None, org: str | None, repo: str | None) -> dict:
|
||||
if not issue_numbers:
|
||||
return {}
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
|
||||
try:
|
||||
labels = api_request("GET", f"{base}/labels?limit=100", auth)
|
||||
label_id = None
|
||||
for lb in labels:
|
||||
if lb["name"] == "status:in-progress":
|
||||
label_id = lb["id"]
|
||||
break
|
||||
except Exception as exc:
|
||||
return {num: f"error fetching repo labels: {_redact(str(exc))}" for num in issue_numbers}
|
||||
|
||||
results = {}
|
||||
if label_id is None:
|
||||
for num in issue_numbers:
|
||||
results[num] = "not present"
|
||||
return results
|
||||
|
||||
for num in issue_numbers:
|
||||
try:
|
||||
url = f"{base}/issues/{num}"
|
||||
issue_data = api_request("GET", url, auth)
|
||||
issue_labels = [lb["name"] for lb in issue_data.get("labels", [])]
|
||||
|
||||
if "status:in-progress" in issue_labels:
|
||||
with _audited("release_in_progress_label", host=h, remote=remote, org=o, repo=r, issue_number=num, request_metadata={"action": "remove status:in-progress"}):
|
||||
api_request("DELETE", f"{url}/labels/{label_id}", auth)
|
||||
results[num] = "released"
|
||||
else:
|
||||
results[num] = "not present"
|
||||
except Exception as exc:
|
||||
results[num] = f"error: {_redact(str(exc))}"
|
||||
|
||||
return results
|
||||
|
||||
def cleanup_in_progress_for_pr(pr_payload: dict, remote: str, host: str | None, org: str | None, repo: str | None) -> dict:
|
||||
body = pr_payload.get("body") or ""
|
||||
title = pr_payload.get("title") or ""
|
||||
branch = pr_payload.get("head", {}).get("ref") or ""
|
||||
|
||||
text = f"{title}\n{body}"
|
||||
issues = extract_linked_issue_numbers(text, branch)
|
||||
|
||||
if not issues:
|
||||
return {"cleanup_status": "no linked issue found"}
|
||||
|
||||
results = release_in_progress_label(issues, remote, host, org, repo)
|
||||
return {"cleanup_status": results}
|
||||
|
||||
# ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
def _resolve(remote: str, host: str | None, org: str | None, repo: str | None):
|
||||
@@ -318,7 +385,7 @@ def gitea_list_prs(
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
url = f"{repo_api_url(h, o, r)}/pulls?state={state}"
|
||||
prs = api_request("GET", url, auth) or []
|
||||
prs = api_get_all(url, auth)
|
||||
return [
|
||||
{
|
||||
"number": pr["number"],
|
||||
@@ -743,6 +810,20 @@ def gitea_edit_pr(
|
||||
with _audited("edit_pr", host=h, remote=remote, org=o, repo=r,
|
||||
pr_number=pr_number, request_metadata={"fields": sorted(payload)}):
|
||||
data = api_request("PATCH", url, auth, payload)
|
||||
|
||||
cleanup_status = None
|
||||
if state == "closed":
|
||||
cleanup = cleanup_in_progress_for_pr(data, remote, host, org, repo)
|
||||
cleanup_status = cleanup.get("cleanup_status")
|
||||
if isinstance(cleanup_status, dict):
|
||||
for issue_num, st in cleanup_status.items():
|
||||
if st == "released":
|
||||
try:
|
||||
comment_url = f"{repo_api_url(h, o, r)}/issues/{issue_num}/comments"
|
||||
api_request("POST", comment_url, auth, {"body": f"Tracker cleanup: removed `status:in-progress` from this issue because linked PR #{pr_number} was closed."})
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"number": data["number"],
|
||||
@@ -750,6 +831,7 @@ def gitea_edit_pr(
|
||||
"body": data.get("body", ""),
|
||||
"state": data["state"],
|
||||
"url": data["html_url"],
|
||||
"cleanup_status": cleanup_status,
|
||||
}
|
||||
|
||||
|
||||
@@ -1021,6 +1103,9 @@ def gitea_merge_pr(
|
||||
"GET", f"{repo_api_url(h, o, r)}/pulls/{pr_number}", auth
|
||||
)
|
||||
result["merge_commit"] = (merged or {}).get("merged_commit_sha")
|
||||
|
||||
cleanup = cleanup_in_progress_for_pr(merged or {}, remote, host, org, repo)
|
||||
result["cleanup_status"] = cleanup.get("cleanup_status")
|
||||
except Exception:
|
||||
result["merge_commit"] = None
|
||||
except Exception as exc: # noqa: BLE001 — redact before surfacing
|
||||
@@ -1157,7 +1242,14 @@ def gitea_close_issue(
|
||||
with _audited("close_issue", host=h, remote=remote, org=o, repo=r,
|
||||
issue_number=issue_number, request_metadata={"state": "closed"}):
|
||||
api_request("PATCH", url, auth, {"state": "closed"})
|
||||
return {"success": True, "message": f"Issue #{issue_number} closed."}
|
||||
|
||||
cleanup_result = release_in_progress_label([issue_number], remote, host, org, repo)
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": f"Issue #{issue_number} closed.",
|
||||
"cleanup_status": cleanup_result
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
@@ -1175,7 +1267,7 @@ def gitea_list_issues(
|
||||
Args:
|
||||
state: Filter by state — 'open', 'closed', or 'all'.
|
||||
label: Filter by label name (e.g. 'important').
|
||||
limit: Max number of issues to return (default: 50).
|
||||
limit: Max number of issues to return across all pages (default: 50).
|
||||
remote: Known instance — 'dadeschools' or 'prgs'.
|
||||
host: Override the Gitea host.
|
||||
org: Override the owner/organization.
|
||||
@@ -1186,11 +1278,11 @@ def gitea_list_issues(
|
||||
"""
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
params = f"state={state}&limit={limit}&type=issues"
|
||||
params = f"state={state}&type=issues"
|
||||
if label:
|
||||
params += f"&labels={label}"
|
||||
url = f"{repo_api_url(h, o, r)}/issues?{params}"
|
||||
issues = api_request("GET", url, auth)
|
||||
issues = api_get_all(url, auth, limit=limit)
|
||||
return [
|
||||
{
|
||||
"number": i["number"],
|
||||
@@ -1293,6 +1385,24 @@ def gitea_whoami(
|
||||
}
|
||||
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_get_authenticated_user(
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
) -> dict:
|
||||
"""Alias for gitea_whoami. Look up the authenticated Gitea account."""
|
||||
return gitea_whoami(remote=remote, host=host)
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_get_current_user(
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
) -> dict:
|
||||
"""Alias for gitea_whoami. Look up the authenticated Gitea account."""
|
||||
return gitea_whoami(remote=remote, host=host)
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_get_profile(
|
||||
remote: str = "dadeschools",
|
||||
@@ -1445,7 +1555,7 @@ def gitea_list_labels(
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
return api_request("GET", f"{base}/labels?limit=100", auth)
|
||||
return api_get_all(f"{base}/labels", auth)
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
|
||||
Executable
+61
@@ -0,0 +1,61 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
# clear-provenance — strip the macOS com.apple.provenance extended attribute so
|
||||
# Python.app can execute .py files created by agent/IDE terminals (issue #3).
|
||||
#
|
||||
# macOS Sequoia+ blocks Python.app from executing files carrying
|
||||
# com.apple.provenance. Files written by the agent terminal get it; shell
|
||||
# scripts are unaffected. This is a macOS security feature, not a bug in our
|
||||
# code — see the Troubleshooting section of the README.
|
||||
#
|
||||
# Run from a terminal with Full Disk Access (e.g. Terminal.app), not the IDE
|
||||
# terminal, or the removal itself may be blocked.
|
||||
|
||||
usage() {
|
||||
cat <<'EOF'
|
||||
usage: scripts/clear-provenance [--dry-run] [path]
|
||||
|
||||
Recursively remove the com.apple.provenance extended attribute under <path>
|
||||
(default: the repository root). macOS only. Only that attribute is removed;
|
||||
other extended attributes are left intact.
|
||||
|
||||
Examples:
|
||||
scripts/clear-provenance --dry-run
|
||||
scripts/clear-provenance
|
||||
scripts/clear-provenance /path/to/file.py
|
||||
EOF
|
||||
}
|
||||
|
||||
dry_run=0
|
||||
while [[ "${1:-}" == --* ]]; do
|
||||
case "$1" in
|
||||
--dry-run) dry_run=1 ;;
|
||||
--help) usage; exit 0 ;;
|
||||
*) usage >&2; exit 2 ;;
|
||||
esac
|
||||
shift
|
||||
done
|
||||
|
||||
if [[ $# -gt 1 ]]; then
|
||||
usage >&2
|
||||
exit 2
|
||||
fi
|
||||
|
||||
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
repo_root="$(cd "$script_dir/.." && pwd)"
|
||||
target="${1:-$repo_root}"
|
||||
|
||||
if [[ ! -e "$target" ]]; then
|
||||
printf 'clear-provenance: no such path: %s\n' "$target" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Remove only com.apple.provenance; tolerate files that do not carry it.
|
||||
if [[ "$dry_run" -eq 1 ]]; then
|
||||
printf 'clear-provenance: [dry-run] would run: xattr -r -d com.apple.provenance %q\n' "$target"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
xattr -r -d com.apple.provenance "$target" 2>/dev/null || true
|
||||
printf 'clear-provenance: removed com.apple.provenance recursively under: %s\n' "$target"
|
||||
@@ -19,6 +19,14 @@ identity, and cleaned up only after a real merge.
|
||||
|
||||
---
|
||||
|
||||
|
||||
## Definitions
|
||||
|
||||
- **Merged**: Gitea PR metadata says `merged=true`.
|
||||
- **Landed**: Equivalent content is present on remote `master`, but PR metadata may not say merged.
|
||||
- **Closed-not-merged**: PR state is closed and `merged=false`.
|
||||
- **Reconciled**: A human/LLM verified whether closed-not-merged content landed, partially landed, or was lost, and repaired issue/label/tracker state.
|
||||
|
||||
## A. Issue-first rule
|
||||
|
||||
**No repository change without a tracking issue.** This includes creating,
|
||||
@@ -133,6 +141,14 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
10. Push the branch.
|
||||
11. Open a PR to `master`.
|
||||
12. **If you are the author, stop before review/merge.**
|
||||
13. **Normal issue work must not directly push to `master`.** PR content should be merged through the forge PR merge mechanism.
|
||||
14. Direct push to `master` is allowed only as a documented recovery exception. If used, the final report must include:
|
||||
- why the PR merge path could not be used
|
||||
- exact commits pushed
|
||||
- PR metadata state
|
||||
- issue labels/state repaired
|
||||
- whether the PR is closed-not-merged
|
||||
|
||||
|
||||
## F. Review workflow
|
||||
|
||||
@@ -148,13 +164,15 @@ Worktree folder = branch with `/` replaced by `-`
|
||||
|
||||
Only an eligible (non-author) reviewer merges. After a real merge:
|
||||
|
||||
1. Confirm remote `master` actually contains the merge commit.
|
||||
2. Close/release the issue; remove `status:in-progress` if used.
|
||||
3. Delete the remote branch.
|
||||
4. Remove the local branch.
|
||||
5. Remove the branch worktree folder (`scripts/worktree-clean --delete-branch <branch>`).
|
||||
6. Fetch/prune.
|
||||
7. Confirm the main checkout is clean and current (`0 0` vs remote).
|
||||
1. Confirm remote `master` actually contains the merge commit (A PR is not done just because `master` moved. A PR is done only when: Gitea reports the PR merged or reconciliation documents equivalent content on `master`; remote `master` contains the expected content; linked issues are closed; `status:in-progress` is removed).
|
||||
2. Close/release the issue.
|
||||
3. Whenever an issue is closed, check for `status:in-progress`: remove it, or report why it could not be removed.
|
||||
4. Do not delete the remote source branch until: PR `merged=true`, or reconciliation confirms content is safely landed, or the issue owner explicitly abandons the work.
|
||||
5. Remove the local branch.
|
||||
6. Remove the branch worktree folder (`scripts/worktree-clean --delete-branch <branch>`). Branches/worktrees are cleaned only after the above is verified.
|
||||
7. Fetch/prune.
|
||||
8. Confirm the main checkout is clean and current (`0 0` vs remote).
|
||||
9. Final merge/reconciliation reports must include both: PR metadata (state, merged flag, merge commit/hash) and Git content (remote master hash, expected content present or not).
|
||||
|
||||
Never run cleanup before the merge is confirmed on remote `master`.
|
||||
|
||||
@@ -165,7 +183,11 @@ Never run cleanup before the merge is confirmed on remote `master`.
|
||||
- No issue exists and one cannot be created.
|
||||
- Worktree state is unclear or unexpected.
|
||||
- Branch/PR state conflicts with the prompt (e.g. prompt says "merged" but it is not).
|
||||
- A PR is closed but not merged.
|
||||
- A PR is closed but not merged (closed with `merged=false`). In this case:
|
||||
- stop normal review/merge
|
||||
- do not delete branches/worktrees
|
||||
- do not start dependent work
|
||||
- run reconciliation
|
||||
- Local `master` is ahead of remote unexpectedly.
|
||||
- The authenticated user is the PR author (for review/merge).
|
||||
- Secrets/tokens appear in the diff.
|
||||
@@ -182,9 +204,10 @@ When in doubt, stop and surface the discrepancy; do not guess or work around a g
|
||||
the commits are preserved on a feature branch (local + remote) first, then
|
||||
`git reset --hard <remote>/master` to realign. Never discard commits that are
|
||||
not safely pushed elsewhere.
|
||||
- **PR closed but not merged:** the work is not in mainline. Re-push the branch,
|
||||
reopen (or open a replacement) PR, and let an eligible reviewer merge. Do not
|
||||
assume "closed" means "merged" — verify remote `master` contains the commits.
|
||||
- **PR closed but not merged (`merged=false`):** do not merge. Run reconciliation: compare PR content to remote `master` and decide:
|
||||
- **fully landed:** comment that content is present on `master`, remove `status:in-progress`, keep/close issue as appropriate, clean up only after content equivalence is confirmed.
|
||||
- **partially landed:** do not clean up, reopen issue if needed, create corrective issue/PR for missing pieces.
|
||||
- **not landed:** reopen issue if needed, reopen PR or create replacement PR, do not clean up source branch/worktree.
|
||||
- **Branch deleted before merge:** if the commits still exist locally (a branch or
|
||||
reflog), re-push them and reopen the PR; otherwise recover via
|
||||
`git fsck --lost-found`. Preserve first, then proceed.
|
||||
@@ -203,6 +226,7 @@ Ready-to-copy templates live in [`templates/`](templates/):
|
||||
- [`review-pr.md`](templates/review-pr.md) — review a PR.
|
||||
- [`merge-pr.md`](templates/merge-pr.md) — merge a PR (eligible reviewer only).
|
||||
- [`recover-bad-state.md`](templates/recover-bad-state.md) — recover from bad state.
|
||||
- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md) — reconcile a closed-not-merged PR.
|
||||
- [`worktree-cleanup.md`](templates/worktree-cleanup.md) — clean up after merge.
|
||||
- [`release-tag.md`](templates/release-tag.md) — create a release tag.
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ Rules (llm-project-workflow):
|
||||
author → STOP.
|
||||
- Do not merge unless the PR is open, mergeable, and its checks/review pass.
|
||||
- No force-merge, no bypassing branch protections.
|
||||
- If the PR is closed but `merged=false`, STOP and run reconciliation. Do not clean up.
|
||||
|
||||
Steps:
|
||||
1. Verify authenticated identity + active profile.
|
||||
@@ -20,9 +21,9 @@ Steps:
|
||||
5. Confirm remote master now contains the merge commit.
|
||||
|
||||
Then run the cleanup template (worktree-cleanup.md):
|
||||
- close/release issue #<n>, remove status:in-progress
|
||||
- close/release issue #<n>, remove status:in-progress (if it cannot be removed, report why)
|
||||
- delete remote branch, remove local branch + worktree folder
|
||||
- fetch/prune; confirm main checkout is clean and current (0 0).
|
||||
|
||||
Handoff: reviewer identity, merge result + commit, cleanup done, issue closed.
|
||||
Handoff: reviewer identity, merge result + commit, cleanup done, issue closed, PR metadata state/merged flag/hash, remote master hash & Git content check.
|
||||
```
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
# Reconcile Closed-Not-Merged PR Prompt
|
||||
|
||||
You are reconciling PR `<pr-number>` in `<repo-name>` which is closed but `merged=false`.
|
||||
|
||||
Rules:
|
||||
|
||||
- Do not delete branches or worktrees before reconciliation is complete.
|
||||
- Compare the PR's exact content to remote `<default-branch>`.
|
||||
- Determine if the content is fully landed, partially landed, or not landed.
|
||||
|
||||
Workflow:
|
||||
|
||||
1. Verify the PR metadata says `state=closed` and `merged=false`.
|
||||
2. Fetch/prune and inspect remote `<default-branch>`.
|
||||
3. If fully landed: comment that it landed, remove `status:in-progress`, close issue, and clean up.
|
||||
4. If partially landed: reopen issue if needed, create corrective PR for missing pieces, do not clean up.
|
||||
5. If not landed: reopen issue/PR, do not clean up.
|
||||
|
||||
Final handoff:
|
||||
|
||||
- PR metadata (state, merged flag, hash)
|
||||
- Git content verification (remote master hash, expected content present or not)
|
||||
- reconciliation decision (fully/partially/not landed)
|
||||
- issue/label state repaired
|
||||
@@ -22,8 +22,7 @@ Act per case:
|
||||
- Local master ahead of remote: confirm the extra commits live on a branch
|
||||
pushed to <remote>, THEN git reset --hard <remote>/master. Verify with
|
||||
`git branch --contains <sha>` first.
|
||||
- PR closed but not merged: re-push the branch, reopen/replace the PR, let an
|
||||
eligible reviewer merge. Do not merge your own.
|
||||
- PR closed but not merged (`merged=false`): stop normal flow and use reconcile-closed-not-merged-pr.md instead.
|
||||
- Branch deleted before merge: recover commits from a local branch/reflog (or
|
||||
git fsck --lost-found), re-push, reopen the PR.
|
||||
- Unauthorized untracked file: do not commit it; leave pre-existing artifacts.
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
# Recover Dirty Worktree Prompt
|
||||
|
||||
You are recovering repository state in `<repo-name>`.
|
||||
|
||||
Rules:
|
||||
|
||||
- Do not reset, delete, clean, or overwrite work unless explicitly instructed.
|
||||
- Do not edit another issue's worktree unless assigned to that issue.
|
||||
- Preserve ambiguous work before any destructive operation.
|
||||
|
||||
Workflow:
|
||||
|
||||
1. Run `git status --short --branch`.
|
||||
2. Identify whether dirty files belong to the current issue, another issue, or
|
||||
unknown work.
|
||||
3. If dirty work belongs to another issue, leave it alone and use a separate
|
||||
worktree for the current task.
|
||||
4. If an unauthorized untracked file was created, stop and report its exact path.
|
||||
5. Remove unauthorized files only when explicitly instructed.
|
||||
6. If local `<default-branch>` is ahead of `<remote>/<default-branch>`, stop and
|
||||
report both commit hashes.
|
||||
7. If cleanup is requested, verify the branch is merged or explicitly abandoned
|
||||
before deleting any branch or worktree.
|
||||
|
||||
Report:
|
||||
|
||||
- current branch
|
||||
- dirty files
|
||||
- ownership assessment
|
||||
- actions taken
|
||||
- remaining blockers
|
||||
@@ -0,0 +1,205 @@
|
||||
"""Unit coverage for shared API pagination and failure handling (#67).
|
||||
|
||||
Covers gitea_auth.api_request failure conversion (timeouts, DNS/network,
|
||||
502/503, malformed error payloads, malformed success JSON, no-secret leakage,
|
||||
preserved success + 429 behavior) and gitea_auth.api_get_all pagination
|
||||
(single/multi page, missing/malformed metadata, limit cap, max_pages, query
|
||||
handling). Everything is mocked — no real network calls are made.
|
||||
"""
|
||||
import io
|
||||
import unittest
|
||||
import urllib.error
|
||||
from unittest.mock import patch
|
||||
|
||||
import gitea_auth
|
||||
import gitea_audit
|
||||
|
||||
FAKE_AUTH = "Basic ZmFrZTpmYWtl" # not a real credential
|
||||
URL = "https://gitea.example.com/api/v1/repos/o/r/issues"
|
||||
|
||||
|
||||
class FakeResp:
|
||||
"""Minimal context-manager stand-in for a urlopen response."""
|
||||
|
||||
def __init__(self, body):
|
||||
self._body = body.encode("utf-8") if isinstance(body, str) else body
|
||||
|
||||
def read(self):
|
||||
return self._body
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc):
|
||||
return False
|
||||
|
||||
|
||||
def http_error(code, body="", headers=None):
|
||||
return urllib.error.HTTPError(
|
||||
url=URL, code=code, msg="err",
|
||||
hdrs=headers or {}, fp=io.BytesIO(body.encode("utf-8")),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# api_request — success path preserved
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestApiRequestSuccess(unittest.TestCase):
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_success_returns_parsed_json(self, mock_open):
|
||||
mock_open.return_value = FakeResp('{"number": 1, "title": "ok"}')
|
||||
self.assertEqual(
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH),
|
||||
{"number": 1, "title": "ok"},
|
||||
)
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_empty_body_returns_none(self, mock_open):
|
||||
mock_open.return_value = FakeResp("")
|
||||
self.assertIsNone(gitea_auth.api_request("GET", URL, FAKE_AUTH))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_429_then_success_still_retries(self, mock_open):
|
||||
mock_open.side_effect = [http_error(429), FakeResp('{"ok": true}')]
|
||||
calls = []
|
||||
result = gitea_auth.api_request(
|
||||
"GET", URL, FAKE_AUTH,
|
||||
sleep_func=lambda d: calls.append(d), rand_func=lambda: 0.0,
|
||||
)
|
||||
self.assertEqual(result, {"ok": True})
|
||||
self.assertEqual(len(calls), 1) # slept once between the two attempts
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# api_request — failure handling
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestApiRequestFailures(unittest.TestCase):
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_timeout_converted_to_runtimeerror(self, mock_open):
|
||||
mock_open.side_effect = TimeoutError("timed out")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertIn("network error contacting Gitea", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_dns_network_failure_converted(self, mock_open):
|
||||
mock_open.side_effect = urllib.error.URLError("Name or service not known")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertIn("network error contacting Gitea", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_502_upstream_message(self, mock_open):
|
||||
mock_open.side_effect = http_error(502, "bad gateway")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
msg = str(ctx.exception)
|
||||
self.assertIn("HTTP 502", msg)
|
||||
self.assertIn("upstream unavailable", msg)
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_503_upstream_message(self, mock_open):
|
||||
mock_open.side_effect = http_error(503, "")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertIn("HTTP 503", str(ctx.exception))
|
||||
self.assertIn("upstream unavailable", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_malformed_error_payload_does_not_crash(self, mock_open):
|
||||
# Non-JSON garbage error body must still yield a clean RuntimeError.
|
||||
mock_open.side_effect = http_error(500, "<html>garbage</html>")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertIn("HTTP 500", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_malformed_success_json_raises_clean_error(self, mock_open):
|
||||
mock_open.return_value = FakeResp("not json{")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertIn("malformed JSON response", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_no_secret_leak_in_error_body(self, mock_open):
|
||||
mock_open.side_effect = http_error(
|
||||
400, "failed: token supersecret123 rejected")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
msg = str(ctx.exception)
|
||||
self.assertNotIn("supersecret123", msg)
|
||||
self.assertIn(gitea_audit.REDACTED, msg)
|
||||
|
||||
@patch("gitea_auth.urllib.request.urlopen")
|
||||
def test_auth_header_never_in_error(self, mock_open):
|
||||
mock_open.side_effect = http_error(400, "bad request")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_request("GET", URL, FAKE_AUTH)
|
||||
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# api_get_all — pagination
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestApiGetAll(unittest.TestCase):
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_single_page(self, mock_req):
|
||||
mock_req.return_value = [{"id": 1}, {"id": 2}] # short page (< page_size)
|
||||
result = gitea_auth.api_get_all(URL, FAKE_AUTH, page_size=50)
|
||||
self.assertEqual(result, [{"id": 1}, {"id": 2}])
|
||||
self.assertEqual(mock_req.call_count, 1)
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_multi_page(self, mock_req):
|
||||
mock_req.side_effect = [
|
||||
[{"id": 1}, {"id": 2}], # full page
|
||||
[{"id": 3}, {"id": 4}], # full page
|
||||
[{"id": 5}], # short page -> stop
|
||||
]
|
||||
result = gitea_auth.api_get_all(URL, FAKE_AUTH, page_size=2)
|
||||
self.assertEqual([r["id"] for r in result], [1, 2, 3, 4, 5])
|
||||
self.assertEqual(mock_req.call_count, 3)
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_missing_metadata_none_page_ends(self, mock_req):
|
||||
mock_req.return_value = None # empty/malformed metadata -> treated as end
|
||||
self.assertEqual(gitea_auth.api_get_all(URL, FAKE_AUTH), [])
|
||||
self.assertEqual(mock_req.call_count, 1)
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_malformed_metadata_non_list_raises(self, mock_req):
|
||||
mock_req.return_value = {"message": "not a list"}
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
gitea_auth.api_get_all(URL, FAKE_AUTH)
|
||||
self.assertIn("expected a list page", str(ctx.exception))
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_limit_caps_results(self, mock_req):
|
||||
mock_req.side_effect = [[{"id": 1}, {"id": 2}], [{"id": 3}, {"id": 4}]]
|
||||
result = gitea_auth.api_get_all(URL, FAKE_AUTH, page_size=2, limit=3)
|
||||
self.assertEqual([r["id"] for r in result], [1, 2, 3])
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_max_pages_safety_cap(self, mock_req):
|
||||
mock_req.side_effect = [
|
||||
[{"id": 1}, {"id": 2}], [{"id": 3}, {"id": 4}], [{"id": 5}, {"id": 6}],
|
||||
]
|
||||
result = gitea_auth.api_get_all(URL, FAKE_AUTH, page_size=2, max_pages=2)
|
||||
self.assertEqual(len(result), 4)
|
||||
self.assertEqual(mock_req.call_count, 2)
|
||||
|
||||
@patch("gitea_auth.api_request")
|
||||
def test_query_params_appended_and_preserved(self, mock_req):
|
||||
mock_req.return_value = [] # first (empty) page ends immediately
|
||||
gitea_auth.api_get_all(URL + "?state=open", FAKE_AUTH, page_size=2)
|
||||
called_url = mock_req.call_args[0][1]
|
||||
self.assertIn("state=open", called_url)
|
||||
self.assertIn("page=1", called_url)
|
||||
self.assertIn("limit=2", called_url)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,60 @@
|
||||
"""Tests for scripts/clear-provenance (#3).
|
||||
|
||||
Exercises argument handling and the inert --dry-run path only — no real xattr
|
||||
mutation, no network. (Actually removing com.apple.provenance is macOS-only and
|
||||
has real side effects, so it is not exercised here.)
|
||||
"""
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent
|
||||
SCRIPT = REPO / "scripts" / "clear-provenance"
|
||||
|
||||
|
||||
def run(*args):
|
||||
proc = subprocess.run(["bash", str(SCRIPT), *args],
|
||||
capture_output=True, text=True, cwd=str(REPO))
|
||||
return proc.returncode, proc.stdout, proc.stderr
|
||||
|
||||
|
||||
class TestClearProvenance(unittest.TestCase):
|
||||
|
||||
def test_dry_run_defaults_to_repo_root(self):
|
||||
rc, out, _ = run("--dry-run")
|
||||
self.assertEqual(rc, 0)
|
||||
self.assertIn("would run: xattr -r -d com.apple.provenance", out)
|
||||
self.assertIn(str(REPO), out)
|
||||
|
||||
def test_dry_run_explicit_path(self):
|
||||
with tempfile.TemporaryDirectory() as d:
|
||||
f = Path(d) / "x.py"
|
||||
f.write_text("print('hi')\n")
|
||||
rc, out, _ = run("--dry-run", str(f))
|
||||
self.assertEqual(rc, 0)
|
||||
self.assertIn(str(f), out)
|
||||
|
||||
def test_missing_path_errors(self):
|
||||
rc, _, err = run("--dry-run", "/no/such/path-xyz")
|
||||
self.assertEqual(rc, 1)
|
||||
self.assertIn("no such path", err)
|
||||
|
||||
def test_bad_flag_exit_2(self):
|
||||
rc, _, _ = run("--bogus")
|
||||
self.assertEqual(rc, 2)
|
||||
|
||||
def test_too_many_args_exit_2(self):
|
||||
rc, _, _ = run("a", "b")
|
||||
self.assertEqual(rc, 2)
|
||||
|
||||
def test_only_targets_provenance_attribute(self):
|
||||
# The command removes only com.apple.provenance, not all xattrs.
|
||||
rc, out, _ = run("--dry-run")
|
||||
self.assertIn("com.apple.provenance", out)
|
||||
self.assertNotIn("xattr -rc", out) # not a blanket "clear all"
|
||||
self.assertNotIn("-c ", out)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -137,5 +137,89 @@ class TestConstants(unittest.TestCase):
|
||||
f"Label '{label['name']}' has invalid color")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Modes: --create-labels / --apply-mapping / --add-label (#6)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestModes(unittest.TestCase):
|
||||
|
||||
def _methods(self, mock_api):
|
||||
return [(c[0][0], c[0][1]) for c in mock_api.call_args_list]
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_create_labels_only_no_mapping(self, mock_api, _auth):
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return [] # no existing labels
|
||||
if method == "POST" and path == "/labels":
|
||||
return {"id": 1, "name": payload["name"]}
|
||||
return None
|
||||
mock_api.side_effect = se
|
||||
manage_labels.main(["--create-labels"])
|
||||
methods = self._methods(mock_api)
|
||||
self.assertTrue(any(m == ("POST", "/labels") for m in methods))
|
||||
self.assertFalse(any(m[0] == "PUT" for m in methods)) # no mapping applied
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_apply_mapping_only_no_label_creation(self, mock_api, _auth):
|
||||
existing = [_make_label(l["name"], i + 1)
|
||||
for i, l in enumerate(manage_labels.LABELS)]
|
||||
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return existing
|
||||
if method == "PUT":
|
||||
return [{"name": "applied"}]
|
||||
return None
|
||||
mock_api.side_effect = se
|
||||
manage_labels.main(["--apply-mapping"])
|
||||
methods = self._methods(mock_api)
|
||||
self.assertFalse(any(m == ("POST", "/labels") for m in methods))
|
||||
put_calls = [m for m in methods if m[0] == "PUT"]
|
||||
self.assertEqual(len(put_calls), len(manage_labels.MAPPING))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_appends_to_issue(self, mock_api, _auth):
|
||||
existing = [_make_label("chore", 5)]
|
||||
|
||||
def se(method, path, auth, payload=None):
|
||||
if method == "GET":
|
||||
return existing
|
||||
if method == "POST":
|
||||
return [{"name": "chore"}]
|
||||
return None
|
||||
mock_api.side_effect = se
|
||||
manage_labels.main(["--add-label", "42", "chore"])
|
||||
posts = [c for c in mock_api.call_args_list
|
||||
if c[0][0] == "POST" and c[0][1] == "/issues/42/labels"]
|
||||
self.assertEqual(len(posts), 1)
|
||||
self.assertEqual(posts[0][0][3], {"labels": [5]}) # append, id 5
|
||||
# POST appends; no PUT (which would replace the whole set).
|
||||
self.assertFalse(any(c[0][0] == "PUT" for c in mock_api.call_args_list))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_unknown_makes_no_write(self, mock_api, _auth):
|
||||
mock_api.side_effect = lambda *a, **k: [] if a[0] == "GET" else None
|
||||
manage_labels.main(["--add-label", "42", "ghost"])
|
||||
# Only the GET label lookup; no POST/PUT for an undefined label.
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_dry_makes_no_write(self, mock_api, _auth):
|
||||
mock_api.side_effect = lambda *a, **k: [_make_label("chore", 5)] if a[0] == "GET" else None
|
||||
manage_labels.main(["--dry", "--add-label", "42", "chore"])
|
||||
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list))
|
||||
|
||||
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
|
||||
@patch("manage_labels.api")
|
||||
def test_add_label_non_numeric_issue_exits(self, mock_api, _auth):
|
||||
with self.assertRaises(SystemExit):
|
||||
manage_labels.main(["--add-label", "notanum", "chore"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
+255
-7
@@ -93,7 +93,8 @@ class TestCloseIssue(unittest.TestCase):
|
||||
result = gitea_close_issue(issue_number=42)
|
||||
self.assertTrue(result["success"])
|
||||
self.assertIn("42", result["message"])
|
||||
payload = mock_api.call_args[0][3]
|
||||
patch_call = next(call for call in mock_api.call_args_list if call[0][0] == "PATCH")
|
||||
payload = patch_call[0][3]
|
||||
self.assertEqual(payload["state"], "closed")
|
||||
|
||||
|
||||
@@ -102,7 +103,7 @@ class TestCloseIssue(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestListIssues(unittest.TestCase):
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_returns_formatted_list(self, _auth, mock_api):
|
||||
mock_api.return_value = [
|
||||
@@ -123,20 +124,20 @@ class TestListIssues(unittest.TestCase):
|
||||
self.assertEqual(result[0]["assignee"], "alice")
|
||||
self.assertEqual(result[1]["assignee"], "")
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_passes_label_filter(self, _auth, mock_api):
|
||||
mock_api.return_value = []
|
||||
gitea_list_issues(label="important")
|
||||
url = mock_api.call_args[0][1]
|
||||
url = mock_api.call_args[0][0]
|
||||
self.assertIn("labels=important", url)
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_passes_state_filter(self, _auth, mock_api):
|
||||
mock_api.return_value = []
|
||||
gitea_list_issues(state="closed")
|
||||
url = mock_api.call_args[0][1]
|
||||
url = mock_api.call_args[0][0]
|
||||
self.assertIn("state=closed", url)
|
||||
|
||||
|
||||
@@ -258,7 +259,7 @@ class TestMirrorRefs(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestListPRs(unittest.TestCase):
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.api_get_all")
|
||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||
def test_list_prs(self, _auth, mock_api):
|
||||
mock_api.return_value = [
|
||||
@@ -861,6 +862,34 @@ class TestWhoami(unittest.TestCase):
|
||||
# ---------------------------------------------------------------------------
|
||||
# Runtime profile (env-configured profile metadata) — issue #19
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server._auth")
|
||||
def test_gitea_get_authenticated_user_alias(self, _auth, mock_api):
|
||||
mock_api.return_value = {
|
||||
"login": "alias_user",
|
||||
"full_name": "Alias User",
|
||||
"id": 999,
|
||||
"email": "alias@example.com"
|
||||
}
|
||||
from mcp_server import gitea_get_authenticated_user
|
||||
result = gitea_get_authenticated_user(remote="prgs")
|
||||
self.assertEqual(result["username"], "alias_user")
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server._auth")
|
||||
def test_gitea_get_current_user_alias(self, _auth, mock_api):
|
||||
mock_api.return_value = {
|
||||
"login": "alias_user",
|
||||
"full_name": "Alias User",
|
||||
"id": 999,
|
||||
"email": "alias@example.com"
|
||||
}
|
||||
from mcp_server import gitea_get_current_user
|
||||
result = gitea_get_current_user(remote="prgs")
|
||||
self.assertEqual(result["username"], "alias_user")
|
||||
|
||||
|
||||
class TestRuntimeProfile(unittest.TestCase):
|
||||
|
||||
def test_defaults_when_unset(self):
|
||||
@@ -1352,3 +1381,222 @@ class TestSubmitPrReview(unittest.TestCase):
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tracker Hygiene Cleanup Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.mock_api = patch("mcp_server.api_request").start()
|
||||
self.mock_auth = patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
self.mock_audit = patch("gitea_audit.write_event").start()
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["merge", "edit", "close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
|
||||
def test_close_issue_removes_in_progress(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "issues/1" in url:
|
||||
return {"state": "closed"}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}, {"name": "bug", "id": 2}]
|
||||
if method == "GET" and "issues/1" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}, {"name": "bug"}]}
|
||||
if method == "DELETE" and url.endswith("/issues/1/labels/1"):
|
||||
return {}
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_close_issue(issue_number=1)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"].get(1), "released")
|
||||
self.mock_audit.assert_called()
|
||||
|
||||
def test_close_issue_no_label_is_noop(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "issues/1" in url:
|
||||
return {"state": "closed"}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}, {"name": "bug", "id": 2}]
|
||||
if method == "GET" and "issues/1" in url:
|
||||
return {"labels": [{"name": "bug"}]}
|
||||
if method == "DELETE" and "labels" in url:
|
||||
self.fail("Should not DELETE labels")
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_close_issue(issue_number=1)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
||||
|
||||
def test_merge_pr_with_closes_removes_label(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
"state": "open",
|
||||
"head": {"sha": "sha123", "ref": "feat/my-branch"},
|
||||
"base": {"ref": "main"},
|
||||
"mergeable": True,
|
||||
"merged_commit_sha": "merge123",
|
||||
"title": "My PR",
|
||||
"body": "Closes #123"
|
||||
}
|
||||
if method == "POST" and "merge" in url:
|
||||
return {}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}, {"name": "bug", "id": 2}]
|
||||
if method == "GET" and "issues/123" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}, {"name": "bug"}]}
|
||||
if method == "DELETE" and url.endswith("/issues/123/labels/1"):
|
||||
return {}
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge")
|
||||
self.assertTrue(res["performed"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
|
||||
def test_merge_pr_with_branch_name_removes_label(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "merger"}
|
||||
if method == "GET" and "pulls/1" in url and "/files" not in url:
|
||||
return {
|
||||
"user": {"login": "author"},
|
||||
"state": "open",
|
||||
"head": {"sha": "sha123", "ref": "fix/issue-123-slug"},
|
||||
"base": {"ref": "main"},
|
||||
"mergeable": True,
|
||||
"merged_commit_sha": "merge123",
|
||||
"title": "My PR",
|
||||
"body": "Fixing things"
|
||||
}
|
||||
if method == "POST" and "merge" in url:
|
||||
return {}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}, {"name": "bug", "id": 2}]
|
||||
if method == "GET" and "issues/123" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}, {"name": "bug"}]}
|
||||
if method == "DELETE" and url.endswith("/issues/123/labels/1"):
|
||||
return {}
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_merge_pr(pr_number=1, confirmation="MERGE PR 1", do="merge")
|
||||
self.assertTrue(res["performed"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
|
||||
def test_close_pr_removes_label_but_does_not_close_issue(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "pulls/1" in url:
|
||||
return {
|
||||
"number": 1,
|
||||
"title": "My PR",
|
||||
"state": "closed",
|
||||
"html_url": "url",
|
||||
"body": "Closes #123",
|
||||
"head": {"ref": "feat/my-branch"}
|
||||
}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}]
|
||||
if method == "GET" and "issues/123" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}]}
|
||||
if method == "DELETE" and url.endswith("/issues/123/labels/1"):
|
||||
return {}
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
if method == "POST" and "comments" in url:
|
||||
return {}
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_edit_pr(pr_number=1, state="closed")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
|
||||
def test_multiple_linked_issues(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "pulls/1" in url:
|
||||
return {
|
||||
"number": 1,
|
||||
"title": "My PR",
|
||||
"state": "closed",
|
||||
"html_url": "url",
|
||||
"body": "Closes #123\nFixes #124",
|
||||
"head": {"ref": "issue-125"}
|
||||
}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}]
|
||||
if method == "GET" and "issues/123" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}]}
|
||||
if method == "GET" and "issues/124" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}]}
|
||||
if method == "GET" and "issues/125" in url:
|
||||
return {"labels": []}
|
||||
if method == "DELETE" and url.endswith("/issues/123/labels/1"):
|
||||
return {}
|
||||
if method == "DELETE" and url.endswith("/issues/124/labels/1"):
|
||||
return {}
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
if method == "POST" and "comments" in url:
|
||||
return {}
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_edit_pr(pr_number=1, state="closed")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||
self.assertEqual(res["cleanup_status"].get(124), "released")
|
||||
self.assertEqual(res["cleanup_status"].get(125), "not present")
|
||||
|
||||
def test_no_linked_issue_found(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "pulls/1" in url:
|
||||
return {
|
||||
"number": 1,
|
||||
"title": "My PR",
|
||||
"state": "closed",
|
||||
"html_url": "url",
|
||||
"body": "No issue link",
|
||||
"head": {"ref": "main"}
|
||||
}
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_edit_pr(pr_number=1, state="closed")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"], "no linked issue found")
|
||||
|
||||
def test_label_removal_failure_reported(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "issues/1" in url:
|
||||
return {"state": "closed"}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "status:in-progress", "id": 1}]
|
||||
if method == "GET" and "issues/1" in url:
|
||||
return {"labels": [{"name": "status:in-progress"}]}
|
||||
if method == "DELETE" and url.endswith("/issues/1/labels/1"):
|
||||
raise RuntimeError("API failure")
|
||||
if method == "PUT" and "labels" in url:
|
||||
self.fail("Should not replace the issue label set")
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_close_issue(issue_number=1)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertIn("error:", res["cleanup_status"].get(1))
|
||||
|
||||
Reference in New Issue
Block a user