diff --git a/agents/triage.md b/agents/triage.md index 7aef58c6..6a37dca2 100644 --- a/agents/triage.md +++ b/agents/triage.md @@ -3,6 +3,7 @@ name: triage description: Inspect an issue, assess information sufficiency, and produce a structured triage decision. skills: - issue-labels + - effort-estimation # curl: required by GitLab forge. On GitHub, the network policy binary # allowlist (policies/github/triage.yaml) excludes **/curl, preventing # it from making network requests even though it is granted here. @@ -128,7 +129,7 @@ Before forming any clarifying question, classify it: - Can you form a plausible root cause hypothesis from the available information? - Could a developer start investigating without contacting the reporter? - **Is progress blocked on other work?** Consider whether the fix depends on an unresolved issue or unmerged PR — in this repo or another. If a developer cannot meaningfully start work until some other issue is resolved, this issue has prerequisites regardless of how clear the problem description is. If the blocking work has no tracking issue yet, you can recommend creating one via the `prerequisites` action's `create` array. -- **Would resolving this issue require modifying CI/workflow files?** Scan the issue title, body, referenced files, and labels for signals that the fix involves changes under CI/pipeline configuration (e.g., `.github/workflows/`, `.gitlab-ci.yml`, `.fullsend/.github/workflows/`, or enrolled-repo shim workflows). Prefer deterministic signals — explicit path references, CI/workflow-scoped labels, mentions of CI pipeline configuration — over vague mentions of "workflow" in non-CI contexts (e.g., "user onboarding workflow"). If the fix likely requires workflow file changes, set `requires_workflow_changes: true` in `triage_summary` and include a warning in the triage comment that the code agent cannot modify workflow files under current permissions and that manual intervention (human PR/MR or maintainer action) is required. +- **Would resolving this issue require modifying CI/workflow files?** Scan the issue title, body, referenced files, and labels for signals that the fix involves changes under CI/pipeline configuration (e.g., `.github/workflows/`, `.gitlab-ci.yml`, `.fullsend/.github/workflows/`, or enrolled-repo shim workflows). Prefer deterministic signals — explicit path references, CI/workflow-scoped labels, mentions of CI pipeline configuration — over vague mentions of "workflow" in non-CI contexts (e.g., "user onboarding workflow"). If the fix likely requires workflow file changes, set `block_auto_promotion` with `blocked: true` and a reason explaining that the code agent cannot modify workflow files under current permissions and that manual intervention is required. - **Does this issue bundle multiple independent concerns?** An issue bundles independent concerns when it lists several distinct problems, tasks, or gaps that share no blocking relationship — each could be filed, triaged, and resolved independently. Use `action: "split"` to decompose the issue into separate sub-issues. Signs of a bundled issue: - A numbered or bulleted list of distinct items (e.g., "1. fix X, 2. add Y, 3. update Z") - Multiple unrelated components, files, or subsystems mentioned with no dependency between them @@ -327,6 +328,8 @@ Information is sufficient for a developer to investigate and fix. **Choosing a category:** the `feature` category covers issues that describe desired new behavior rather than a defect in existing functionality — the reporter expects something that has never been implemented. Use `feature` only when the described behavior clearly never existed in the product. If there is _any_ possibility the behavior is a regression (it used to work, or the reporter references a specific version where it worked), use `insufficient` instead and ask for version or timeline information. When in doubt, ask — do not prematurely reclassify. +**Estimating effort:** For bug, documentation, and performance categories, use the `effort-estimation` skill to score implementation effort and populate `block_auto_promotion`. Feature issues already route to human review and do not need effort estimation. + ```json { "action": "sufficient", @@ -349,9 +352,12 @@ Information is sufficient for a developer to investigate and fix. "impact": "Who is affected and how", "recommended_fix": "What a developer should investigate.", "proposed_test_case": "Conceptual description of a test that would verify the fix — what to test, expected vs actual behavior, and edge cases to cover. Do not assume a specific test framework or file layout.", - "requires_workflow_changes": false + "block_auto_promotion": { + "blocked": false, + "reason": "Low effort; single-file fix with existing test coverage" + } }, - "comment": "A triage summary comment formatted in markdown. Focus on information not already present in the issue body — omit sections that merely restate what the reporter wrote. Include the proposed test case as a fenced code block.", + "comment": "A triage summary comment formatted in markdown. Focus on information not already present in the issue body — omit sections that merely restate what the reporter wrote. Do not include fenced code blocks; summarize test cases and fixes in prose. Use inline `backtick` references for identifiers.", "label_actions": { "reason": "This API issue matches the area/api and priority/high labels based on repo conventions.", "actions": [ @@ -362,7 +368,12 @@ Information is sufficient for a developer to investigate and fix. } ``` -**Workflow change detection (optional):** If the issue likely requires modifying CI/pipeline configuration files (`.github/workflows/`, `.gitlab-ci.yml`, `.fullsend/.github/workflows/`, or enrolled-repo shim workflows), set `requires_workflow_changes: true` in `triage_summary`. When set, the post-triage script skips auto-triggering the code agent because the code agent cannot modify workflow files under current permissions. The triage comment should warn about this limitation and note that manual intervention is required. When `requires_workflow_changes` is not set or is `false`, auto-triggering proceeds normally. +**Blocking auto-promotion:** Use the `block_auto_promotion` field in `triage_summary` to prevent the post-triage script from auto-promoting the issue to the code agent. Set `blocked: true` with a `reason` when: +- The `effort-estimation` skill determines the issue requires human review (effort >= 4). +- The fix requires modifying CI/pipeline configuration files (`.github/workflows/`, `.gitlab-ci.yml`, `.fullsend/.github/workflows/`, or enrolled-repo shim workflows) that the code agent cannot modify under current permissions. +- Any other condition where auto-dispatch would be premature. + +When `blocked` is `true`, the post-script applies `triaged` instead of `ready-to-code` and appends the reason to the triage comment. When `blocked` is `false` (or omitted), auto-promotion proceeds normally for bug/documentation/performance categories. **Label recommendations (optional, all actions):** If the `issue-labels` skill identifies labels that should be applied or removed, include them in the `label_actions` field. This field is optional for all actions. If no labels clearly apply, omit it entirely. diff --git a/docs/code.md b/docs/code.md index ffc0016d..b3723131 100644 --- a/docs/code.md +++ b/docs/code.md @@ -33,7 +33,7 @@ on issues (not PRs). | Label | Meaning | |-------|---------| -| `ready-to-code` | Triggers the code agent. Applied by the [triage](triage.md) agent for low-risk categories (bug, documentation, performance), or manually by a human for feature work after prioritization. Not applied when the triage result sets `requires_workflow_changes`, since the code agent cannot modify workflow files. | +| `ready-to-code` | Triggers the code agent. Applied by the [triage](triage.md) post-script for low-risk categories (bug, documentation, performance) when auto-promotion is not blocked, or manually by a human for feature work, high-effort issues, or workflow changes after review. | | `ready-for-review` | Applied by the code agent after pushing a PR. In per-repo installs, triggers the [review agent](review.md) when applied to a PR. Also marks workflow state for humans and the [retro agent](retro.md). | ## Configuration diff --git a/docs/triage.md b/docs/triage.md index f14656bc..e63f1b70 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -40,14 +40,14 @@ These labels are managed by the triage agent based on its assessment of the issu | Label | Meaning | |-------|---------| | `needs-info` | The issue lacks sufficient information. The agent posted clarifying questions. | -| `ready-to-code` | The issue is fully specified and low-risk (bug, documentation, performance). Bug and documentation categories also receive their eponymous labels (`bug`, `documentation`) automatically. Triggers the [code agent](code.md). This behavior is configurable via [Variables](#variables). Exception: when `requires_workflow_changes` is set in the triage result, `triaged` is applied instead because the code agent cannot modify workflow files. | -| `triaged` | The issue is fully specified but is a feature or other category that requires human prioritization before coding. | +| `ready-to-code` | The issue is fully specified and low-risk (bug, documentation, performance) with auto-promotion not blocked. Bug and documentation categories also receive their eponymous labels (`bug`, `documentation`) automatically. Triggers the [code agent](code.md). This behavior is configurable via [Variables](#variables). | +| `triaged` | The issue requires human review before coding: feature work, other categories, or bug/docs/performance issues where `block_auto_promotion` is set (high effort, workflow changes, etc.). | | `duplicate` | The issue duplicates an existing one. The agent identified the original and the issue is closed automatically. | | `blocked` | The issue depends on another issue or external condition. The agent identified the blocker. | | `feature` | The issue is a feature request. Applied alongside `triaged` so humans can prioritize before coding begins. | | `question` | The issue is a question rather than a bug or feature request. | -| `bug` | The issue is a confirmed bug. Applied alongside `ready-to-code` to categorize the issue. | -| `documentation` | The issue concerns documentation improvements or additions. Applied alongside `ready-to-code` to categorize the issue. | +| `bug` | The issue is a confirmed bug. Applied alongside `ready-to-code` or `triaged` to categorize the issue. | +| `documentation` | The issue concerns documentation improvements or additions. Applied alongside `ready-to-code` or `triaged` to categorize the issue. | | `not-planned` | The issue is out of scope, invalid, or spam. The issue is closed with reason "not planned". | | `pr-open` | An open PR or merge request already addresses this issue. Applied either by the triage agent's `in-progress` action — used when a PR/MR *fixes* the issue, as opposed to `prerequisites`/`blocked` when a PR/MR must merely land first — or by the code agent's pre-check when it finds a human PR before dispatching. No automation clears this label when the linked PR/MR is closed without merging: nothing re-triages on PR/MR close, so the issue keeps `pr-open` — and the in-progress comment stays on the issue — until triage runs again, via an issue edit or a manual `/fs-triage`. | @@ -144,6 +144,28 @@ This gives the triage agent the subtlety it needs to distinguish between controller-runtime code, without adding label documentation to `AGENTS.md` where every agent would pay the context cost. +### Skill: `effort-estimation` + +The triage agent includes an `effort-estimation` skill that scores +implementation effort and decides whether the issue should be held for human +review before auto-promoting to the code agent. The skill populates the +`block_auto_promotion` field in `triage_summary`: + +- `blocked: true` + `reason`: the post-script applies `triaged` instead of + `ready-to-code` and appends the reason to the triage comment. +- `blocked: false` + `reason`: auto-promotion proceeds normally. + +By default, the skill scores effort on a 1 to 5 scale across four dimensions +(scope, testing, domain knowledge, risk). Issues scoring >= 4 are blocked. + +The same `block_auto_promotion` field is used for workflow-change detection: +if the fix requires modifying GitHub Actions workflow files, the agent sets +`blocked: true` with a reason explaining that the code agent cannot modify +workflow files. + +See [Fullsend's Customizing with Skills docs](https://fullsend.sh/docs/guides/user/customizing-with-skills.html) +to know where to add the skill files. + ### Variables | Variable | Description | Default | Valid values | diff --git a/eval/triage/cases/008-effort-high-multi-component/annotations.yaml b/eval/triage/cases/008-effort-high-multi-component/annotations.yaml new file mode 100644 index 00000000..568a8d99 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/annotations.yaml @@ -0,0 +1,55 @@ +# This case tests the effort-estimation gate. The bug spans multiple +# components (session, auth middleware, views, rate limiter) and requires +# new test infrastructure. The agent should score effort >= 4 and set +# block_auto_promotion.blocked = true, resulting in "triaged" instead +# of "ready-to-code". +state: open + +labels: + required: + - triaged + forbidden: + - ready-to-code + +max_turns: 30 +max_cost_usd: 2.00 + +triage_expectations: | + This issue reports a memory leak in the session store and a security + flaw where logout does not invalidate tokens. The fix touches at least + four files across three packages (auth, middleware, api) and requires + new test fixtures for session lifecycle. + + The effort-estimation skill should score this high because: + - Scope: four files across three packages (session.py, views.py, + auth middleware, rate_limit.py) — 4. + - Testing: no existing session lifecycle tests; needs new test + infrastructure for time-dependent behavior (mocking time.time) — 4. + - Domain knowledge: requires understanding session token security + (stolen tokens usable after logout), TTL eviction strategies, and + coordinating invalidation across the session store, auth middleware, + and rate limiter — 4. + - Risk: changing session management affects every authenticated + endpoint; incorrect eviction could log out active users — 4. + + Overall effort should be >= 4, triggering block_auto_promotion. + + A good triage should: + + 1. Verify the claims against the code: confirm SESSIONS only evicts + on lookup, confirm logout_handler is a no-op, confirm RATE_LIMITS + has the same accumulation pattern. + 2. Identify this as a defect (bug or security) — the session TTL is + implemented but the eviction is broken, and logout is documented + but not functional. + 3. Set block_auto_promotion.blocked = true with a reason citing the + multi-component scope and testing requirements. + 4. The post-script should apply "triaged" (not "ready-to-code"). + + Scoring guide: + A score of 1 means the agent misidentified the action or category. + A score of 3 means correct action and category but did not flag the + effort level or missed the rate_limit.py parallel issue. + A score of 5 means correct triage, verified all claims against code, + identified the cross-cutting nature, and blocked auto-promotion with + a clear reason. diff --git a/eval/triage/cases/008-effort-high-multi-component/input.yaml b/eval/triage/cases/008-effort-high-multi-component/input.yaml new file mode 100644 index 00000000..200c8a8f --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/input.yaml @@ -0,0 +1,41 @@ +forge: github +fixture: + type: issue + title: "Session tokens never expire in practice — memory leak and security risk" + body: | + ## Bug Report + + **What happened:** + We noticed our production server's memory usage climbs steadily over time + and never drops. After profiling, we traced it to the in-memory `SESSIONS` + dict in `src/auth/session.py` — it grows indefinitely because expired + sessions are only removed on lookup (`get_session`), never proactively + cleaned. Tokens that are never looked up again stay in memory forever. + + On top of the memory leak, `logout_handler` in `src/auth/views.py` does + not delete the session — the token remains valid for the full TTL even + after the user explicitly logs out. Combined with the lack of cleanup, + this means a stolen token can be used long after the user thought they + signed out. + + **Steps to reproduce:** + 1. Start the server and log in 10,000 times (scripted). + 2. Never revisit any of those sessions. + 3. Observe `SESSIONS` dict size — it holds all 10,000 entries. + 4. Log out — the session token is still usable afterward. + + **Expected behavior:** + - A background task or TTL-based eviction should remove expired sessions. + - `logout_handler` should invalidate the session token immediately. + - The auth middleware should stop accepting tokens after logout. + + **Impact:** + - Production OOMs every ~3 days, requiring manual restarts. + - Security: logout does not actually invalidate access. + - Affects the rate limiter too — `RATE_LIMITS` in + `src/middleware/rate_limit.py` has the same pattern (entries accumulate + without proactive cleanup). + + **Environment:** + - Python 3.12, single-process deployment + - ~2,000 active users, ~15,000 logins/day diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/README.md b/eval/triage/cases/008-effort-high-multi-component/repo/README.md new file mode 100644 index 00000000..11a679d1 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/README.md @@ -0,0 +1,31 @@ +# User Service + +A Python web application with authentication, user management API, +rate limiting, and session management. + +## Architecture + +``` +src/ + auth/ Session management, validators, login/logout views + api/ REST endpoints for user CRUD + middleware/ Auth enforcement, rate limiting + db/ Data access layer +tests/ Unit tests +``` + +## Running + +```bash +pip install -r requirements.txt +python -m src.main +``` + +## Configuration + +Environment variables: + +- `SESSION_TTL` — session timeout in seconds (default: 3600) +- `RATE_LIMIT_WINDOW` — rate limit window in seconds (default: 60) +- `RATE_LIMIT_MAX` — max requests per window (default: 100) +- `LOG_LEVEL` — logging verbosity (default: INFO) diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/app.py b/eval/triage/cases/008-effort-high-multi-component/repo/app.py new file mode 100644 index 00000000..5cc6c593 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/app.py @@ -0,0 +1,110 @@ +"""Minimal HTTP server for the auth + user management service.""" + +import json +import logging +from http.server import HTTPServer, BaseHTTPRequestHandler + +from src.auth.views import login_handler, logout_handler, session_status_handler +from src.api.users import list_users, get_user, update_user_handler, delete_user_handler +from src.middleware.rate_limit import check_rate_limit + +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger(__name__) + + +class Request: + def __init__(self, params, headers, remote_addr, body_json=None): + self.params = params + self.headers = headers + self.remote_addr = remote_addr + self.json = body_json + self.user_id = None + + +class AppHandler(BaseHTTPRequestHandler): + def _read_body(self): + length = int(self.headers.get("Content-Length", 0)) + if length: + return json.loads(self.rfile.read(length)) + return {} + + def _send(self, result): + if isinstance(result, tuple): + body, status = result + else: + body, status = result, 200 + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps(body).encode()) + + def _make_request(self, body=None): + headers = {k: v for k, v in self.headers.items()} + return Request( + params=body or {}, + headers=headers, + remote_addr=self.client_address[0], + body_json=body, + ) + + def _rate_limited(self): + if not check_rate_limit(self.client_address[0]): + self._send(({"status": "error", "message": "rate limit exceeded"}, 429)) + return True + return False + + def _user_id_from_path(self): + parts = self.path.strip("/").split("/") + if len(parts) >= 2: + return parts[1] + return None + + def do_POST(self): + if self._rate_limited(): + return + body = self._read_body() + req = self._make_request(body) + if self.path == "/login": + self._send(login_handler(req)) + elif self.path == "/logout": + self._send(logout_handler(req)) + else: + self._send(({"status": "error", "message": "not found"}, 404)) + + def do_GET(self): + if self._rate_limited(): + return + req = self._make_request() + if self.path == "/session": + self._send(session_status_handler(req)) + elif self.path == "/users": + self._send(list_users(req)) + elif self.path.startswith("/users/"): + self._send(get_user(req, self._user_id_from_path())) + else: + self._send(({"status": "error", "message": "not found"}, 404)) + + def do_PUT(self): + if self._rate_limited(): + return + body = self._read_body() + req = self._make_request(body) + if self.path.startswith("/users/"): + self._send(update_user_handler(req, self._user_id_from_path())) + else: + self._send(({"status": "error", "message": "not found"}, 404)) + + def do_DELETE(self): + if self._rate_limited(): + return + req = self._make_request() + if self.path.startswith("/users/"): + self._send(delete_user_handler(req, self._user_id_from_path())) + else: + self._send(({"status": "error", "message": "not found"}, 404)) + + +if __name__ == "__main__": + server = HTTPServer(("0.0.0.0", 8000), AppHandler) + logger.info("Listening on http://0.0.0.0:8000") + server.serve_forever() diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/api/users.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/api/users.py new file mode 100644 index 00000000..03f6ec92 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/api/users.py @@ -0,0 +1,65 @@ +"""User management API endpoints. + +All endpoints require authentication via the require_auth middleware. +Rate limiting is applied at the router level (see main.py). +""" + +import logging + +from ..middleware.auth import require_auth +from ..db.users import get_all_users, get_user_by_id, update_user, delete_user + +logger = logging.getLogger(__name__) + + +@require_auth +def list_users(request): + """Return all users. + + No pagination — returns the full list. This is fine for small + deployments but will need cursor-based pagination eventually. + """ + users = get_all_users() + return {"users": users, "count": len(users)} + + +@require_auth +def get_user(request, user_id): + """Return a single user by ID.""" + user = get_user_by_id(user_id) + if not user: + return {"status": "error", "message": "user not found"}, 404 + return {"user": user} + + +@require_auth +def update_user_handler(request, user_id): + """Update user fields. + + Accepts a JSON body with the fields to update. Does not validate + which fields are being changed — the caller can overwrite anything + including the role. + """ + data = request.json + if not data: + return {"status": "error", "message": "request body required"}, 400 + + existing = get_user_by_id(user_id) + if not existing: + return {"status": "error", "message": "user not found"}, 404 + + update_user(user_id, data) + logger.info("User %s updated by %s", user_id, request.user_id) + return {"status": "ok"} + + +@require_auth +def delete_user_handler(request, user_id): + """Delete a user by ID.""" + existing = get_user_by_id(user_id) + if not existing: + return {"status": "error", "message": "user not found"}, 404 + + delete_user(user_id) + logger.info("User %s deleted by %s", user_id, request.user_id) + return {"status": "ok"} diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/session.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/session.py new file mode 100644 index 00000000..f73f8771 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/session.py @@ -0,0 +1,77 @@ +"""Session management for authenticated users. + +Sessions are stored in-memory in a global dict keyed by token. Each session +tracks the owning user, creation time, and last activity timestamp. Sessions +expire after SESSION_TTL seconds from creation. + +Expiration is checked lazily on lookup — there is no background reaper. +""" + +import os +import time +import hashlib +import logging + +logger = logging.getLogger(__name__) + +SESSIONS: dict[str, dict] = {} +SESSION_TTL = int(os.environ.get("SESSION_TTL", "3600")) + + +def create_session(user_id: str, ip_address: str = "unknown") -> str: + """Create a new session and return the session token. + + The token is a SHA-256 hash of the user ID, current timestamp, and a + monotonic counter to avoid collisions when two requests arrive in the + same clock tick. + """ + raw = f"{user_id}:{time.time()}:{len(SESSIONS)}".encode() + token = hashlib.sha256(raw).hexdigest() + SESSIONS[token] = { + "user_id": user_id, + "created_at": time.time(), + "last_active": time.time(), + "ip_address": ip_address, + } + logger.info("Session created for user=%s from ip=%s", user_id, ip_address) + return token + + +def get_session(token: str) -> dict | None: + """Look up a session by token. + + Returns None if the session does not exist or has expired. Expired + sessions are removed from the store on access (lazy eviction). + """ + session = SESSIONS.get(token) + if not session: + return None + if time.time() - session["created_at"] > SESSION_TTL: + logger.info( + "Session expired for user=%s (age=%ds, ttl=%ds)", + session["user_id"], + int(time.time() - session["created_at"]), + SESSION_TTL, + ) + del SESSIONS[token] + return None + return session + + +def refresh_session(token: str) -> None: + """Update the last_active timestamp for a session. + + Called by the auth middleware on every authenticated request so the + session metadata reflects actual usage. + """ + session = SESSIONS.get(token) + if session: + session["last_active"] = time.time() + + +def active_session_count() -> int: + """Return the number of sessions currently in the store. + + Note: this includes expired sessions that have not been lazily evicted. + """ + return len(SESSIONS) diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/validators.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/validators.py new file mode 100644 index 00000000..2ae3ce63 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/validators.py @@ -0,0 +1,39 @@ +"""Input validators for authentication. + +Centralizes validation logic so views don't duplicate regex patterns +or length checks. +""" + +import re + +EMAIL_PATTERN = r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$" +PASSWORD_MIN_LENGTH = 8 +PASSWORD_MAX_LENGTH = 128 + + +def validate_email(email: str) -> None: + """Validate an email address format. + + Raises ValueError if the email does not match the expected pattern. + """ + if not isinstance(email, str): + raise TypeError("email must be a string") + if not re.match(EMAIL_PATTERN, email): + raise ValueError("invalid email format") + + +def validate_password(password: str) -> None: + """Validate password meets minimum requirements. + + Raises ValueError if the password is too short or too long. + """ + if not isinstance(password, str): + raise TypeError("password must be a string") + if len(password) < PASSWORD_MIN_LENGTH: + raise ValueError( + f"password must be at least {PASSWORD_MIN_LENGTH} characters" + ) + if len(password) > PASSWORD_MAX_LENGTH: + raise ValueError( + f"password must be at most {PASSWORD_MAX_LENGTH} characters" + ) diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/views.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/views.py new file mode 100644 index 00000000..b25f5b69 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/auth/views.py @@ -0,0 +1,60 @@ +"""Authentication views — login, logout, and session status.""" + +import logging + +from .validators import validate_email, validate_password +from .session import create_session, get_session + +logger = logging.getLogger(__name__) + + +def login_handler(request): + """Handle user login. + + Validates credentials, creates a session, and returns a session token. + """ + email = request.params.get("email") + password = request.params.get("password") + + if not email or not password: + return {"status": "error", "message": "email and password required"}, 400 + + try: + validate_email(email) + validate_password(password) + except (ValueError, TypeError) as exc: + return {"status": "error", "message": str(exc)}, 400 + + # In production this would check against a password hash in the database. + # For this codebase the check is stubbed out. + user_id = f"user-{email.split('@')[0]}" + + ip_address = request.headers.get("X-Forwarded-For", request.remote_addr) + token = create_session(user_id, ip_address=ip_address) + logger.info("Login succeeded for user=%s", user_id) + return {"status": "ok", "session_token": token} + + +def logout_handler(request): + """Handle user logout. + + Should invalidate the session token so it cannot be reused. + """ + # BUG: session cleanup is not implemented — the token stays valid + # for the remainder of its TTL even after the user logs out. + logger.info("Logout requested (session not invalidated)") + return {"status": "ok"} + + +def session_status_handler(request): + """Return information about the current session.""" + token = request.headers.get("Authorization", "").removeprefix("Bearer ") + session = get_session(token) + if not session: + return {"status": "error", "message": "no active session"}, 401 + return { + "status": "ok", + "user_id": session["user_id"], + "created_at": session["created_at"], + "last_active": session["last_active"], + } diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/db/users.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/db/users.py new file mode 100644 index 00000000..fdd00a6f --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/db/users.py @@ -0,0 +1,75 @@ +"""User database layer. + +In-memory user store. In production this would be backed by a database, +but the interface is the same — the rest of the codebase depends only on +the functions exported here, not on the storage mechanism. +""" + +import logging + +logger = logging.getLogger(__name__) + +USERS: dict[str, dict] = { + "user-alice": { + "id": "user-alice", + "email": "alice@example.com", + "name": "Alice Chen", + "role": "admin", + "active": True, + }, + "user-bob": { + "id": "user-bob", + "email": "bob@example.com", + "name": "Bob Martinez", + "role": "member", + "active": True, + }, + "user-carol": { + "id": "user-carol", + "email": "carol@example.com", + "name": "Carol Wu", + "role": "member", + "active": False, + }, +} + + +def get_all_users() -> list[dict]: + """Return all users as a list of dicts.""" + return list(USERS.values()) + + +def get_user_by_id(user_id: str) -> dict | None: + """Return a user by ID, or None if not found.""" + return USERS.get(user_id) + + +def get_user_by_email(email: str) -> dict | None: + """Return a user by email address, or None if not found.""" + for user in USERS.values(): + if user["email"] == email: + return user + return None + + +def update_user(user_id: str, data: dict) -> bool: + """Update a user's fields. Returns True if the user exists. + + No validation on which fields are updated — the caller is responsible + for checking permissions and field names. + """ + user = USERS.get(user_id) + if not user: + return False + user.update(data) + logger.info("Updated user %s: fields=%s", user_id, list(data.keys())) + return True + + +def delete_user(user_id: str) -> bool: + """Delete a user by ID. Returns True if the user existed.""" + if user_id in USERS: + del USERS[user_id] + logger.info("Deleted user %s", user_id) + return True + return False diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/auth.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/auth.py new file mode 100644 index 00000000..631a166a --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/auth.py @@ -0,0 +1,36 @@ +"""Authentication middleware. + +Wraps request handlers to enforce that a valid session token is present +in the Authorization header. Refreshes the session's last_active timestamp +on each authenticated request. +""" + +import logging + +from ..auth.session import get_session, refresh_session + +logger = logging.getLogger(__name__) + + +def require_auth(handler): + """Middleware decorator that requires a valid session token. + + Expects the token in an Authorization: Bearer header. + Returns 401 if the token is missing, invalid, or expired. + """ + def wrapper(request, *args, **kwargs): + auth_header = request.headers.get("Authorization", "") + if not auth_header.startswith("Bearer "): + return {"status": "error", "message": "missing auth token"}, 401 + + token = auth_header.removeprefix("Bearer ") + session = get_session(token) + if not session: + logger.warning("Auth failed: invalid or expired token") + return {"status": "error", "message": "unauthorized"}, 401 + + refresh_session(token) + request.user_id = session["user_id"] + logger.debug("Authenticated user=%s", session["user_id"]) + return handler(request, *args, **kwargs) + return wrapper diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/rate_limit.py b/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/rate_limit.py new file mode 100644 index 00000000..005bd021 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/src/middleware/rate_limit.py @@ -0,0 +1,57 @@ +"""Rate limiting middleware. + +Tracks per-IP request timestamps in an in-memory dict and rejects +requests that exceed the configured threshold. The sliding window is +cleaned on each check — old entries outside the window are discarded. + +Note: the cleanup only runs when a request arrives for that IP. IPs +that stop sending requests leave stale entries in RATE_LIMITS +indefinitely (same lazy-eviction pattern as the session store). +""" + +import os +import time +import logging + +logger = logging.getLogger(__name__) + +RATE_LIMITS: dict[str, list[float]] = {} +WINDOW = int(os.environ.get("RATE_LIMIT_WINDOW", "60")) +MAX_REQUESTS = int(os.environ.get("RATE_LIMIT_MAX", "100")) + + +def check_rate_limit(client_ip: str) -> bool: + """Return True if the request is within rate limits. + + Cleans up timestamps outside the current window before checking. + """ + now = time.time() + if client_ip not in RATE_LIMITS: + RATE_LIMITS[client_ip] = [] + + # Remove entries outside the window + RATE_LIMITS[client_ip] = [ + t for t in RATE_LIMITS[client_ip] if now - t < WINDOW + ] + + if len(RATE_LIMITS[client_ip]) >= MAX_REQUESTS: + logger.warning( + "Rate limit exceeded for ip=%s (%d requests in %ds window)", + client_ip, + len(RATE_LIMITS[client_ip]), + WINDOW, + ) + return False + + RATE_LIMITS[client_ip].append(now) + return True + + +def rate_limit_middleware(handler): + """Wrap a handler with rate limiting.""" + def wrapper(request): + client_ip = request.headers.get("X-Forwarded-For", request.remote_addr) + if not check_rate_limit(client_ip): + return {"status": "error", "message": "rate limit exceeded"}, 429 + return handler(request) + return wrapper diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_db.py b/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_db.py new file mode 100644 index 00000000..4ca02491 --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_db.py @@ -0,0 +1,64 @@ +"""Tests for the user database layer.""" + +from src.db.users import ( + get_all_users, + get_user_by_id, + get_user_by_email, + update_user, + delete_user, + USERS, +) + + +class TestGetAllUsers: + def test_returns_list(self): + result = get_all_users() + assert isinstance(result, list) + assert len(result) > 0 + + def test_contains_expected_users(self): + users = get_all_users() + emails = [u["email"] for u in users] + assert "alice@example.com" in emails + + +class TestGetUserById: + def test_existing_user(self): + user = get_user_by_id("user-alice") + assert user is not None + assert user["email"] == "alice@example.com" + + def test_missing_user(self): + assert get_user_by_id("user-nonexistent") is None + + +class TestGetUserByEmail: + def test_existing_email(self): + user = get_user_by_email("bob@example.com") + assert user is not None + assert user["id"] == "user-bob" + + def test_missing_email(self): + assert get_user_by_email("nobody@example.com") is None + + +class TestUpdateUser: + def test_update_existing(self): + original_name = USERS["user-bob"]["name"] + update_user("user-bob", {"name": "Robert Martinez"}) + assert USERS["user-bob"]["name"] == "Robert Martinez" + # Restore + USERS["user-bob"]["name"] = original_name + + def test_update_missing(self): + assert update_user("user-nonexistent", {"name": "Ghost"}) is False + + +class TestDeleteUser: + def test_delete_existing(self): + USERS["user-temp"] = {"id": "user-temp", "email": "temp@example.com"} + assert delete_user("user-temp") is True + assert "user-temp" not in USERS + + def test_delete_missing(self): + assert delete_user("user-nonexistent") is False diff --git a/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_validators.py b/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_validators.py new file mode 100644 index 00000000..e97e9a9c --- /dev/null +++ b/eval/triage/cases/008-effort-high-multi-component/repo/tests/test_validators.py @@ -0,0 +1,44 @@ +"""Tests for input validators.""" + +import pytest +from src.auth.validators import validate_email, validate_password + + +class TestValidateEmail: + def test_valid_simple(self): + validate_email("user@example.com") + + def test_valid_with_plus(self): + validate_email("user+tag@example.com") + + def test_valid_with_dots(self): + validate_email("first.last@example.com") + + def test_invalid_no_at(self): + with pytest.raises(ValueError): + validate_email("not-an-email") + + def test_invalid_no_domain(self): + with pytest.raises(ValueError): + validate_email("user@") + + def test_none_raises_type_error(self): + with pytest.raises(TypeError): + validate_email(None) + + +class TestValidatePassword: + def test_valid_password(self): + validate_password("securepassword123") + + def test_too_short(self): + with pytest.raises(ValueError, match="at least 8"): + validate_password("short") + + def test_too_long(self): + with pytest.raises(ValueError, match="at most 128"): + validate_password("a" * 200) + + def test_none_raises_type_error(self): + with pytest.raises(TypeError): + validate_password(None) diff --git a/eval/triage/cases/009-effort-low-single-file/annotations.yaml b/eval/triage/cases/009-effort-low-single-file/annotations.yaml new file mode 100644 index 00000000..3aabdda8 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/annotations.yaml @@ -0,0 +1,54 @@ +# This case tests the effort-estimation gate for a low-effort bug. +# The fix is a single character change in one file, with existing tests +# that already cover username validation. The agent should score effort +# low (< 4) and NOT block auto-promotion, resulting in "ready-to-code". +state: open + +labels: + required: + - ready-to-code + forbidden: + - triaged + +max_turns: 30 +max_cost_usd: 2.00 + +triage_expectations: | + This issue reports that two-character usernames are rejected when they + should be valid. The fix is a single-character regex change in + validators.py ({2,29} -> {1,29}). + + The effort-estimation skill should score this low because: + - Scope: single line in a single file (validators.py) — 1. + - Testing: existing test suite covers username validation with + boundary cases (test_valid_exactly_3_chars, test_invalid_too_short); + the fix would need minor test updates but no new infrastructure — 1-2. + - Domain knowledge: obvious from the error message and regex — 1. + - Risk: no behavior change for callers that already pass 3+ character + usernames; only loosens a constraint — 1. + + Overall effort should be well under 4, so block_auto_promotion.blocked + should be false (or omitted). + + A good triage should: + + 1. Verify the reporter's analysis against the code: confirm the regex + pattern is ^[a-zA-Z][a-zA-Z0-9_-]{2,29}$ and that changing {2,29} + to {1,29} is the correct fix. + 2. Identify this as a bug — the minimum length is too restrictive, + not a feature request for new functionality. + 3. Set block_auto_promotion.blocked = false (or omit block_auto_promotion) + with a reason citing the low effort. + 4. The post-script should apply "ready-to-code" + "bug". + 5. Note that the existing test test_valid_exactly_3_chars should be + updated to test_valid_exactly_2_chars, and test_invalid_too_short + needs a new boundary (single-character username). + + Scoring guide: + A score of 1 means the agent misidentified the action or category. + A score of 2 means correct action but blocked auto-promotion despite + the trivial scope, or missed the existing test coverage. + A score of 3 means correct action and labels but did not verify the + regex against the code. + A score of 5 means correct triage, verified the regex, identified + specific test updates needed, and set low effort with a clear reason. diff --git a/eval/triage/cases/009-effort-low-single-file/input.yaml b/eval/triage/cases/009-effort-low-single-file/input.yaml new file mode 100644 index 00000000..f1d49efe --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/input.yaml @@ -0,0 +1,34 @@ +forge: github +fixture: + type: issue + title: "Registration rejects valid two-letter usernames like 'Jo'" + body: | + ## Bug Report + + **What happened:** + Trying to register with the username `Jo` returns a 400 error: + "username must start with a letter, be 3-30 characters, and contain + only letters, digits, hyphens, and underscores." + + The username `Jo` is two characters long and the validator requires + a minimum of 3. But two-character usernames should be allowed — many + real names are two letters (Jo, Li, Bo, Al, Ed). + + **Steps to reproduce:** + 1. POST `/register` with `{"email": "jo@example.com", "password": + "securepass1", "username": "Jo"}` + 2. Response: 400 with the validation error above. + + **Expected behavior:** + Two-character usernames should be accepted. The minimum should be 2, + not 3. + + **Root cause:** + In `src/auth/validators.py`, the `USERNAME_PATTERN` regex uses + `{2,29}` for the trailing characters after the first letter. Combined + with the mandatory first letter, this enforces a 3-character minimum. + Changing `{2,29}` to `{1,29}` would fix it. + + **Environment:** + - Python 3.12 + - Testing locally with curl diff --git a/eval/triage/cases/009-effort-low-single-file/repo/README.md b/eval/triage/cases/009-effort-low-single-file/repo/README.md new file mode 100644 index 00000000..8d341f97 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/README.md @@ -0,0 +1,25 @@ +# Auth Service + +A Python authentication service with email validation, password +verification, and login/logout endpoints. + +## Running + +```bash +pip install -r requirements.txt +python -m src.main +``` + +## API + +| Method | Path | Description | +|--------|-------------|--------------------------| +| POST | `/login` | Authenticate with email and password | +| POST | `/logout` | End the current session | +| POST | `/register` | Create a new account | + +## Testing + +```bash +pytest tests/ +``` diff --git a/eval/triage/cases/009-effort-low-single-file/repo/app.py b/eval/triage/cases/009-effort-low-single-file/repo/app.py new file mode 100644 index 00000000..3b0a86b4 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/app.py @@ -0,0 +1,58 @@ +"""Minimal HTTP server for the auth service.""" + +import json +import logging +from http.server import HTTPServer, BaseHTTPRequestHandler + +from src.auth.views import login_handler, logout_handler, register_handler + +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger(__name__) + + +class Request: + def __init__(self, params, headers, remote_addr): + self.params = params + self.headers = headers + self.remote_addr = remote_addr + self.json = None + + +class AppHandler(BaseHTTPRequestHandler): + def _read_body(self): + length = int(self.headers.get("Content-Length", 0)) + if length: + return json.loads(self.rfile.read(length)) + return {} + + def _send(self, result): + if isinstance(result, tuple): + body, status = result + else: + body, status = result, 200 + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(json.dumps(body).encode()) + + def do_POST(self): + body = self._read_body() + headers = {k: v for k, v in self.headers.items()} + req = Request(params=body, headers=headers, remote_addr=self.client_address[0]) + + routes = { + "/login": login_handler, + "/logout": logout_handler, + "/register": register_handler, + } + handler = routes.get(self.path) + if handler: + self._send(handler(req)) + else: + self._send(({"status": "error", "message": "not found"}, 404)) + + +if __name__ == "__main__": + server = HTTPServer(("0.0.0.0", 8000), AppHandler) + logger.info("Listening on http://0.0.0.0:8000") + server.serve_forever() diff --git a/eval/triage/cases/009-effort-low-single-file/repo/src/auth/validators.py b/eval/triage/cases/009-effort-low-single-file/repo/src/auth/validators.py new file mode 100644 index 00000000..8603aae0 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/src/auth/validators.py @@ -0,0 +1,57 @@ +"""Input validators for authentication. + +Centralizes validation logic for email, password, and username fields. +All validators raise ValueError on invalid input. +""" + +import re + +EMAIL_PATTERN = r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$" +PASSWORD_MIN_LENGTH = 8 +PASSWORD_MAX_LENGTH = 128 +USERNAME_PATTERN = r"^[a-zA-Z][a-zA-Z0-9_-]{2,29}$" + + +def validate_email(email: str) -> None: + """Validate an email address format. + + Raises ValueError if the email does not match the expected pattern. + Raises TypeError if the input is not a string. + """ + if not isinstance(email, str): + raise TypeError("email must be a string") + if not re.match(EMAIL_PATTERN, email): + raise ValueError("invalid email format") + + +def validate_password(password: str) -> None: + """Validate password meets minimum requirements. + + Checks length only — no complexity requirements (those belong in + a password policy layer, not a format validator). + """ + if not isinstance(password, str): + raise TypeError("password must be a string") + if len(password) < PASSWORD_MIN_LENGTH: + raise ValueError( + f"password must be at least {PASSWORD_MIN_LENGTH} characters" + ) + if len(password) > PASSWORD_MAX_LENGTH: + raise ValueError( + f"password must be at most {PASSWORD_MAX_LENGTH} characters" + ) + + +def validate_username(username: str) -> None: + """Validate a username. + + Usernames must start with a letter, contain only letters, digits, + hyphens, and underscores, and be 3-30 characters long. + """ + if not isinstance(username, str): + raise TypeError("username must be a string") + if not re.match(USERNAME_PATTERN, username): + raise ValueError( + "username must start with a letter, be 3-30 characters, " + "and contain only letters, digits, hyphens, and underscores" + ) diff --git a/eval/triage/cases/009-effort-low-single-file/repo/src/auth/views.py b/eval/triage/cases/009-effort-low-single-file/repo/src/auth/views.py new file mode 100644 index 00000000..bff7fa58 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/src/auth/views.py @@ -0,0 +1,61 @@ +"""Authentication views — login, logout, registration.""" + +import logging + +from .validators import validate_email, validate_password, validate_username + +logger = logging.getLogger(__name__) + + +def login_handler(request): + """Handle user login. + + Validates the email and password, then authenticates against the + user store. Returns a session token on success. + """ + email = request.params.get("email") + password = request.params.get("password") + + if not email or not password: + return {"status": "error", "message": "email and password required"}, 400 + + try: + validate_email(email) + validate_password(password) + except (ValueError, TypeError) as exc: + return {"status": "error", "message": str(exc)}, 400 + + # ... authenticate against user store ... + return {"status": "ok", "session_token": "stub-token"} + + +def logout_handler(request): + """Handle user logout.""" + return {"status": "ok"} + + +def register_handler(request): + """Handle user registration. + + Validates all input fields before creating the account. + """ + email = request.params.get("email") + password = request.params.get("password") + username = request.params.get("username") + + if not email or not password or not username: + return { + "status": "error", + "message": "email, password, and username required", + }, 400 + + try: + validate_email(email) + validate_password(password) + validate_username(username) + except (ValueError, TypeError) as exc: + return {"status": "error", "message": str(exc)}, 400 + + # ... create user in store ... + logger.info("Registered user: email=%s username=%s", email, username) + return {"status": "ok", "message": "account created"}, 201 diff --git a/eval/triage/cases/009-effort-low-single-file/repo/tests/test_validators.py b/eval/triage/cases/009-effort-low-single-file/repo/tests/test_validators.py new file mode 100644 index 00000000..f9e7085f --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/tests/test_validators.py @@ -0,0 +1,105 @@ +"""Tests for input validators. + +Covers email, password, and username validation with both valid and +invalid inputs, plus type-error guards. +""" + +import pytest +from src.auth.validators import validate_email, validate_password, validate_username + + +class TestValidateEmail: + def test_valid_simple(self): + validate_email("user@example.com") + + def test_valid_with_plus(self): + validate_email("user+tag@example.com") + + def test_valid_with_dots(self): + validate_email("first.last@example.com") + + def test_valid_with_percent(self): + validate_email("user%tag@example.com") + + def test_invalid_no_at(self): + with pytest.raises(ValueError, match="invalid email"): + validate_email("not-an-email") + + def test_invalid_no_domain(self): + with pytest.raises(ValueError, match="invalid email"): + validate_email("user@") + + def test_invalid_no_tld(self): + with pytest.raises(ValueError, match="invalid email"): + validate_email("user@example") + + def test_none_raises_type_error(self): + with pytest.raises(TypeError, match="email must be a string"): + validate_email(None) + + def test_empty_string(self): + with pytest.raises(ValueError, match="invalid email"): + validate_email("") + + +class TestValidatePassword: + def test_valid_password(self): + validate_password("securepass123") + + def test_exactly_min_length(self): + validate_password("a" * 8) + + def test_exactly_max_length(self): + validate_password("a" * 128) + + def test_too_short(self): + with pytest.raises(ValueError, match="at least 8"): + validate_password("short") + + def test_too_long(self): + with pytest.raises(ValueError, match="at most 128"): + validate_password("a" * 200) + + def test_none_raises_type_error(self): + with pytest.raises(TypeError, match="password must be a string"): + validate_password(None) + + +class TestValidateUsername: + def test_valid_simple(self): + validate_username("alice") + + def test_valid_with_numbers(self): + validate_username("alice123") + + def test_valid_with_hyphens(self): + validate_username("alice-chen") + + def test_valid_with_underscores(self): + validate_username("alice_chen") + + def test_valid_exactly_3_chars(self): + validate_username("abc") + + def test_valid_exactly_30_chars(self): + validate_username("a" * 30) + + def test_invalid_starts_with_number(self): + with pytest.raises(ValueError, match="must start with a letter"): + validate_username("123alice") + + def test_invalid_too_short(self): + with pytest.raises(ValueError, match="must start with a letter"): + validate_username("ab") + + def test_invalid_too_long(self): + with pytest.raises(ValueError, match="must start with a letter"): + validate_username("a" * 31) + + def test_invalid_special_chars(self): + with pytest.raises(ValueError, match="must start with a letter"): + validate_username("alice@chen") + + def test_none_raises_type_error(self): + with pytest.raises(TypeError, match="username must be a string"): + validate_username(None) diff --git a/eval/triage/cases/009-effort-low-single-file/repo/tests/test_views.py b/eval/triage/cases/009-effort-low-single-file/repo/tests/test_views.py new file mode 100644 index 00000000..ed072c33 --- /dev/null +++ b/eval/triage/cases/009-effort-low-single-file/repo/tests/test_views.py @@ -0,0 +1,88 @@ +"""Tests for authentication views. + +Uses a minimal request stub to test handler logic without a real +HTTP server. +""" + +from src.auth.views import login_handler, register_handler + + +class StubRequest: + """Minimal request object for testing.""" + + def __init__(self, params=None, headers=None): + self.params = params or {} + self.headers = headers or {} + self.remote_addr = "127.0.0.1" + self.json = None + + def get(self, key, default=None): + return self.params.get(key, default) + + +class TestLoginHandler: + def test_missing_email(self): + req = StubRequest(params={"password": "goodpassword1"}) + result, status = login_handler(req) + assert status == 400 + assert "required" in result["message"] + + def test_missing_password(self): + req = StubRequest(params={"email": "user@example.com"}) + result, status = login_handler(req) + assert status == 400 + assert "required" in result["message"] + + def test_invalid_email(self): + req = StubRequest(params={"email": "bad", "password": "goodpassword1"}) + result, status = login_handler(req) + assert status == 400 + assert "email" in result["message"] + + def test_short_password(self): + req = StubRequest( + params={"email": "user@example.com", "password": "short"} + ) + result, status = login_handler(req) + assert status == 400 + assert "at least" in result["message"] + + def test_valid_login(self): + req = StubRequest( + params={"email": "user@example.com", "password": "goodpassword1"}, + headers={}, + ) + result = login_handler(req) + # login_handler returns a dict (no status tuple) on success + assert result["status"] == "ok" + + +class TestRegisterHandler: + def test_missing_fields(self): + req = StubRequest(params={"email": "user@example.com"}) + result, status = register_handler(req) + assert status == 400 + + def test_invalid_username(self): + req = StubRequest( + params={ + "email": "user@example.com", + "password": "goodpassword1", + "username": "123bad", + } + ) + result, status = register_handler(req) + assert status == 400 + assert "must start" in result["message"] + + def test_valid_registration(self): + req = StubRequest( + params={ + "email": "new@example.com", + "password": "securepassword", + "username": "newuser", + } + ) + result, status = register_handler(req) + assert status == 201 + assert result["status"] == "ok" diff --git a/harness/triage.yaml b/harness/triage.yaml index f8248cdc..f05bcf76 100644 --- a/harness/triage.yaml +++ b/harness/triage.yaml @@ -17,6 +17,9 @@ host_files: dest: /sandbox/workspace/.gcp-oidc-token optional: true +skills: + - skills/effort-estimation + pre_script: scripts/pre-triage.sh post_script: scripts/post-triage.sh diff --git a/schemas/triage-result.schema.json b/schemas/triage-result.schema.json index 83252306..08876817 100644 --- a/schemas/triage-result.schema.json +++ b/schemas/triage-result.schema.json @@ -185,7 +185,16 @@ "impact": { "type": "string", "minLength": 1 }, "recommended_fix": { "type": "string", "minLength": 1 }, "proposed_test_case": { "type": "string", "minLength": 1 }, - "requires_workflow_changes": { "type": "boolean" } + "block_auto_promotion": { + "description": "Set blocked to true with a reason to prevent auto-promotion; set blocked to false with a reason when auto-promotion is safe.", + "type": "object", + "required": ["blocked", "reason"], + "properties": { + "blocked": { "type": "boolean" }, + "reason": { "type": "string", "minLength": 1, "maxLength": 1024 } + }, + "additionalProperties": false + } }, "additionalProperties": false }, diff --git a/scripts/post-triage-test.sh b/scripts/post-triage-test.sh index 8284848d..08e0780c 100755 --- a/scripts/post-triage-test.sh +++ b/scripts/post-triage-test.sh @@ -484,6 +484,11 @@ run_test "label-actions-applied" \ '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady.","label_actions":{"reason":"API crash matches area/api label.","actions":[{"action":"add","label":"area/api"}]}}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=area/api --silent" +# Fenced code blocks in comment must be stripped (mirrors post-scribe.sh enforcement). +run_test_stdout "comment-fenced-code-block-warning" \ + '{"action":"insufficient","reasoning":"missing repro","clarity_scores":{"symptom":0.6,"cause":0.3,"reproduction":0.1,"impact":0.5,"overall":0.39},"comment":"Please try:\n```bash\necho hello\n```\nand report back."}' \ + "::warning::Stripping fenced code blocks from triage comment" + run_test_stdout "label-actions-control-label-refused" \ '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady.","label_actions":{"reason":"Tried to set control label.","actions":[{"action":"add","label":"ready-to-code"}]}}' \ "::warning::Refused to add control label 'ready-to-code' -- control labels are managed by the triage pipeline" @@ -561,6 +566,10 @@ run_test_no_pattern() { echo "PASS: ${test_name}" } +run_test_no_pattern "comment-fenced-code-block-stripped" \ + '{"action":"insufficient","reasoning":"missing repro","clarity_scores":{"symptom":0.6,"cause":0.3,"reproduction":0.1,"impact":0.5,"overall":0.39},"comment":"Please try:\n```bash\necho hello\n```\nand report back."}' \ + '```' + run_test_no_pattern "label-actions-all-refused-no-reason" \ '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady.","label_actions":{"reason":"Should not appear.","actions":[{"action":"add","label":"ready-to-code"}]}}' \ "Should not appear." @@ -724,43 +733,38 @@ run_validated_dir_test "validated-dir-neither-filename" \ "" \ "true" -# --- Workflow change detection tests (#325) --- +# --- Auto-promotion blocking tests (#2207, #325) --- -# Bug with requires_workflow_changes=true should get triaged instead of ready-to-code. -run_test "workflow-changes-bug-gets-triaged" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching step","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","requires_workflow_changes":true},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ +# Bug with block_auto_promotion.blocked=true (workflow changes) gets triaged. +run_test "blocked-workflow-bug-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching step","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","block_auto_promotion":{"blocked":true,"reason":"Fix requires modifying workflow files; the code agent cannot modify these under current permissions"}},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" -# Bug with requires_workflow_changes=true should NOT get ready-to-code. -run_test_no_pattern "workflow-changes-bug-no-ready-to-code" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching step","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","requires_workflow_changes":true},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ +# Bug with block_auto_promotion.blocked=true should NOT get ready-to-code. +run_test_no_pattern "blocked-bug-no-ready-to-code" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching step","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","block_auto_promotion":{"blocked":true,"reason":"Fix requires modifying workflow files"}},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ "labels[]=ready-to-code" -# Documentation with requires_workflow_changes=true should get triaged instead of ready-to-code. -run_test "workflow-changes-documentation-gets-triaged" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Update CI docs","severity":"low","category":"documentation","problem":"Outdated CI docs","root_cause_hypothesis":"Not updated","reproduction_steps":["step 1"],"environment":"Linux","impact":"Contributors","recommended_fix":"Update workflow and docs","proposed_test_case":"test_docs","requires_workflow_changes":true},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ +# Documentation with block_auto_promotion.blocked=true gets triaged. +run_test "blocked-documentation-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Update CI docs","severity":"low","category":"documentation","problem":"Outdated CI docs","root_cause_hypothesis":"Not updated","reproduction_steps":["step 1"],"environment":"Linux","impact":"Contributors","recommended_fix":"Update workflow and docs","proposed_test_case":"test_docs","block_auto_promotion":{"blocked":true,"reason":"Fix requires modifying workflow files"}},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" -# Performance with requires_workflow_changes=true should get triaged instead of ready-to-code. -run_test "workflow-changes-performance-gets-triaged" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Speed up CI","severity":"medium","category":"performance","problem":"Slow CI","root_cause_hypothesis":"No parallelism","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Add parallel steps","proposed_test_case":"test_speed","requires_workflow_changes":true},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ +# Performance with block_auto_promotion.blocked=true gets triaged. +run_test "blocked-performance-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Speed up CI","severity":"medium","category":"performance","problem":"Slow CI","root_cause_hypothesis":"No parallelism","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Add parallel steps","proposed_test_case":"test_speed","block_auto_promotion":{"blocked":true,"reason":"Estimated effort 4.3/5 (multi-component optimization)"}},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" -# Bug without requires_workflow_changes still gets ready-to-code (regression guard). -run_test "no-workflow-flag-bug-still-gets-ready-to-code" \ +# Bug without block_auto_promotion still gets ready-to-code (regression guard). +run_test "no-block-flag-bug-still-gets-ready-to-code" \ '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash"},"comment":"## Triage Summary\n\nReady."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=ready-to-code --silent" -# Bug with requires_workflow_changes=false still gets ready-to-code. -run_test "workflow-false-bug-gets-ready-to-code" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash","requires_workflow_changes":false},"comment":"## Triage Summary\n\nReady."}' \ +# Bug with block_auto_promotion.blocked=false gets ready-to-code. +run_test "unblocked-bug-gets-ready-to-code" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix crash","severity":"high","category":"bug","problem":"Crash","root_cause_hypothesis":"Buffer overflow","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix buffer","proposed_test_case":"test_crash","block_auto_promotion":{"blocked":false,"reason":"Low effort (1.5/5); single-file fix"}},"comment":"## Triage Summary\n\nReady."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=ready-to-code --silent" -# Workflow changes warning appears in stdout. -run_test_stdout "workflow-changes-warning-emitted" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","requires_workflow_changes":true},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ - "::warning::Triage detected workflow file changes required (#325)" - # --- TRIAGE_AUTO_CODE configuration tests (#1754) --- # Helper: run_test with extra env vars. Accepts a 5th arg: newline-separated @@ -1073,9 +1077,9 @@ run_test_with_env "auto-code-on-uppercase-still-matches" \ "false" \ $'TRIAGE_AUTO_CODE=on\nTRIAGE_AUTO_CODE_CATEGORIES=Bug,Documentation' -# TRIAGE_AUTO_CODE=off with workflow-changes: still triaged (both guards agree). -run_test_with_env "auto-code-off-with-workflow-changes-gets-triaged" \ - '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI","severity":"high","category":"bug","problem":"CI broken","root_cause_hypothesis":"Missing step","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_ci","requires_workflow_changes":true},"comment":"## Triage Summary\n\nNeeds workflow changes."}' \ +# TRIAGE_AUTO_CODE=off with block_auto_promotion: still triaged (both guards agree). +run_test_with_env "auto-code-off-with-blocked-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI","severity":"high","category":"bug","problem":"CI broken","root_cause_hypothesis":"Missing step","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_ci","block_auto_promotion":{"blocked":true,"reason":"Fix requires modifying workflow files"}},"comment":"## Triage Summary\n\nNeeds workflow changes."}' \ "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" \ "false" \ "TRIAGE_AUTO_CODE=off" @@ -1619,6 +1623,53 @@ export ISSUE_URL="https://github.com/test-org/test-repo/issues/42" export GH_TOKEN="fake-token" unset GITLAB_TOKEN +# --- block_auto_promotion tests (#2207) --- + +# Blocked bug warning appears in stdout. +run_test_stdout "blocked-warning-emitted" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix CI caching","severity":"high","category":"bug","problem":"CI cache miss","root_cause_hypothesis":"Missing cache key","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Update workflow","proposed_test_case":"test_cache","block_auto_promotion":{"blocked":true,"reason":"Fix requires modifying workflow files"}},"comment":"## Triage Summary\n\nThis requires workflow changes."}' \ + "::warning::Skipping ready-to-code — auto-promotion blocked (see comment for details)" + +# Blocked reason is appended to the comment. +run_test "blocked-reason-in-comment" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Refactor auth","severity":"high","category":"bug","problem":"Auth issue","root_cause_hypothesis":"Architectural","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Refactor","proposed_test_case":"test_auth","block_auto_promotion":{"blocked":true,"reason":"Estimated effort 4.3/5 (multi-component fix)"}},"comment":"## Triage Summary\n\nHigh effort."}' \ + "Auto-promotion blocked:" + +# High-effort bug (blocked=true) gets triaged, not ready-to-code. +run_test "effort-high-bug-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Refactor auth middleware","severity":"high","category":"bug","problem":"Auth edge cases","root_cause_hypothesis":"Architectural issue","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Refactor auth","proposed_test_case":"test_auth","block_auto_promotion":{"blocked":true,"reason":"Estimated effort 4.3/5 (multi-component fix with new test fixtures needed)"}},"comment":"## Triage Summary\n\nSubstantial refactor."}' \ + "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" + +# Low-effort bug (blocked=false) gets ready-to-code. +run_test "effort-low-bug-gets-ready-to-code" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix typo","severity":"low","category":"bug","problem":"Typo in error","root_cause_hypothesis":"Copy-paste error","reproduction_steps":["step 1"],"environment":"Linux","impact":"Minor","recommended_fix":"Fix typo","proposed_test_case":"test_msg","block_auto_promotion":{"blocked":false,"reason":"Low effort (1/5); single-line fix"}},"comment":"## Triage Summary\n\nTrivial."}' \ + "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=ready-to-code --silent" + +# Feature with block_auto_promotion is unaffected (already goes to triaged). +run_test "blocked-feature-still-gets-triaged" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Add dark mode","severity":"medium","category":"feature","problem":"No dark mode","root_cause_hypothesis":"Not implemented","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Add theme toggle","proposed_test_case":"test_dark_mode","block_auto_promotion":{"blocked":true,"reason":"High effort"}},"comment":"## Triage Summary\n\nFeature."}' \ + "gh api repos/test-org/test-repo/issues/42/labels -f labels[]=triaged --silent" + +# Blocked feature must NOT get block-reason footer in comment. +run_test_no_pattern "blocked-feature-no-block-reason-in-comment" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Add dark mode","severity":"medium","category":"feature","problem":"No dark mode","root_cause_hypothesis":"Not implemented","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Add theme toggle","proposed_test_case":"test_dark_mode","block_auto_promotion":{"blocked":true,"reason":"High effort"}},"comment":"## Triage Summary\n\nFeature."}' \ + "Auto-promotion blocked:" + +# Blocked with empty reason still gets a fallback footer. +run_test "blocked-empty-reason-gets-fallback" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Fix bug","severity":"high","category":"bug","problem":"Bug","root_cause_hypothesis":"Root","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix","proposed_test_case":"test","block_auto_promotion":{"blocked":true,"reason":""}},"comment":"## Triage Summary\n\nBug."}' \ + "No reason provided" + +# Block reason with workflow-command injection attempt must be sanitized. +run_test_no_pattern "blocked-reason-injection-sanitized" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Inject","severity":"high","category":"bug","problem":"Bug","root_cause_hypothesis":"Root","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix","proposed_test_case":"test","block_auto_promotion":{"blocked":true,"reason":"reason\n::error::injected"}},"comment":"## Triage Summary\n\nBug."}' \ + "::error::injected" + +# Triple-colon bypass: :::error::: must not survive as ::error::. +run_test_no_pattern "blocked-reason-triple-colon-sanitized" \ + '{"action":"sufficient","reasoning":"all clear","clarity_scores":{"symptom":0.9,"cause":0.85,"reproduction":0.9,"impact":0.8,"overall":0.87},"triage_summary":{"title":"Inject","severity":"high","category":"bug","problem":"Bug","root_cause_hypothesis":"Root","reproduction_steps":["step 1"],"environment":"Linux","impact":"All users","recommended_fix":"Fix","proposed_test_case":"test","block_auto_promotion":{"blocked":true,"reason":"reason\n:::error:::injected"}},"comment":"## Triage Summary\n\nBug."}' \ + "::error:" + # --- Summary --- echo "" diff --git a/scripts/post-triage.sh b/scripts/post-triage.sh index 4156ecd5..7b49a185 100755 --- a/scripts/post-triage.sh +++ b/scripts/post-triage.sh @@ -507,6 +507,11 @@ fi ACTION=$(jq -r '.action' "${RESULT_FILE}") COMMENT=$(jq -r '.comment // empty' "${RESULT_FILE}") +if echo "${COMMENT}" | grep -q '```'; then + echo "::warning::Stripping fenced code blocks from triage comment" + COMMENT=$(echo "${COMMENT}" | sed '/^```/d') +fi + forge_validate_issue_url forge_parse_issue_url @@ -819,11 +824,18 @@ ${FAILED_CREATES}" # own the "bug,documentation,performance" default. An absent or unset # TRIAGE_AUTO_CODE_CATEGORIES means no categories auto-promote. # - # Workflow-change guard (#325): if triage detected that the fix requires - # modifying workflow files (.github/workflows/, .fullsend/.github/workflows/, - # or shim workflows), skip ready-to-code regardless of category. The code - # agent cannot modify workflow files under current permissions. - REQUIRES_WORKFLOW=$(jq -r '.triage_summary.requires_workflow_changes // false' "${RESULT_FILE}") + # Auto-promotion gate (#2207, #325): the triage agent can block + # auto-promotion via block_auto_promotion.blocked (e.g., high effort, + # workflow file changes). When blocked, bug/docs/performance categories + # receive triaged instead of ready-to-code, and the reason is appended + # to the comment. + BLOCKED=$(jq -r '.triage_summary.block_auto_promotion.blocked // false' "${RESULT_FILE}") + BLOCK_REASON=$(jq -r '.triage_summary.block_auto_promotion.reason // empty' "${RESULT_FILE}") + # Collapse runs of 2+ colons to a single colon so untrusted text + # can never form a GHA workflow command (e.g., ::error::). + while [[ "${BLOCK_REASON}" == *::* ]]; do + BLOCK_REASON="${BLOCK_REASON//::/:}" + done CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}") echo "Category: ${CATEGORY}" @@ -859,27 +871,31 @@ ${FAILED_CREATES}" AUTO_CODE_ALLOWED=false fi - # Workflow-change guard: if triage detected workflow file changes, always - # log the (#325) warning for operational visibility. Only block auto- - # promotion (apply triaged early) when the category would otherwise - # auto-promote to ready-to-code. - WORKFLOW_BLOCKED=false - if [[ "${REQUIRES_WORKFLOW}" == "true" ]]; then - echo "::warning::Triage detected workflow file changes required (#325)" - if [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then - echo "Applying triaged label (workflow changes required)..." - forge_add_label "triaged" - WORKFLOW_BLOCKED=true + # block_auto_promotion gate: when the agent sets blocked=true, apply + # triaged instead of ready-to-code and append the reason to the comment. + # Only relevant for categories that would otherwise auto-promote. + AUTO_PROMOTION_BLOCKED=false + if [[ "${BLOCKED}" == "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + echo "::warning::Skipping ready-to-code — auto-promotion blocked (see comment for details)" + echo "Applying triaged label (auto-promotion blocked)..." + forge_add_label "triaged" + AUTO_PROMOTION_BLOCKED=true + if [[ -z "${BLOCK_REASON}" ]]; then + BLOCK_REASON="No reason provided" fi + COMMENT="${COMMENT} + +--- +**Auto-promotion blocked:** ${BLOCK_REASON}" fi case "${CATEGORY}" in bug) echo "Applying bug label..." forge_add_label "bug" - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi @@ -887,19 +903,19 @@ ${FAILED_CREATES}" documentation) echo "Applying documentation label..." forge_add_label "documentation" - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi ;; performance) - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi diff --git a/scripts/post-triage.src.sh b/scripts/post-triage.src.sh index 963cef52..de52f0a1 100755 --- a/scripts/post-triage.src.sh +++ b/scripts/post-triage.src.sh @@ -67,6 +67,11 @@ fi ACTION=$(jq -r '.action' "${RESULT_FILE}") COMMENT=$(jq -r '.comment // empty' "${RESULT_FILE}") +if echo "${COMMENT}" | grep -q '```'; then + echo "::warning::Stripping fenced code blocks from triage comment" + COMMENT=$(echo "${COMMENT}" | sed '/^```/d') +fi + forge_validate_issue_url forge_parse_issue_url @@ -379,11 +384,18 @@ ${FAILED_CREATES}" # own the "bug,documentation,performance" default. An absent or unset # TRIAGE_AUTO_CODE_CATEGORIES means no categories auto-promote. # - # Workflow-change guard (#325): if triage detected that the fix requires - # modifying workflow files (.github/workflows/, .fullsend/.github/workflows/, - # or shim workflows), skip ready-to-code regardless of category. The code - # agent cannot modify workflow files under current permissions. - REQUIRES_WORKFLOW=$(jq -r '.triage_summary.requires_workflow_changes // false' "${RESULT_FILE}") + # Auto-promotion gate (#2207, #325): the triage agent can block + # auto-promotion via block_auto_promotion.blocked (e.g., high effort, + # workflow file changes). When blocked, bug/docs/performance categories + # receive triaged instead of ready-to-code, and the reason is appended + # to the comment. + BLOCKED=$(jq -r '.triage_summary.block_auto_promotion.blocked // false' "${RESULT_FILE}") + BLOCK_REASON=$(jq -r '.triage_summary.block_auto_promotion.reason // empty' "${RESULT_FILE}") + # Collapse runs of 2+ colons to a single colon so untrusted text + # can never form a GHA workflow command (e.g., ::error::). + while [[ "${BLOCK_REASON}" == *::* ]]; do + BLOCK_REASON="${BLOCK_REASON//::/:}" + done CATEGORY=$(jq -r '.triage_summary.category // "unknown"' "${RESULT_FILE}") echo "Category: ${CATEGORY}" @@ -419,27 +431,31 @@ ${FAILED_CREATES}" AUTO_CODE_ALLOWED=false fi - # Workflow-change guard: if triage detected workflow file changes, always - # log the (#325) warning for operational visibility. Only block auto- - # promotion (apply triaged early) when the category would otherwise - # auto-promote to ready-to-code. - WORKFLOW_BLOCKED=false - if [[ "${REQUIRES_WORKFLOW}" == "true" ]]; then - echo "::warning::Triage detected workflow file changes required (#325)" - if [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then - echo "Applying triaged label (workflow changes required)..." - forge_add_label "triaged" - WORKFLOW_BLOCKED=true + # block_auto_promotion gate: when the agent sets blocked=true, apply + # triaged instead of ready-to-code and append the reason to the comment. + # Only relevant for categories that would otherwise auto-promote. + AUTO_PROMOTION_BLOCKED=false + if [[ "${BLOCKED}" == "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + echo "::warning::Skipping ready-to-code — auto-promotion blocked (see comment for details)" + echo "Applying triaged label (auto-promotion blocked)..." + forge_add_label "triaged" + AUTO_PROMOTION_BLOCKED=true + if [[ -z "${BLOCK_REASON}" ]]; then + BLOCK_REASON="No reason provided" fi + COMMENT="${COMMENT} + +--- +**Auto-promotion blocked:** ${BLOCK_REASON}" fi case "${CATEGORY}" in bug) echo "Applying bug label..." forge_add_label "bug" - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi @@ -447,19 +463,19 @@ ${FAILED_CREATES}" documentation) echo "Applying documentation label..." forge_add_label "documentation" - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi ;; performance) - if [[ "${WORKFLOW_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then + if [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]] && [[ "${AUTO_CODE_ALLOWED}" == "true" ]]; then echo "Deferring ready-to-code label (${CATEGORY}) until after label_actions..." DEFERRED_LABEL="ready-to-code" - elif [[ "${WORKFLOW_BLOCKED}" != "true" ]]; then + elif [[ "${AUTO_PROMOTION_BLOCKED}" != "true" ]]; then echo "Applying triaged label (auto-code disabled for ${CATEGORY})..." forge_add_label "triaged" fi diff --git a/scripts/validate-output-schema-test.sh b/scripts/validate-output-schema-test.sh index 71289d2f..cb148491 100755 --- a/scripts/validate-output-schema-test.sh +++ b/scripts/validate-output-schema-test.sh @@ -297,6 +297,32 @@ run_test_custom_filename_output "nested-additional-property-shows-allowed" \ "false" \ "allowed properties: actionable, category, description, file, line, remediation, severity" +# --- block_auto_promotion schema tests (#2207) --- + +run_test "valid-sufficient-with-block-auto-promotion" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"blocked":true,"reason":"Estimated effort 4.3/5"}},"comment":"Triage complete."}' \ + "true" + +run_test "valid-sufficient-with-unblocked-auto-promotion" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"blocked":false,"reason":"Low effort"}},"comment":"Triage complete."}' \ + "true" + +run_test "block-auto-promotion-missing-reason-rejected" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"blocked":true}},"comment":"Triage complete."}' \ + "false" + +run_test "block-auto-promotion-missing-blocked-rejected" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"reason":"High effort"}},"comment":"Triage complete."}' \ + "false" + +run_test "block-auto-promotion-empty-reason-rejected" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"blocked":true,"reason":""}},"comment":"Triage complete."}' \ + "false" + +run_test "block-auto-promotion-extra-field-rejected" \ + '{"action":"sufficient","reasoning":"clear","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix ptr","proposed_test_case":"test_fix","block_auto_promotion":{"blocked":true,"reason":"High effort","effort_score":2.5}},"comment":"Triage complete."}' \ + "false" + # --- Structural failures --- run_test "missing-action" \ diff --git a/skills/effort-estimation/SKILL.md b/skills/effort-estimation/SKILL.md new file mode 100644 index 00000000..b8ade4e4 --- /dev/null +++ b/skills/effort-estimation/SKILL.md @@ -0,0 +1,83 @@ +--- +name: effort-estimation +description: >- + Score implementation effort for triaged issues and decide whether to block + auto-promotion to the code agent. Produces a block_auto_promotion object + in the triage result. +--- + +# Effort Estimation + +Estimate the implementation effort for the issue being triaged and decide +whether it should be held for human review instead of auto-promoting to +the code agent. This skill applies only when the triage action is `sufficient`. + +## Step 1: Gather signals from the codebase + +Base your estimate on what you observe in the repository, not on the +reporter's claims about difficulty. Reporters may underestimate or +overestimate effort. + +## Step 2: Score effort + +Rate the issue on each dimension using a 1--5 scale. + +**Scope:** +1. Single line/file, isolated change +2. One component, a few files +3. Multiple components or cross-cutting +4. Architectural change, many files/packages +5. System-wide redesign or multi-repo change + +**Testing:** +1. Existing tests cover the fix +2. Minor test additions +3. New test suite or fixtures needed +4. Test infrastructure changes required +5. New testing strategy or framework needed + +**Domain knowledge:** +1. Obvious from error message +2. Requires reading surrounding code +3. Requires understanding subsystem design +4. Requires cross-repo or external API knowledge +5. Requires domain expertise outside the team + +**Risk:** +1. No behavior change for other callers +2. Low risk of regression +3. Moderate regression surface +4. High regression risk, needs careful rollout +5. Breaking change affecting downstream consumers + +Compute the overall effort as the average of the four dimensions, rounded +to one decimal place. + +## Step 3: Derive the review decision + +If the overall effort score is >= 4, the issue requires human review +before the code agent is dispatched. + +## Output + +Populate the `block_auto_promotion` field in `triage_summary`: + +```json +"block_auto_promotion": { + "blocked": true, + "reason": "Estimated effort 4.3/5 (multi-component fix with new test fixtures needed)" +} +``` + +When `blocked` is `false`, set `reason` to a short explanation of why +auto-promotion is safe: + +```json +"block_auto_promotion": { + "blocked": false, + "reason": "Low effort (1.3/5); single-file fix with existing test coverage" +} +``` + +The `reason` is appended to the triage comment when auto-promotion is +blocked, so write it for a human maintainer audience.