-
Notifications
You must be signed in to change notification settings - Fork 12
feat(triage): effort-estimation skill and auto-promotion gate #646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] "Blocking auto-promotion" section overstates This line tells the model it can set Suggested fix: Qualify the doc to state explicitly that the reason-append/triaged-override behavior only applies to bug/documentation/performance categories, matching the |
||
| - 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. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Domain-knowledge annotation score contradicts the skill's own rubric, making this fixture potentially flaky at the >=4 threshold This annotation scores "Domain knowledge... coordinating invalidation across the session store, auth middleware, and rate limiter" as a 4. But Suggested fix: Either revise the justification to genuinely require cross-repo/external-API knowledge (matching level 4), or lower the expected score to 3 and re-verify the threshold math with the other three dimensions still yields >=4. |
||
| (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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
Uh oh!
There was an error while loading. Please reload this page.