fix:agent clients lack timeout and retry logic hang indefinitely when server is unresponsive#701
Conversation
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ngs to client configuration Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…utability Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…onment Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ecurity, and analysis Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…tasks_controller Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ntions Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
dosubot
Bot
added
the
size:XXL
…ic-hang-indefinitely-when-server-is-unresponsive
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds many agent-specification docs and tooling configs for a "tessl" MCP server, introduces an unauthenticated client health endpoint and client resilience configuration, refactors task-assignment logic and agent activation, adds an expire-benchmarks UI/action and route, updates OpenAPI/specs/tests, adjusts broadcasts/caching in Agent model/views, and upgrades dependencies and CI workflows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Agent Client
participant API as Client Task API
participant Service as TaskAssignmentService
participant Agent as Agent Model
participant DB as Database
participant UI as Browser UI
Client->>API: GET /api/v1/client/tasks/new
API->>Service: find_next_task()
Service->>DB: scoped query (project filter?, FOR UPDATE SKIP LOCKED)
DB-->>Service: task or nil
Service-->>API: task payload
API->>Agent: activate_pending_agent_with_benchmarks() (side effect)
Agent->>DB: record activation / log
API-->>Client: task response
UI->>API: POST /agents/:id/expire_benchmarks
API->>Agent: delete hashcat_benchmarks
Agent->>Agent: check_benchmark_age()
API-->>UI: redirect to agent#show with notice
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Comment |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 CI must passWonderful, this rule succeeded.All CI checks must pass. This protection prevents manual merges that bypass the merge queue.
🟢 Do not merge outdated PRsWonderful, this rule succeeded.Make sure PRs are within 10 commits of the base branch before merging
|
There was a problem hiding this comment.
Pull request overview
This PR expands the client API to better support resilient agent connectivity (timeouts/retries/circuit breaker recommendations + a health probe), while also introducing several operational/tooling additions (Tessl/MCP config, new review agents) and some UI/admin improvements around agent benchmarks and Turbo broadcasts.
Changes:
- Add an unauthenticated client health endpoint and document it in OpenAPI + rswag.
- Expose server-recommended HTTP resilience settings from
GET /api/v1/client/configurationbacked byApplicationConfig. - Improve agent UI behavior (expire benchmarks action, safer fragment caching, reduced Turbo broadcast scope) and adjust task assignment to allow agents with no project assignments to work across projects.
Reviewed changes
Copilot reviewed 64 out of 65 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tessl.json | Adds Tessl dependency pin/config. |
| swagger/v1/swagger.json | Adds ErrorMetadata schema refactor; documents new health endpoint; adds resilience config fields. |
| spec/swagger_helper.rb | Defines ErrorMetadata; adds DevSkim ignore annotations for MD5 wording. |
| spec/requests/api/v1/client_spec.rb | Updates configuration schema + asserts new resilience defaults in response. |
| spec/requests/api/v1/client/health_spec.rb | Adds rswag spec for unauthenticated health endpoint. |
| spec/requests/api/v1/client/agents_spec.rb | Switches error metadata schema to $ref for reuse. |
| docs/api-reference-agent-auth.md | Expands API reference (TOC + resilience recommendation docs). |
| docs/README.md | Updates docs index description for agent API reference. |
| config/routes/client_api.rb | Routes new GET /api/v1/client/health. |
| config/routes.rb | Adds member route POST /agents/:id/expire_benchmarks. |
| config/configs/application_config.rb | Adds resilience-related config values + validation/coercion. |
| app/views/api/v1/client/configuration.json.jbuilder | Emits recommended_timeouts, recommended_retry, recommended_circuit_breaker. |
| app/views/agents/_capabilities_tab.html.erb | Adds “Expire Benchmarks” button; reworks caching to avoid safe_can? cache poisoning. |
| app/services/task_assignment_service.rb | Makes project scoping conditional so agents with no projects can work globally; refactors query building. |
| app/models/agent.rb | Adds targeted broadcast guards for configuration/capabilities tabs; removes model→service call for task assignment. |
| app/models/ability.rb | Grants expire_benchmarks on own agents. |
| app/controllers/api/v1/client/tasks_controller.rb | Uses TaskAssignmentService directly; auto-activates pending agents with benchmarks. |
| app/controllers/api/v1/client/health_controller.rb | Implements unauthenticated JSON health check with DB probe and degraded status handling. |
| app/controllers/agents_controller.rb | Adds expire_benchmarks action to clear benchmarks and force re-benchmarking. |
| Gemfile.lock | Updates multiple gem versions (dependency refresh). |
| Gemfile | Tightens/bumps gem version constraints to match lockfile. |
| GOTCHAS.md | Adds operational/testing/docs gotchas (rswag required props, unauth endpoints guidance, Turbo cache + safe_can?, etc.). |
| AGENTS.md | Updates test DB URL guidance; documents unauth endpoint patterns and architectural constraints. |
| .tessl/.gitignore | Ignores Tessl-managed artifacts. |
| .mdformat.toml | Excludes agent/skills directories from formatting; aligns config formatting. |
| .mcp.json | Adds MCP server config for Tessl. |
| .github/workflows/dep.yml | Adds Dependency Review workflow. |
| .github/workflows/check_security_vulnerability.yml | Adds DevSkim scan workflow + SARIF upload. |
| .github/skills/.gitignore | Ignores Tessl-managed skill files. |
| .github/agents/spec-flow-analyzer.agent.md | Adds reviewer agent definition. |
| .github/agents/security-sentinel.agent.md | Adds reviewer agent definition. |
| .github/agents/schema-drift-detector.agent.md | Adds reviewer agent definition. |
| .github/agents/repo-research-analyst.agent.md | Adds reviewer agent definition. |
| .github/agents/pr-comment-resolver.agent.md | Adds reviewer agent definition. |
| .github/agents/performance-oracle.agent.md | Adds reviewer agent definition. |
| .github/agents/pattern-recognition-specialist.agent.md | Adds reviewer agent definition. |
| .github/agents/lint.agent.md | Adds reviewer agent definition. |
| .github/agents/learnings-researcher.agent.md | Adds reviewer agent definition. |
| .github/agents/kieran-typescript-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/kieran-rails-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/kieran-python-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/julik-frontend-races-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/git-history-analyzer.agent.md | Adds reviewer agent definition. |
| .github/agents/framework-docs-researcher.agent.md | Adds reviewer agent definition. |
| .github/agents/figma-design-sync.agent.md | Adds reviewer agent definition. |
| .github/agents/dhh-rails-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/design-iterator.agent.md | Adds reviewer agent definition. |
| .github/agents/design-implementation-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/deployment-verification-agent.agent.md | Adds reviewer agent definition. |
| .github/agents/data-migration-expert.agent.md | Adds reviewer agent definition. |
| .github/agents/data-integrity-guardian.agent.md | Adds reviewer agent definition. |
| .github/agents/code-simplicity-reviewer.agent.md | Adds reviewer agent definition. |
| .github/agents/bug-reproduction-validator.agent.md | Adds reviewer agent definition. |
| .github/agents/best-practices-researcher.agent.md | Adds reviewer agent definition. |
| .github/agents/architecture-strategist.agent.md | Adds reviewer agent definition. |
| .github/agents/ankane-readme-writer.agent.md | Adds reviewer agent definition. |
| .github/agents/agent-native-reviewer.agent.md | Adds reviewer agent definition. |
| .gemini/skills/.gitignore | Ignores Tessl-managed skill files. |
| .gemini/settings.json | Adds Tessl MCP server config. |
| .cursor/skills/.gitignore | Ignores Tessl-managed skill files. |
| .cursor/mcp.json | Adds Tessl MCP server config. |
| .codex/skills/.gitignore | Ignores Tessl-managed skill files. |
| .codex/config.toml | Adds Tessl MCP server config. |
| .claude/skills/.gitignore | Ignores Tessl-managed skill files. |
| .claude/settings.json | Enables specified Claude plugins. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| "name": "my-project", | ||
| "dependencies": { |
There was a problem hiding this comment.
PR title/description don’t match the actual code changes: this PR adds client resilience recommendations + a new unauthenticated health endpoint, plus UI/admin changes (expire benchmarks, broadcast behavior) and broad dependency/workflow updates. Consider updating the PR description (or splitting into smaller PRs) so reviewers and release notes reflect the real scope.
There was a problem hiding this comment.
Acknowledged. The PR scope grew organically from agent resilience work into related agent lifecycle improvements. The PR description has been updated to reflect the actual scope.
| # Fields whose changes should trigger a configuration tab broadcast. | ||
| CONFIGURATION_BROADCAST_FIELDS = %w[ | ||
| enabled client_signature last_ipaddress advanced_configuration | ||
| custom_label operating_system | ||
| ].freeze |
There was a problem hiding this comment.
CONFIGURATION_BROADCAST_FIELDS doesn’t include fields rendered in the configuration tab like user_id (and potentially other displayed attributes). If an admin reassigns an agent to a different user, broadcast_tab_updates won’t refresh the configuration tab, leaving stale UI until a manual reload. Consider adding any configuration-tab-affecting attributes (e.g., user_id) to this list or switching to a more explicit predicate for when to broadcast configuration updates.
There was a problem hiding this comment.
Good catch — added user_id to CONFIGURATION_BROADCAST_FIELDS in this push.
| def activate_pending_agent_with_benchmarks | ||
| return unless @agent.pending? && @agent.hashcat_benchmarks.exists? | ||
| return unless @agent.activate | ||
|
|
||
| Rails.logger.info( | ||
| "[AgentLifecycle] auto_activate: agent_id=#{@agent.id} " \ | ||
| "reason=pending_with_benchmarks benchmark_count=#{@agent.hashcat_benchmarks.count} " \ | ||
| "timestamp=#{Time.zone.now}" | ||
| ) |
There was a problem hiding this comment.
activate_pending_agent_with_benchmarks does an exists? check and then logs @agent.hashcat_benchmarks.count, which issues an additional query. Since this endpoint can be hit frequently, consider collapsing this to a single query (e.g., fetch count once and use it both for the predicate and logging) to avoid extra DB load.
There was a problem hiding this comment.
Good catch — collapsed the exists? + count into a single count query, reusing the result for both the predicate and logging.
| "200": { | ||
| "description": "healthy", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "status": { | ||
| "type": "string", | ||
| "description": "Overall health status (ok or degraded)" | ||
| }, | ||
| "api_version": { | ||
| "type": "integer", | ||
| "description": "API version" | ||
| }, | ||
| "timestamp": { | ||
| "type": "string", | ||
| "format": "date-time", | ||
| "description": "Server timestamp" | ||
| }, | ||
| "database": { | ||
| "type": "string", | ||
| "description": "Database health (healthy or unhealthy)" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "status", | ||
| "api_version", | ||
| "timestamp", | ||
| "database" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
GET /api/v1/client/health can return 503 Service Unavailable when status is degraded (per the controller), but the OpenAPI spec only documents a 200 response. Add a 503 response (with the same schema) so clients and generated SDKs can handle degraded health correctly.
There was a problem hiding this comment.
Fixed — added a response(503, "degraded") block to the rswag spec. Swagger will be regenerated in this push.
| response(200, "healthy") do | ||
| schema type: :object, | ||
| properties: { | ||
| status: { type: :string, description: "Overall health status (ok or degraded)" }, | ||
| api_version: { type: :integer, description: "API version" }, | ||
| timestamp: { type: :string, format: "date-time", description: "Server timestamp" }, | ||
| database: { type: :string, description: "Database health (healthy or unhealthy)" } | ||
| }, | ||
| required: %i[status api_version timestamp database] |
There was a problem hiding this comment.
The health endpoint implementation returns 503 when the database check fails, but the rswag spec only defines/tests the 200 response. Add a response(503, ...) case (stubbing the DB check to fail) so the documented behavior stays in sync with the controller.
There was a problem hiding this comment.
Fixed — added a response(503, ...) rswag test case that stubs the DB check to fail.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (13)
.codex/config.toml (1)
1-4: Droptype = "stdio"from the Codex MCP stanza.Codex’s config reference and sample config define MCP transport implicitly: stdio servers use
command/args, and HTTP servers useurl. I don’t seemcp_servers.<id>.typein the documented schema, so this key risks being ignored or rejected by some Codex builds. (developers.openai.com)Suggested change
[mcp_servers.tessl] -type = "stdio" command = "tessl" args = [ "mcp", "start" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/config.toml around lines 1 - 4, Remove the unsupported "type = \"stdio\"" key from the Codex MCP stanza [mcp_servers.tessl]; retain the existing "command = \"tessl\"" and "args = [ \"mcp\", \"start\" ]" lines so the MCP transport is defined implicitly via command/args as per the Codex config schema..github/workflows/dep.yml (1)
17-17: Avoid introducing a deprecated license filter in a new workflow.Line 17 uses
deny-licenses, which the action docs mark as deprecated for possible removal in a future major release. I'd switch this new workflow toallow-licensesor move the policy into an external config before this becomes upgrade debt. (github.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/dep.yml at line 17, The workflow uses the deprecated deny-licenses key; update the dependency-review action configuration by removing or replacing deny-licenses and instead use allow-licenses (or move license policy into an external configuration file consumed by the action) so the new workflow doesn't introduce deprecated configuration; search for the deny-licenses entry and replace it with an appropriate allow-licenses whitelist or reference to an external policy per the dependency-review-action docs..mdformat.toml (1)
10-13: Cover the other Tessl-managed skill directories here too.Lines 10-13 exclude
.github/*and.kiro/*, but this PR also introduces Tessl-managed content under.claude/skills,.codex/skills,.cursor/skills, and.gemini/skills. Leaving those out meansmdformatcan still rewrite generated files there and create avoidable churn.Suggested diff
exclude = [ "venv/**", "**/node_modules/**", "**/*.txt", "**/*.mdc", "**/*.tpl.md", "**/CHANGELOG.md", "target/**", "megalinter-reports/**", ".github/agents/**", ".github/skills/**", + ".claude/skills/**", + ".codex/skills/**", + ".cursor/skills/**", + ".gemini/skills/**", ".kiro/agents/**", ".kiro/skills/**", ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.mdformat.toml around lines 10 - 13, The mdformat exclude list currently includes ".github/agents/**", ".github/skills/**", ".kiro/agents/**", and ".kiro/skills/**" but misses other Tessl-managed directories, so update the exclude patterns to also cover the introduced generated directories (e.g. add ".claude/skills/**", ".claude/agents/**", ".codex/skills/**", ".codex/agents/**", ".cursor/skills/**", ".cursor/agents/**", ".gemini/skills/**", ".gemini/agents/**") so mdformat won't rewrite generated files there; modify the exclusion array in .mdformat.toml to include these new glob patterns alongside the existing ".github/*" and ".kiro/*" entries..github/agents/lint.agent.md (1)
13-17: Separate Brakeman findings from the lint auto-fix path.
standardrb/erblintcan safely flow into auto-fix + style commits, butbin/brakemanfindings are security issues and usually need deliberate review. Folding them into “Take Action: Commit fixes withstyle: linting” risks misclassifying security remediations as cosmetic changes.Suggested wording
2. **Execute Appropriate Tools**: - For Ruby files: `bundle exec standardrb` for checking, `bundle exec standardrb --fix` for auto-fixing - For ERB templates: `bundle exec erblint --lint-all` for checking, `bundle exec erblint --lint-all --autocorrect` for auto-fixing - - For security: `bin/brakeman` for vulnerability scanning + - For security: `bin/brakeman` for vulnerability scanning and separate reporting 3. **Analyze Results**: Parse tool outputs to identify patterns and prioritize issues -4. **Take Action**: Commit fixes with `style: linting` +4. **Take Action**: + - Auto-fix and commit lint-only issues with `style: linting` + - Report Brakeman findings separately for manual review/remediation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/agents/lint.agent.md around lines 13 - 17, Separate Brakeman security findings from the auto-fix lint flow: update the "Take Action" guidance so that `standardrb` and `erblint` auto-fixes still commit with `style: linting`, but `bin/brakeman` results are routed to a distinct review/PR process (e.g., label or branch) requiring manual triage and a different commit message or workflow, and ensure the doc mentions `bin/brakeman` explicitly as not to be auto-fixed or committed under `style: linting`..github/agents/pr-comment-resolver.agent.md (1)
47-58: Require targeted validation before reporting “Resolved.”The current flow lets the agent confirm resolution after a manual double-check, but without running the narrowest relevant tests/lints. For PR feedback that changes behavior, that is too easy to mark resolved prematurely.
Suggested wording
4. **Verify the Resolution**: After making changes: - Double-check that the change addresses the original comment - Ensure no unintended modifications were made - Verify the code still follows project conventions + - Run the narrowest relevant validation available (tests, lint, type checks) before reporting success 5. **Report the Resolution**: Provide a clear, concise summary that includes: @@ - - A confirmation that the issue has been resolved + - A confirmation that the issue has been resolved, including what validation was run🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/agents/pr-comment-resolver.agent.md around lines 47 - 58, Update the "Verify the Resolution" and "Report the Resolution" steps to require running targeted validation (e.g., the smallest reproducing unit of tests, focused linters, or a specific test file/command) before marking a comment as resolved; specifically, modify the text under the "Verify the Resolution" and "Report the Resolution" headings to instruct the agent to run the narrowest relevant tests/lints and confirm they pass, then include in the "Report the Resolution" summary which exact test/lint command or file was run and its result before stating the issue is resolved..github/workflows/check_security_vulnerability.yml (1)
29-36: Action versions are valid and secure, but consider using v1 tag for DevSkim.Both versions are current:
microsoft/DevSkim-Action@v1.0.16(latest as of May 2025) andgithub/codeql-action/upload-sarif@v4.32.6(latest as of March 2026) have no known vulnerabilities. However, the DevSkim-Action README recommends pinning to the major-version tagv1instead of the specific release to benefit from periodic security patches while maintaining compatibility. The current pinned version is safe for reproducibility; switching tov1would improve the security update posture.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_security_vulnerability.yml around lines 29 - 36, Update the DevSkim action version string to use the major-version tag recommended by the README: replace the hard-pinned reference "microsoft/DevSkim-Action@v1.0.16" with "microsoft/DevSkim-Action@v1" (leave the github/codeql-action/upload-sarif@v4.32.6 step unchanged); this keeps reproducibility while allowing upstream security patches for the DevSkim action.config/configs/application_config.rb (1)
110-118: Validation is correct but consider zero as an edge case.The validation correctly ensures positive integers. One consideration:
recommended_retry_initial_delay: 0might be a valid configuration for "no delay between retries" in some use cases. If zero should be allowed for delay-related settings, consider usingvalue >= 0for those specific attributes.💡 Optional: Allow zero for delay settings if needed
If zero-delay retries are a valid use case:
def validate_resilience_settings + non_zero_attrs = RESILIENCE_ATTRIBUTES - %i[recommended_retry_initial_delay] RESILIENCE_ATTRIBUTES.each do |attr| value = public_send(attr) - unless value.is_a?(Integer) && value.positive? + min_value = non_zero_attrs.include?(attr) ? 1 : 0 + unless value.is_a?(Integer) && value >= min_value raise Anyway::Config::ValidationError, "#{attr} must be a positive integer, got: #{value.inspect}" end end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/configs/application_config.rb` around lines 110 - 118, The current validate_resilience_settings method rejects zero for all RESILIENCE_ATTRIBUTES; if some attributes (e.g. recommended_retry_initial_delay or other delay-related keys) should allow zero, update validate_resilience_settings to treat those specially: define a small set/array like zero_allowed_attributes (including recommended_retry_initial_delay) and when iterating RESILIENCE_ATTRIBUTES use value.is_a?(Integer) && (zero_allowed_attributes.include?(attr) ? value >= 0 : value.positive?) to validate; keep raising the same Anyway::Config::ValidationError with the existing message when validation fails so behavior is unchanged for other attributes..github/agents/figma-design-sync.agent.md (1)
79-82: Scope this rule to layout sections instead of every component.Making
w-fulland parent-owned padding/max-width an absolute requirement will push the agent to strip legitimate sizing contracts from reusable leaf components, not just page sections. I'd phrase this as the default for top-level layout/section components, or “unless the component’s existing contract says otherwise.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/agents/figma-design-sync.agent.md around lines 79 - 82, Update the rule text in the bullet list that currently mandates "Components should ALWAYS be full width (`w-full`)..." etc., to scope it to top-level layout/section components rather than every component; rephrase to say something like "Top-level layout/section components should default to full width (`w-full`) and avoid outer padding/max-width—unless the component's existing sizing contract specifies otherwise," and similarly clarify that width constraints and horizontal padding should typically be handled by parent wrapper divs for those top-level sections; edit the same bullet group so the guidance applies to section/layout components only and preserves exceptions for reusable leaf components.config/routes/client_api.rb (1)
10-30: Update the route documentation comment to include the new health endpoint.The route comment block (lines 10-28) documents all routes but doesn't include the newly added health endpoint. Consider adding:
# - GET /client/health -> health#index🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/routes/client_api.rb` around lines 10 - 30, Update the top comment block documenting routes to include the new health endpoint by adding a line like "# - GET /client/health -> health#index" so the documented routes reflect the actual namespace :client route and the health#index action; modify the comment near the existing route list (where other client routes are documented) to include this entry.app/controllers/agents_controller.rb (2)
78-87: Consider alphabetical method ordering.Per coding guidelines, methods should be in alphabetical order except for
initializeand CRUD actions.expire_benchmarksshould be placed aftereditand beforeindexto maintain alphabetical ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/agents_controller.rb` around lines 78 - 87, Move the expire_benchmarks method within AgentsController so methods follow the project's alphabetical ordering rules (exceptions: initialize and standard CRUD actions); specifically, relocate def expire_benchmarks to sit after the def edit method and before the def index method in the class, keeping the method body and existing redirect/notice behavior unchanged.
78-87: Add YARD documentation for the publicexpire_benchmarksaction.Per coding guidelines, all public methods should be documented using YARD syntax.
📝 Proposed YARD documentation
+ # POST /agents/1/expire_benchmarks + # + # Deletes all benchmarks for the agent and transitions it to pending, + # forcing the agent to re-benchmark on its next heartbeat. + # + # `@return` [void] Redirects to the agent's capabilities tab with a notice. def expire_benchmarks `@agent.hashcat_benchmarks.destroy_all` `@agent.check_benchmark_age`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/agents_controller.rb` around lines 78 - 87, Add a YARD docblock above the public controller action expire_benchmarks describing its behavior (deletes all hashcat benchmarks for the agent and transitions it to pending so it re-benchmarks on next heartbeat), include tags such as `@return` [void] (or `@return` [ActionController::Redirecting] per project conventions) and `@route` or `@see` if your style guide requires route information, and indicate any side effects (redirects to agent_url with anchor "capabilities" and a notice). Use the method name expire_benchmarks to locate the action and place the YARD comment immediately above its def line.app/controllers/api/v1/client/health_controller.rb (1)
24-39: Add YARD documentation for theindexmethod.Per coding guidelines, all public methods should be documented using YARD syntax.
📝 Proposed YARD documentation
+ # Returns server health status for agent connectivity checks. + # + # Performs a lightweight database connectivity test and returns + # health status as JSON. No authentication required. + # + # `@return` [void] Renders JSON with status, api_version, timestamp, and database health. + # Returns HTTP 200 when healthy, 503 when degraded. # GET /api/v1/client/health def index🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/api/v1/client/health_controller.rb` around lines 24 - 39, Add a YARD docstring above the public index method in HealthController describing its purpose, HTTP route (GET /api/v1/client/health), return type (renders JSON), keys returned in the payload (status, api_version, timestamp, database), and possible HTTP status codes (:ok and :service_unavailable) — target the index method symbol and place the `@return` and `@note` tags as appropriate so the index method is fully documented per YARD standards.spec/requests/api/v1/client/health_spec.rb (1)
19-51: Consider adding a test for the degraded/503 response.The health controller returns a 503 status when the database is unhealthy. Consider adding a test case for this scenario to ensure the degraded state is properly documented in the OpenAPI spec.
🧪 Proposed test for degraded state
response(503, "degraded") do schema type: :object, properties: { status: { type: :string, description: "Overall health status (ok or degraded)" }, api_version: { type: :integer, description: "API version" }, timestamp: { type: :string, format: "date-time", description: "Server timestamp" }, database: { type: :string, description: "Database health (healthy or unhealthy)" } }, required: %i[status api_version timestamp database] before do allow(ActiveRecord::Base.connection).to receive(:execute).and_raise(StandardError.new("Connection failed")) end run_test! do expect(response).to have_http_status(:service_unavailable) data = JSON.parse(response.body, symbolize_names: true) expect(data[:status]).to eq("degraded") expect(data[:database]).to eq("unhealthy") end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/requests/api/v1/client/health_spec.rb` around lines 19 - 51, Add a new response(503, "degraded") test block next to the existing response(200, "healthy") in spec/requests/api/v1/client/health_spec.rb that mirrors the healthy schema, but stubs the DB check to fail; inside the block use a before hook to make ActiveRecord::Base.connection.execute raise (e.g. StandardError("Connection failed")), call run_test!, and assert expect(response).to have_http_status(:service_unavailable) and that the parsed response body has status "degraded" and database "unhealthy" (keeping the same timestamp and api_version assertions as appropriate).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/agents/ankane-readme-writer.agent.md:
- Around line 3-5: The agent's config currently grants broad tool access via the
"tools: '*'" and allows automatic scope granting with "infer: true"; restrict
this by replacing "tools: '*'" with an explicit minimal list of
documentation-related tools (e.g., the specific read and write tools your
platform uses for editing README files) and set "infer: false" so scopes are not
auto-granted; update the keys in .github/agents/ankane-readme-writer.agent.md
(tools and infer) accordingly to only include the necessary read/write tools for
README edits.
- Around line 28-32: Add an explicit rule to the numbered prompt list that
preserves existing project-specific sections unless the user explicitly requests
their removal: update the list around the existing items (the numbered rules 1–5
and the line "Preserve project-specific sections when reformatting existing
READMEs.") to include a new rule stating "Retain any nonstandard or
project-specific sections when reformatting; only remove them if the user
explicitly asks." Ensure the agent's "update" flow respects that rule (do not
drop custom sections by default) while keeping the other rules (imperative
voice, sentence length, exact section order, and "Remove ALL HTML comments")
intact.
- Line 68: Update the README template guidance to wrap placeholder tokens like
<gemname> and <user> in inline code formatting (e.g., `\<gemname\>`, `\<user\>`)
wherever they appear so they are rendered as literal placeholders instead of
being interpreted as HTML; search for the placeholder occurrences in the
ankane-readme-writer agent content and replace plain occurrences of "<gemname>"
and "<user>" with inline-code versions and ensure any similar tokens follow the
same inline code pattern.
In @.github/agents/deployment-verification-agent.agent.md:
- Around line 123-133: The code block under "Sample console verification"
contains a non-executable placeholder `old_column: [present values]`; update the
example so it can be pasted into a Rails console by replacing the placeholder
with a real predicate (e.g., use Record.where(new_column:
nil).where.not(old_column: nil).count) or move the placeholder text outside the
fenced ruby block and keep the console examples executable (targets: the
Record.where(...) and Record.order("RANDOM()").limit(10).pluck(...) lines).
In @.github/agents/git-history-analyzer.agent.md:
- Line 36: The current use of the literal command "git shortlog -sn --" returns
repo-wide authorship and must be scoped to the files being analyzed; change the
code that builds the git command (the invocation that currently uses "git
shortlog -sn --") to append the target path(s) (e.g., a variable like
targetPaths or pathsToAnalyze) after the "--", properly quoting/escaping
multiple paths, so the command becomes "git shortlog -sn -- <paths...>" and
handles single or multiple file/path inputs.
In @.github/agents/julik-frontend-races-reviewer.agent.md:
- Around line 19-24: Replace the incorrect agent name string
"julik-frontend-reviewer" in the example dialogue with the correct agent name
"julik-frontend-races-reviewer" so the prompt and assistant response match the
spec; update the quoted lines that mention the agent (e.g., the user/assistant
example block containing "julik-frontend-reviewer") to reference
"julik-frontend-races-reviewer" consistently.
- Around line 113-123: The timeout helper setTimeoutWithCancelation defines
handlerWithCancelation but mistakenly passes an undefined handler to setTimeout,
causing a ReferenceError; update the call inside setTimeoutWithCancelation to
pass handlerWithCancelation (and keep the existing cancelToken/cancel logic
intact) so the cancelable timer uses the correct function reference.
In @.github/agents/kieran-rails-reviewer.agent.md:
- Around line 18-25: The example block contains two "assistant:" entries inside
a single <example>, which breaks the one-response-per-example rule; update the
agent spec by splitting that <example> into two separate <example> blocks (one
with the assistant reply "I've refactored the EmailProcessor service to handle
attachments." and a second with "Let me have Kieran review these changes to the
EmailProcessor service.") or remove the redundant assistant line so each
<example> contains exactly one assistant response, and apply the same fix to the
similar occurrence mentioned around the other <example> (lines referenced in the
comment).
In @.github/agents/pattern-recognition-specialist.agent.md:
- Around line 38-43: The "Naming Convention Analysis" bullet merges "Constants
and configuration values" with the following instruction sentence; split this
into two list items so the list remains well-formed: keep the bullet for
"Constants and configuration values" as its own line and create a separate
bullet (or immediately following sentence) that starts with "Identify deviations
from established conventions and suggest improvements." Update the block
containing the heading "3. **Naming Convention Analysis**" and the sub-items
(Variables, Classes, Files, Constants) so each sub-item is a distinct list entry
and the instruction to identify deviations is not appended to the Constants
line.
In @.github/agents/performance-oracle.agent.md:
- Around line 3-5: Replace the broad tools wildcard under the YAML key tools
(currently set to '*') with the minimal, explicit read/search tool set the agent
actually needs (instead of tools: '*'); edit the same block where infer: true
appears to enumerate only the specific tool identifiers used for analysis (e.g.,
file.read, file.search or the repository search/read tool names your system
uses) so the agent has least privilege.
In @.github/workflows/dep.yml:
- Around line 4-6: The workflow's permissions block only grants "contents: read"
which prevents the dependency-review-action from creating PR comments when
"comment-summary-in-pr: on-failure" is enabled; update the permissions section
to add "pull-requests: write" (or remove/disable the "comment-summary-in-pr:
on-failure" setting) so the dependency review can post comments. Locate the
permissions stanza (the "permissions" key and its "contents: read" entry) and
either add "pull-requests: write" alongside it or change the
dependency-review-action configuration (the "comment-summary-in-pr" option) to a
mode that doesn't require PR write permissions.
In `@AGENTS.md`:
- Around line 380-385: The fenced code block around the example view test is
missing a language specifier (MD040); update the block so the opening fence is
"```ruby" and ensure it closes with "```" so the snippet (e.g., render partial:
"agents/configuration_tab", locals: { agent: agent } / expect(rendered).to
have_content("Configuration")) is treated as Ruby, or alternatively convert the
snippet into consistent bullet points if it was not intended as code; also
ensure the note about stubbing safe_can? remains formatted consistently with the
chosen style.
In `@docs/api-reference-agent-auth.md`:
- Around line 593-611: Update the retry guidance to avoid blind retries on
non-idempotent endpoints: explicitly call out that mutating POSTs (e.g.,
accept_task, submit_crack, submit_error) must not be retried blindly and either
should be excluded from the "Retry? Yes" cases in the table or require an
idempotency key/request ID; add text describing the required idempotency header
semantics and server behavior (dedupe by idempotency key) or instruct clients to
mark these endpoints as non-retryable and only retry after explicit confirmation
from the server; keep the exponential-backoff + jitter recommendation for
idempotent operations and 3xx/5xx/429 cases but ensure the table rows and
surrounding prose clearly differentiate idempotent vs non-idempotent endpoints.
- Around line 15-22: Add the missing liveness probe documentation for GET
/api/v1/client/health: insert a TOC entry under the "Agent Authentication" (or
immediately before auth/config entries) linking to a new "GET
/api/v1/client/health" section, and create that section describing the endpoint
purpose (client liveness probe), expected HTTP response codes (200 OK with a
simple health JSON payload, 503 on unhealthy), required headers (if any), and a
minimal example response body so agents know to call it before fetching
auth/config. Ensure the new section title matches "GET /api/v1/client/health"
and the TOC anchor matches that title.
- Around line 595-597: The fenced code block containing the expression "delay =
min(initial_delay * 2^attempt, max_delay) + random(0, delay * 0.5)" is missing a
language label; update the markdown fence to include a language (e.g., "text")
so markdownlint MD040 is satisfied by changing ``` to ```text for that block.
---
Nitpick comments:
In @.codex/config.toml:
- Around line 1-4: Remove the unsupported "type = \"stdio\"" key from the Codex
MCP stanza [mcp_servers.tessl]; retain the existing "command = \"tessl\"" and
"args = [ \"mcp\", \"start\" ]" lines so the MCP transport is defined implicitly
via command/args as per the Codex config schema.
In @.github/agents/figma-design-sync.agent.md:
- Around line 79-82: Update the rule text in the bullet list that currently
mandates "Components should ALWAYS be full width (`w-full`)..." etc., to scope
it to top-level layout/section components rather than every component; rephrase
to say something like "Top-level layout/section components should default to
full width (`w-full`) and avoid outer padding/max-width—unless the component's
existing sizing contract specifies otherwise," and similarly clarify that width
constraints and horizontal padding should typically be handled by parent wrapper
divs for those top-level sections; edit the same bullet group so the guidance
applies to section/layout components only and preserves exceptions for reusable
leaf components.
In @.github/agents/lint.agent.md:
- Around line 13-17: Separate Brakeman security findings from the auto-fix lint
flow: update the "Take Action" guidance so that `standardrb` and `erblint`
auto-fixes still commit with `style: linting`, but `bin/brakeman` results are
routed to a distinct review/PR process (e.g., label or branch) requiring manual
triage and a different commit message or workflow, and ensure the doc mentions
`bin/brakeman` explicitly as not to be auto-fixed or committed under `style:
linting`.
In @.github/agents/pr-comment-resolver.agent.md:
- Around line 47-58: Update the "Verify the Resolution" and "Report the
Resolution" steps to require running targeted validation (e.g., the smallest
reproducing unit of tests, focused linters, or a specific test file/command)
before marking a comment as resolved; specifically, modify the text under the
"Verify the Resolution" and "Report the Resolution" headings to instruct the
agent to run the narrowest relevant tests/lints and confirm they pass, then
include in the "Report the Resolution" summary which exact test/lint command or
file was run and its result before stating the issue is resolved.
In @.github/workflows/check_security_vulnerability.yml:
- Around line 29-36: Update the DevSkim action version string to use the
major-version tag recommended by the README: replace the hard-pinned reference
"microsoft/DevSkim-Action@v1.0.16" with "microsoft/DevSkim-Action@v1" (leave the
github/codeql-action/upload-sarif@v4.32.6 step unchanged); this keeps
reproducibility while allowing upstream security patches for the DevSkim action.
In @.github/workflows/dep.yml:
- Line 17: The workflow uses the deprecated deny-licenses key; update the
dependency-review action configuration by removing or replacing deny-licenses
and instead use allow-licenses (or move license policy into an external
configuration file consumed by the action) so the new workflow doesn't introduce
deprecated configuration; search for the deny-licenses entry and replace it with
an appropriate allow-licenses whitelist or reference to an external policy per
the dependency-review-action docs.
In @.mdformat.toml:
- Around line 10-13: The mdformat exclude list currently includes
".github/agents/**", ".github/skills/**", ".kiro/agents/**", and
".kiro/skills/**" but misses other Tessl-managed directories, so update the
exclude patterns to also cover the introduced generated directories (e.g. add
".claude/skills/**", ".claude/agents/**", ".codex/skills/**",
".codex/agents/**", ".cursor/skills/**", ".cursor/agents/**",
".gemini/skills/**", ".gemini/agents/**") so mdformat won't rewrite generated
files there; modify the exclusion array in .mdformat.toml to include these new
glob patterns alongside the existing ".github/*" and ".kiro/*" entries.
In `@app/controllers/agents_controller.rb`:
- Around line 78-87: Move the expire_benchmarks method within AgentsController
so methods follow the project's alphabetical ordering rules (exceptions:
initialize and standard CRUD actions); specifically, relocate def
expire_benchmarks to sit after the def edit method and before the def index
method in the class, keeping the method body and existing redirect/notice
behavior unchanged.
- Around line 78-87: Add a YARD docblock above the public controller action
expire_benchmarks describing its behavior (deletes all hashcat benchmarks for
the agent and transitions it to pending so it re-benchmarks on next heartbeat),
include tags such as `@return` [void] (or `@return` [ActionController::Redirecting]
per project conventions) and `@route` or `@see` if your style guide requires route
information, and indicate any side effects (redirects to agent_url with anchor
"capabilities" and a notice). Use the method name expire_benchmarks to locate
the action and place the YARD comment immediately above its def line.
In `@app/controllers/api/v1/client/health_controller.rb`:
- Around line 24-39: Add a YARD docstring above the public index method in
HealthController describing its purpose, HTTP route (GET /api/v1/client/health),
return type (renders JSON), keys returned in the payload (status, api_version,
timestamp, database), and possible HTTP status codes (:ok and
:service_unavailable) — target the index method symbol and place the `@return` and
`@note` tags as appropriate so the index method is fully documented per YARD
standards.
In `@config/configs/application_config.rb`:
- Around line 110-118: The current validate_resilience_settings method rejects
zero for all RESILIENCE_ATTRIBUTES; if some attributes (e.g.
recommended_retry_initial_delay or other delay-related keys) should allow zero,
update validate_resilience_settings to treat those specially: define a small
set/array like zero_allowed_attributes (including
recommended_retry_initial_delay) and when iterating RESILIENCE_ATTRIBUTES use
value.is_a?(Integer) && (zero_allowed_attributes.include?(attr) ? value >= 0 :
value.positive?) to validate; keep raising the same
Anyway::Config::ValidationError with the existing message when validation fails
so behavior is unchanged for other attributes.
In `@config/routes/client_api.rb`:
- Around line 10-30: Update the top comment block documenting routes to include
the new health endpoint by adding a line like "# - GET /client/health
-> health#index" so the documented routes reflect the actual namespace :client
route and the health#index action; modify the comment near the existing route
list (where other client routes are documented) to include this entry.
In `@spec/requests/api/v1/client/health_spec.rb`:
- Around line 19-51: Add a new response(503, "degraded") test block next to the
existing response(200, "healthy") in spec/requests/api/v1/client/health_spec.rb
that mirrors the healthy schema, but stubs the DB check to fail; inside the
block use a before hook to make ActiveRecord::Base.connection.execute raise
(e.g. StandardError("Connection failed")), call run_test!, and assert
expect(response).to have_http_status(:service_unavailable) and that the parsed
response body has status "degraded" and database "unhealthy" (keeping the same
timestamp and api_version assertions as appropriate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99cbcc5b-e53a-461a-9cfe-af410b62749d
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (64)
.claude/settings.json.claude/skills/.gitignore.codex/config.toml.codex/skills/.gitignore.cursor/mcp.json.cursor/skills/.gitignore.gemini/settings.json.gemini/skills/.gitignore.github/agents/agent-native-reviewer.agent.md.github/agents/ankane-readme-writer.agent.md.github/agents/architecture-strategist.agent.md.github/agents/best-practices-researcher.agent.md.github/agents/bug-reproduction-validator.agent.md.github/agents/code-simplicity-reviewer.agent.md.github/agents/data-integrity-guardian.agent.md.github/agents/data-migration-expert.agent.md.github/agents/deployment-verification-agent.agent.md.github/agents/design-implementation-reviewer.agent.md.github/agents/design-iterator.agent.md.github/agents/dhh-rails-reviewer.agent.md.github/agents/figma-design-sync.agent.md.github/agents/framework-docs-researcher.agent.md.github/agents/git-history-analyzer.agent.md.github/agents/julik-frontend-races-reviewer.agent.md.github/agents/kieran-python-reviewer.agent.md.github/agents/kieran-rails-reviewer.agent.md.github/agents/kieran-typescript-reviewer.agent.md.github/agents/learnings-researcher.agent.md.github/agents/lint.agent.md.github/agents/pattern-recognition-specialist.agent.md.github/agents/performance-oracle.agent.md.github/agents/pr-comment-resolver.agent.md.github/agents/repo-research-analyst.agent.md.github/agents/schema-drift-detector.agent.md.github/agents/security-sentinel.agent.md.github/agents/spec-flow-analyzer.agent.md.github/skills/.gitignore.github/workflows/check_security_vulnerability.yml.github/workflows/dep.yml.mcp.json.mdformat.toml.tessl/.gitignoreAGENTS.mdGOTCHAS.mdGemfileapp/controllers/agents_controller.rbapp/controllers/api/v1/client/health_controller.rbapp/controllers/api/v1/client/tasks_controller.rbapp/models/ability.rbapp/models/agent.rbapp/services/task_assignment_service.rbapp/views/agents/_capabilities_tab.html.erbapp/views/api/v1/client/configuration.json.jbuilderconfig/configs/application_config.rbconfig/routes.rbconfig/routes/client_api.rbdocs/README.mddocs/api-reference-agent-auth.mdspec/requests/api/v1/client/agents_spec.rbspec/requests/api/v1/client/health_spec.rbspec/requests/api/v1/client_spec.rbspec/swagger_helper.rbswagger/v1/swagger.jsontessl.json
| tools: | ||
| - '*' | ||
| infer: true |
There was a problem hiding this comment.
Restrict this agent’s tool access.
This agent only rewrites README content, but tools: ['*'] gives it every capability and infer: true allows that scope to be granted automatically. Please narrow this to the minimum read/write tools needed for documentation edits so a misfire cannot take unrelated actions.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 3-3: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 3-3: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/agents/ankane-readme-writer.agent.md around lines 3 - 5, The agent's
config currently grants broad tool access via the "tools: '*'" and allows
automatic scope granting with "infer: true"; restrict this by replacing "tools:
'*'" with an explicit minimal list of documentation-related tools (e.g., the
specific read and write tools your platform uses for editing README files) and
set "infer: false" so scopes are not auto-granted; update the keys in
.github/agents/ankane-readme-writer.agent.md (tools and infer) accordingly to
only include the necessary read/write tools for README edits.
There was a problem hiding this comment.
These agent spec files are auto-generated by tessl and managed externally. Changes should be made upstream in the tessl skill definitions, not in these generated files.
There was a problem hiding this comment.
@unclesp1d3r, understood — thanks for the context! I'll keep that in mind for future reviews and avoid flagging issues in these externally managed files.
✏️ Learnings added
Learnt from: unclesp1d3r
Repo: unclesp1d3r/CipherSwarm PR: 701
File: .github/agents/ankane-readme-writer.agent.md:3-5
Timestamp: 2026-03-12T16:15:25.979Z
Learning: In the CipherSwarm repository, files under `.github/agents/*.agent.md` (and similar agent spec files) are auto-generated and managed externally by Tessl. Any changes to these files (e.g., tool permissions, infer settings) must be made upstream in the Tessl skill definitions, not directly in the generated files. Do not raise review comments requesting edits to these files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| - Verify all sentences are 15 words or less | ||
| - Ensure all verbs are in imperative form | ||
| - Confirm sections appear in the correct order | ||
| - Check that all placeholder values (like <gemname>, <user>) are clearly marked |
There was a problem hiding this comment.
Wrap placeholder tokens in inline code.
Raw <gemname> / <user> placeholders can be treated as HTML in markdown output. Require inline code formatting here so generated README placeholders stay visible.
Proposed fix
-- Check that all placeholder values (like <gemname>, <user>) are clearly marked
+- Check that all placeholder values (like `<gemname>`, `<user>`) are clearly marked📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Check that all placeholder values (like <gemname>, <user>) are clearly marked | |
| - Check that all placeholder values (like `<gemname>`, `<user>`) are clearly marked |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/agents/ankane-readme-writer.agent.md at line 68, Update the README
template guidance to wrap placeholder tokens like <gemname> and <user> in inline
code formatting (e.g., `\<gemname\>`, `\<user\>`) wherever they appear so they
are rendered as literal placeholders instead of being interpreted as HTML;
search for the placeholder occurrences in the ankane-readme-writer agent content
and replace plain occurrences of "<gemname>" and "<user>" with inline-code
versions and ensure any similar tokens follow the same inline code pattern.
There was a problem hiding this comment.
These agent spec files are auto-generated by tessl and managed externally. Changes should be made upstream in the tessl skill definitions, not in these generated files.
There was a problem hiding this comment.
@unclesp1d3r, understood! I'll keep that in mind — since these files are auto-generated and managed by Tessl, any content fixes need to go upstream in the Tessl skill definitions rather than being applied directly here.
(˳˘ ɜ˘)˳ ♡
✏️ Learnings added
Learnt from: unclesp1d3r
Repo: unclesp1d3r/CipherSwarm PR: 701
File: .github/agents/ankane-readme-writer.agent.md:68-68
Timestamp: 2026-03-12T16:15:28.746Z
Learning: In the CipherSwarm repository, agent spec files under `.github/agents/` (e.g., `ankane-readme-writer.agent.md`, `architecture-strategist.agent.md`, etc.) are auto-generated and managed externally by Tessl. Changes to their content must be made upstream in the Tessl skill definitions, not directly in these generated files. Do not suggest direct edits to these files in code reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
|
||
| 3. **Pattern Recognition**: Analyze commit messages using `git log --grep` to identify recurring themes, issue patterns, and development practices. Look for keywords like 'fix', 'bug', 'refactor', 'performance', etc. | ||
|
|
||
| 4. **Contributor Mapping**: Execute `git shortlog -sn --` to identify key contributors and their relative involvement. Cross-reference with specific file changes to map expertise domains. |
There was a problem hiding this comment.
Scope git shortlog to the files being analyzed.
git shortlog -sn -- reports repository-wide authorship, so it won't support the file-level “expertise domains” mapping this agent is supposed to produce. Pass the target path(s) after --, otherwise the output will be mostly noise on a large repo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/agents/git-history-analyzer.agent.md at line 36, The current use of
the literal command "git shortlog -sn --" returns repo-wide authorship and must
be scoped to the files being analyzed; change the code that builds the git
command (the invocation that currently uses "git shortlog -sn --") to append the
target path(s) (e.g., a variable like targetPaths or pathsToAnalyze) after the
"--", properly quoting/escaping multiple paths, so the command becomes "git
shortlog -sn -- <paths...>" and handles single or multiple file/path inputs.
There was a problem hiding this comment.
These agent spec files are auto-generated by tessl and managed externally. Changes should be made upstream in the tessl skill definitions, not in these generated files.
There was a problem hiding this comment.
@unclesp1d3r, understood — I'll keep that in mind for future reviews and won't flag changes in those generated files.
✏️ Learnings added
Learnt from: unclesp1d3r
Repo: unclesp1d3r/CipherSwarm PR: 701
File: .github/agents/git-history-analyzer.agent.md:36-36
Timestamp: 2026-03-12T16:15:27.008Z
Learning: In the CipherSwarm repository (unclesp1d3r/CipherSwarm), files under `.github/agents/*.agent.md` (and similarly `.github/skills/`, `.claude/skills/`, `.codex/skills/`, `.cursor/skills/`, `.gemini/skills/`) are auto-generated and managed externally by Tessl. Review comments suggesting direct edits to these files are not actionable; any changes must be made upstream in the Tessl skill definitions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| **View Tests (spec/views/) — planned:** | ||
|
|
||
| - Partial rendering tests (e.g., agent configuration tab) | ||
| - Use `render partial:` with locals, assert on `rendered` | ||
| - Stub `safe_can?` when the partial uses authorization checks | ||
|
|
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block lacks a language specifier, which markdownlint flags as MD040.
📝 Proposed fix
**View Tests (spec/views/) — planned:**
-- Partial rendering tests (e.g., agent configuration tab)
-- Use `render partial:` with locals, assert on `rendered`
-- Stub `safe_can?` when the partial uses authorization checks
+```ruby
+# Example view test structure
+render partial: "agents/configuration_tab", locals: { agent: agent }
+expect(rendered).to have_content("Configuration")
+```
+
+- Stub `safe_can?` when the partial uses authorization checksAlternatively, if these are meant as bullet points rather than code, ensure the formatting is consistent.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 383-383: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 380 - 385, The fenced code block around the example
view test is missing a language specifier (MD040); update the block so the
opening fence is "```ruby" and ensure it closes with "```" so the snippet (e.g.,
render partial: "agents/configuration_tab", locals: { agent: agent } /
expect(rendered).to have_content("Configuration")) is treated as Ruby, or
alternatively convert the snippet into consistent bullet points if it was not
intended as code; also ensure the note about stubbing safe_can? remains
formatted consistently with the chosen style.
There was a problem hiding this comment.
False positive — these are bullet points, not a fenced code block. There is no unlabeled code fence in this section of AGENTS.md.
There was a problem hiding this comment.
@unclesp1d3r, you're right — I apologize for the false positive. Looking at the diff again, those are plain bullet points using inline code backticks (single backticks around render partial: and rendered), not a fenced code block. There is no unlabeled triple-backtick fence in this section, so the MD040 warning doesn't apply here. The markdownlint tool appears to have incorrectly flagged the inline backtick within the bullet point.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tessl.json (1)
5-9: Reduce secret-scanner noise for the pinned SHAs.These values look like intentional commit pins, but they will keep tripping generic secret detectors. Please add an allowlist/suppression for this manifest or these fields in the same PR so real secret findings do not get buried in repeat noise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tessl.json` around lines 5 - 9, These pinned commit SHAs in tessl.json (the "version" fields such as under "getsentry/skills") are intentional and causing secret-scanner noise; add a suppression/allowlist for this manifest and/or these specific JSON fields in the same PR so scanners ignore them—specifically update the secret-scanner config to whitelist the file name tessl.json and the JSON key "version" (or add a fingerprint/regex matching the pinned SHA format for these entries) and include a short justification comment for the suppression so future reviewers understand it is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tessl.json`:
- Around line 5-9: These pinned commit SHAs in tessl.json (the "version" fields
such as under "getsentry/skills") are intentional and causing secret-scanner
noise; add a suppression/allowlist for this manifest and/or these specific JSON
fields in the same PR so scanners ignore them—specifically update the
secret-scanner config to whitelist the file name tessl.json and the JSON key
"version" (or add a fingerprint/regex matching the pinned SHA format for these
entries) and include a short justification comment for the suppression so future
reviewers understand it is intentional.
🧪 CI InsightsHere's what we observed from your CI run for 2039a33. 🟢 All jobs passed!But CI Insights is watching 👀 |
…nd auto-activation Adds tests to satisfy undercover coverage requirements: - agents_spec: expire_benchmarks authorization and functionality - health_spec: database failure returns degraded status - tasks_spec: activate_pending_agent_with_benchmarks branch coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/requests/api/v1/client/tasks_spec.rb (1)
1044-1056: Consider verifying agent state when activation fails.The test verifies that the activation log is not emitted when
activatereturns false, but doesn't confirm the agent remains in thependingstate. Adding this assertion would strengthen the test.♻️ Optional: Add state verification
context "when activate fails" do it "does not log activation" do pending_agent = create(:agent, state: "pending") create(:hashcat_benchmark, agent: pending_agent) allow(Rails.logger).to receive(:info) allow_any_instance_of(Agent).to receive(:activate).and_return(false) # rubocop:disable RSpec/AnyInstance get "/api/v1/client/tasks/new", headers: { "Authorization" => "Bearer #{pending_agent.token}" } + pending_agent.reload + expect(pending_agent.state).to eq("pending") expect(Rails.logger).not_to have_received(:info).with(/\[AgentLifecycle\] auto_activate/) end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/requests/api/v1/client/tasks_spec.rb` around lines 1044 - 1056, The test should also assert the agent's state didn't change when activation fails: after issuing the GET request, reload the pending_agent and add an expectation that pending_agent.reload.state == "pending" (reference: the pending_agent factory/variable and Agent#activate method) to ensure the record remains in the pending state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/requests/api/v1/client/tasks_spec.rb`:
- Around line 1044-1056: The test should also assert the agent's state didn't
change when activation fails: after issuing the GET request, reload the
pending_agent and add an expectation that pending_agent.reload.state ==
"pending" (reference: the pending_agent factory/variable and Agent#activate
method) to ensure the record remains in the pending state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6d32b53-d307-42b6-a923-bd94666dbe76
📒 Files selected for processing (4)
spec/requests/agents_spec.rbspec/requests/api/v1/client/health_spec.rbspec/requests/api/v1/client/tasks_spec.rbtessl.json
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/requests/api/v1/client/health_spec.rb
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 67 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| scope = Attack.incomplete | ||
| .joins(campaign: { hash_list: :hash_type }) | ||
| .includes(campaign: %i[hash_list project]) | ||
| .where(hash_lists: { hash_type_id: allowed_hash_type_ids }) | ||
|
|
||
| scope = scope.where(campaigns: { project_id: agent.project_ids }) if agent.project_ids.present? | ||
|
|
||
| scope.order("campaigns.priority DESC, attacks.complexity_value, attacks.created_at") |
There was a problem hiding this comment.
Same multi-tenancy concern as above: when agent.project_ids is blank, available_attacks becomes effectively unscoped by project and may hand the agent work from campaigns in projects it should not access. If “no project assignments” is intended to mean “all projects the owning user can access”, scope to that set explicitly; otherwise keep the project filter mandatory.
There was a problem hiding this comment.
This is by design — agents with no project assignments are intentionally allowed to work on any project, matching the convention used by attack resources (word lists, rule lists, mask lists) and Task#compatible_agent?. See AGENTS.md under "Agent Task Assignment".
| # Returns attacks available for the agent based on projects and hash types. | ||
| # | ||
| # Agents with no project assignments can work on any project (same convention | ||
| # as attack resources like word lists, rule lists, and Task#agent_compatible?). | ||
| # |
There was a problem hiding this comment.
The inline documentation references Task#agent_compatible?, but there doesn’t appear to be any such method in the codebase. This makes the comment misleading for future maintainers; either update it to reference the actual compatibility logic/policy, or remove the reference.
There was a problem hiding this comment.
Fixed in this push — corrected to Task#compatible_agent?.
| response(200, "healthy") do | ||
| schema type: :object, | ||
| properties: { | ||
| status: { type: :string, description: "Overall health status (ok or degraded)" }, | ||
| api_version: { type: :integer, description: "API version" }, | ||
| timestamp: { type: :string, format: "date-time", description: "Server timestamp" }, | ||
| database: { type: :string, description: "Database health (healthy or unhealthy)" } | ||
| }, | ||
| required: %i[status api_version timestamp database] |
There was a problem hiding this comment.
This rswag spec (and the generated swagger/v1/swagger.json) documents only a 200 response, but the controller returns 503 Service Unavailable with {status:"degraded", database:"unhealthy"} when the DB check fails. Add a response(503, ...) block (with schema + example) so the OpenAPI accurately reflects runtime behavior, then regenerate the swagger JSON.
There was a problem hiding this comment.
Fixed in this push — added response(503, "degraded") rswag test case.
| <% if safe_can?(:expire_benchmarks, agent) %> | ||
| <%= button_to "Expire Benchmarks", | ||
| expire_benchmarks_agent_path(agent), | ||
| method: :post, | ||
| class: "btn btn-outline-warning btn-sm", | ||
| data: { turbo_confirm: "This will delete all benchmarks and force the agent to re-benchmark on its next check-in. Continue?" } %> | ||
| <% end %> |
There was a problem hiding this comment.
safe_can? is evaluated when this partial is rendered in Turbo Stream broadcasts (see Agent#broadcast_tab_updates), where there is no current_user/Warden context. In that environment safe_can? returns false, so the broadcasted update will remove the “Expire Benchmarks” button for all viewers, even if they’re authorized. Authorization-gated UI elements shouldn’t live inside broadcast-rendered partials; move the button outside the broadcasted fragment (or update only the benchmark table via broadcasts) and rely on controller authorization for enforcement.
| .joins(attack: { campaign: :hash_list }) | ||
| .where(hash_lists: { hash_type_id: allowed_hash_type_ids }) | ||
|
|
||
| scope = scope.where(campaigns: { project_id: agent.project_ids }) if agent.project_ids.present? | ||
|
|
||
| task = scope |
There was a problem hiding this comment.
When agent.project_ids is blank, this now omits any campaigns.project_id scoping, meaning an agent with no project assignments can claim paused tasks from any project. Given the app’s project-based multi-tenancy, this is a cross-tenant access risk. Consider scoping to the agent’s user-accessible projects (e.g., agent.user.all_project_ids) or requiring explicit project assignment before allowing task pickup.
There was a problem hiding this comment.
🧹 Nitpick comments (6)
docs/user-guide/troubleshooting-agents.md (6)
825-862: Fix protocol inconsistency and add language specifier.The curl examples use
http://(lines 830, 860, 888) while other sections usehttps://. For security best practices, consider:
- Using
https://consistently for production examples- Adding a note if
http://is intentional for local/development environmentsAlso, the bash code block at line 828 should specify
bashas the language for syntax highlighting.🔒 Proposed fix for security and consistency
- ```bash + ```bash # This endpoint does not require authentication - curl -v http://your-server/api/v1/client/health + curl -v https://your-server/api/v1/client/health ``` **Expected response (healthy server)**:Apply similar changes to other curl examples in this section (lines 860, 888).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 825 - 862, Update the curl examples under the "Test server health endpoint" and the "Verify resilience configuration is being fetched" code blocks to use https:// instead of http:// for production-safe examples, and add a brief parenthetical note stating that http:// may be used for local development only; also mark the first bash code fence with a language specifier (```bash) so the health-check block is highlighted correctly — locate these changes in the code blocks that contain the curl commands for /api/v1/client/health and /api/v1/client/configuration in the "Test server health endpoint" and subsequent verification steps.
205-211: Add language specifiers to code blocks and consider protocol consistency.
The bash code block at line 205 should specify
bashas the language for proper syntax highlighting. As per static analysis hints, fenced code blocks should have a language specified.The health endpoint example uses
http://(line 206) while the authentication example useshttps://(line 209). Consider using consistent protocols or adding a comment explaining when to use HTTP vs HTTPS to avoid user confusion.📝 Proposed fix
- ``` + ```bash # Test health endpoint (unauthenticated) - curl -v http://server.example.com/api/v1/client/health + curl -v https://server.example.com/api/v1/client/health # Test authentication curl -H "Authorization: Bearer <token>" \ https://server.example.com/api/v1/client/authenticate ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 205 - 211, Add a language specifier ("bash") to the fenced code block containing the curl examples and make the protocol consistent: update the health-check curl line ("curl -v http://server.example.com/api/v1/client/health") to use https to match the authenticate example ("curl -H \"Authorization: Bearer <token>\" https://server.example.com/api/v1/client/authenticate"), or alternatively add a brief inline comment before the two curl lines explaining when to use http vs https.
1119-1127: Ensure consistency with earlier health endpoint examples.This section duplicates the health endpoint check pattern from lines 205-211. For consistency:
- Add
bashlanguage specifier to the code block (line 1121)- Consider using
https://instead ofhttp://for production examples (line 1123)- The "Alternative" label (line 1125) is a nice touch that could be backported to the earlier example at line 208 for consistency
📝 Proposed fix for consistency
```bash # Check health endpoint (no authentication required) - curl -v http://server.example.com/api/v1/client/health + curl -v https://server.example.com/api/v1/client/health # Alternative: check authenticated endpoint curl -I https://server.example.com/api/v1/client/authenticate ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 1119 - 1127, Update the health-check code block to match earlier examples: add the bash code-fence language specifier, change the health endpoint URL in the curl command from http:// to https://, and preserve the "Alternative: check authenticated endpoint" label and its curl -I https://... line; also mirror this "Alternative" label and formatting in the earlier health-check example so both examples are consistent (look for the health endpoint check code blocks / health endpoint curl examples to apply these edits).
911-928: Add language specifier to YAML code block.The YAML configuration example at lines 915-926 is missing a language specifier. As per static analysis hints, fenced code blocks should specify their language for proper syntax highlighting and validation.
📝 Proposed fix
Server operators can tune resilience parameters without redeploying agents by modifying the application configuration: -```yaml +```yaml # config/application.yml (example) recommended_connect_timeout: 10 # seconds recommended_read_timeout: 30 # seconds🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 911 - 928, The fenced code block showing the server-side configuration is missing a language specifier; update the Markdown block that contains the YAML keys (e.g., recommended_connect_timeout, recommended_read_timeout, recommended_write_timeout, etc.) to start with ```yaml so the block becomes a YAML-annotated fence for proper highlighting and linting.
385-393: Add language specifiers to fenced code elements.The inline code references at lines 390-391 appear to be list items, not fenced code blocks. However, if these were intended as code blocks, they should specify a language. The static analysis tool flagged these lines - please verify the intended formatting.
If the intent is to show individual configuration keys as inline code within bullet points, the current markdown is correct. If these should be formatted as a code block showing the structure, consider:
📝 Alternative formatting
These parameters allow server operators to tune agent behavior without redeploying agent software. Agents should apply these values when first connecting and refresh them periodically (e.g., every 24 hours) by re-fetching the configuration endpoint. See the [Client Resilience Recommendations](../api-reference-agent-auth.md#client-resilience-recommendations) section of the API reference for implementation details. -Or keep as-is if the list format is preferred. The static analysis warning may be a false positive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 385 - 393, Verify whether the four timeout keys (connect_timeout, read_timeout, write_timeout, request_timeout) and the retry/circuit-breaker keys (max_attempts, initial_delay, max_delay, failure_threshold, timeout) were intended as inline code in the bullet list or as a fenced code block; if inline is intended, leave the bullets as-is, otherwise replace the bullets with a single fenced code block (specify a language such as yaml or json) showing the configuration structure containing those keys so the static analysis tool warning is resolved and the examples are properly syntax-highlighted.
815-824: Consider clarifying agent implementation requirements.The diagnostic steps provide clear guidance, but line 822-823 suggests checking for specific agent features:
cipherswarm-agent --version cipherswarm-agent --featuresThese commands assume a specific agent CLI interface. Since agents can be implemented in various languages (the PR context mentions Go clients), consider adding a note that these are example commands and agents should consult their specific documentation for version/feature checking.
📝 Proposed clarification
1. **Check if agent supports resilience features**: ```bash - # Check agent version and features + # Check agent version and features (commands vary by agent implementation) cipherswarm-agent --version cipherswarm-agent --features ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user-guide/troubleshooting-agents.md` around lines 815 - 824, Update the diagnostic step that shows the CLI commands "cipherswarm-agent --version" and "cipherswarm-agent --features" to make clear these are example commands and may differ across agent implementations (e.g., Go clients or other language-specific CLIs); modify the text around those symbols to read something like "Check agent version and features (commands vary by agent implementation)" and add a short note telling readers to consult their agent's documentation for the exact commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/user-guide/troubleshooting-agents.md`:
- Around line 825-862: Update the curl examples under the "Test server health
endpoint" and the "Verify resilience configuration is being fetched" code blocks
to use https:// instead of http:// for production-safe examples, and add a brief
parenthetical note stating that http:// may be used for local development only;
also mark the first bash code fence with a language specifier (```bash) so the
health-check block is highlighted correctly — locate these changes in the code
blocks that contain the curl commands for /api/v1/client/health and
/api/v1/client/configuration in the "Test server health endpoint" and subsequent
verification steps.
- Around line 205-211: Add a language specifier ("bash") to the fenced code
block containing the curl examples and make the protocol consistent: update the
health-check curl line ("curl -v
http://server.example.com/api/v1/client/health") to use https to match the
authenticate example ("curl -H \"Authorization: Bearer <token>\"
https://server.example.com/api/v1/client/authenticate"), or alternatively add a
brief inline comment before the two curl lines explaining when to use http vs
https.
- Around line 1119-1127: Update the health-check code block to match earlier
examples: add the bash code-fence language specifier, change the health endpoint
URL in the curl command from http:// to https://, and preserve the "Alternative:
check authenticated endpoint" label and its curl -I https://... line; also
mirror this "Alternative" label and formatting in the earlier health-check
example so both examples are consistent (look for the health endpoint check code
blocks / health endpoint curl examples to apply these edits).
- Around line 911-928: The fenced code block showing the server-side
configuration is missing a language specifier; update the Markdown block that
contains the YAML keys (e.g., recommended_connect_timeout,
recommended_read_timeout, recommended_write_timeout, etc.) to start with ```yaml
so the block becomes a YAML-annotated fence for proper highlighting and linting.
- Around line 385-393: Verify whether the four timeout keys (connect_timeout,
read_timeout, write_timeout, request_timeout) and the retry/circuit-breaker keys
(max_attempts, initial_delay, max_delay, failure_threshold, timeout) were
intended as inline code in the bullet list or as a fenced code block; if inline
is intended, leave the bullets as-is, otherwise replace the bullets with a
single fenced code block (specify a language such as yaml or json) showing the
configuration structure containing those keys so the static analysis tool
warning is resolved and the examples are properly syntax-highlighted.
- Around line 815-824: Update the diagnostic step that shows the CLI commands
"cipherswarm-agent --version" and "cipherswarm-agent --features" to make clear
these are example commands and may differ across agent implementations (e.g., Go
clients or other language-specific CLIs); modify the text around those symbols
to read something like "Check agent version and features (commands vary by agent
implementation)" and add a short note telling readers to consult their agent's
documentation for the exact commands.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee61d3fd-c677-4861-b919-f3c2096bd219
📒 Files selected for processing (1)
docs/user-guide/troubleshooting-agents.md
- Add user_id to CONFIGURATION_BROADCAST_FIELDS for admin reassignment - Fix capabilities broadcast indentation in agent.rb - Collapse exists?+count into single count query in tasks_controller - Fix stale method reference (agent_compatible? → compatible_agent?) - Fix incorrect SystemHealthCheckService reference in health_controller - Add pull-requests: write permission to dep.yml for comment-summary - Add health check section and idempotency warning to API reference - Add 503 degraded response to health_spec and regenerate swagger.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 68 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "ErrorMetadata": { | ||
| "type": "object", | ||
| "nullable": true, | ||
| "description": "Additional metadata about an agent error", | ||
| "properties": { | ||
| "error_date": { | ||
| "type": "string", | ||
| "format": "date-time", | ||
| "description": "The date of the error" | ||
| }, | ||
| "other": { | ||
| "type": "object", | ||
| "nullable": true, | ||
| "description": "Other metadata", | ||
| "additionalProperties": true | ||
| } | ||
| }, | ||
| "required": [ | ||
| "error_date" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
ErrorMetadata is defined with only error_date and other, and does not allow additional top-level keys. However the API currently accepts/stores arbitrary metadata keys at the top level (e.g., metadata: { error_date: ..., key: "value" } in request specs; controller also adds metadata[:additional_info]). This makes the OpenAPI contract inaccurate and (with openapi_no_additional_properties / openapi_all_properties_required) would force clients to send/receive fields the server doesn’t actually structure that way. Either update the schema to allow arbitrary top-level keys (e.g., additionalProperties: true alongside error_date), or standardize the API to nest arbitrary fields under other and always include other (nullable) when metadata is present.
| ErrorMetadata: { | ||
| type: :object, | ||
| nullable: true, | ||
| description: "Additional metadata about an agent error", | ||
| properties: { | ||
| error_date: { type: :string, format: "date-time", description: "The date of the error" }, | ||
| other: { type: :object, nullable: true, description: "Other metadata", additionalProperties: true } | ||
| }, | ||
| required: %i[error_date] | ||
| }, |
There was a problem hiding this comment.
The ErrorMetadata schema here doesn’t match actual submit_error behavior/tests: metadata is treated as a free-form hash with top-level keys (and the controller adds keys like :additional_info). With openapi_no_additional_properties=true and openapi_all_properties_required=true, declaring other but not allowing additional top-level keys (or always returning other) will make schema validation/documentation misleading. Align the schema with the real payload shape (either allow additionalProperties: true at the root, or enforce { error_date, other } in the API and always include other).
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
swagger/v1/swagger.json (1)
3-16:⚠️ Potential issue | 🔴 CriticalScope SDK retries to safe operations only.
The root-level
x-speakeasy-retriespolicy is inherited by every operation generated in the SDK, including non-idempotentPOSToperations likeaccept_task,submit_crack, andsubmit_error. This creates a critical risk: retries can duplicate side effects if the server commits the write but the client loses the response.To fix this, remove the global policy and instead add
x-speakeasy-retriesonly to idempotent operations (e.g.,GET,HEAD), or if retries are needed onPOSToperations, ensure each defines an idempotency header so the same key is reused on retry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@swagger/v1/swagger.json` around lines 3 - 16, The global x-speakeasy-retries policy at the root should be removed and applied only to safe/idempotent operations: delete the root-level "x-speakeasy-retries" block and add an "x-speakeasy-retries" object to individual idempotent operations (e.g., all GET and HEAD operation objects) instead; for any POST operations that must retry, modify those operation objects (for example "accept_task", "submit_crack", "submit_error") to either omit retries or add an explicit idempotency header contract so retries reuse the same idempotency key. Ensure you update the swagger JSON by removing the top-level x-speakeasy-retries and adding the same backoff/statusCodes/retryConnectionErrors fields only under the operation objects that should retry (or replace with an idempotency header definition for POSTs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/services/task_assignment_service.rb`:
- Around line 306-323: Add a REASONING comment block immediately before the
available_attacks method explaining: intent (why we extracted this logic into
TaskAssignmentService), alternatives considered (e.g., keeping logic on Task
model or controller), decision rationale (consistency with
Task#compatible_agent? and find_unassigned_paused_task, readability,
testability), performance implications (DB joins, indexes, and filtering by
allowed_hash_type_ids / project_ids), and future considerations (caching,
pagination, and potential edge cases). Keep the block concise, use clear
headings for each section, and reference the method name available_attacks and
related behaviors (Task#compatible_agent?, find_unassigned_paused_task) so
reviewers can see the linkage.
In `@docs/api-reference-agent-auth.md`:
- Around line 631-635: The pseudocode for retry jitter uses delay on the
right-hand side before it's defined; change it to compute a base_delay first as
base_delay = min(initial_delay * 2^attempt, max_delay) and then compute delay
from that base as delay = base_delay + random(0, base_delay * 0.5), replacing
the old single-line expression that referenced delay on the right-hand side;
ensure the symbols initial_delay, attempt, max_delay, base_delay, and delay are
used exactly as shown.
In `@swagger/v1/swagger.json`:
- Around line 1295-1318: Update the health endpoint response schemas so the 200
and 503 responses use explicit enum values instead of free-form strings: for the
200 response schema (where "status" and "database" are currently defined) change
"status" to an enum with only "ok" and "database" to an enum with only
"healthy"; for the 503 response schema change "status" to an enum with only
"degraded" and "database" to an enum with only "unhealthy". Locate and modify
the JSON objects that define the response schemas for HTTP 200 and 503 (the
properties "status" and "database" in the health response definitions) to use
"enum" arrays with those specific values and keep other fields (api_version,
timestamp) unchanged.
---
Outside diff comments:
In `@swagger/v1/swagger.json`:
- Around line 3-16: The global x-speakeasy-retries policy at the root should be
removed and applied only to safe/idempotent operations: delete the root-level
"x-speakeasy-retries" block and add an "x-speakeasy-retries" object to
individual idempotent operations (e.g., all GET and HEAD operation objects)
instead; for any POST operations that must retry, modify those operation objects
(for example "accept_task", "submit_crack", "submit_error") to either omit
retries or add an explicit idempotency header contract so retries reuse the same
idempotency key. Ensure you update the swagger JSON by removing the top-level
x-speakeasy-retries and adding the same
backoff/statusCodes/retryConnectionErrors fields only under the operation
objects that should retry (or replace with an idempotency header definition for
POSTs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b115979b-118d-4ea0-8ff6-422848eb9a50
📒 Files selected for processing (8)
.github/workflows/dep.ymlapp/controllers/api/v1/client/health_controller.rbapp/controllers/api/v1/client/tasks_controller.rbapp/models/agent.rbapp/services/task_assignment_service.rbdocs/api-reference-agent-auth.mdspec/requests/api/v1/client/health_spec.rbswagger/v1/swagger.json
🚧 Files skipped from review as they are similar to previous changes (3)
- spec/requests/api/v1/client/health_spec.rb
- app/controllers/api/v1/client/health_controller.rb
- .github/workflows/dep.yml
| # Agents with no project assignments can work on any project (same convention | ||
| # as attack resources like word lists, rule lists, and Task#compatible_agent?). | ||
| # | ||
| # Ordering strategy: | ||
| # 1. campaigns.priority DESC: Higher campaign priority first (high=2, normal=0, deferred=-1) | ||
| # 2. attacks.complexity_value: Within same priority, simpler attacks first | ||
| # 3. attacks.created_at: Tie-breaker for same priority and complexity | ||
| # | ||
| # @return [ActiveRecord::Relation<Attack>] attacks ordered by campaign priority, complexity, creation time | ||
| def available_attacks | ||
| Attack.incomplete | ||
| .joins(campaign: { hash_list: :hash_type }) | ||
| .includes(campaign: %i[hash_list project]) | ||
| .where(campaigns: { project_id: agent.project_ids }) | ||
| .where(hash_lists: { hash_type_id: allowed_hash_type_ids }) | ||
| .order("campaigns.priority DESC, attacks.complexity_value, attacks.created_at") | ||
| scope = Attack.incomplete | ||
| .joins(campaign: { hash_list: :hash_type }) | ||
| .includes(campaign: %i[hash_list project]) | ||
| .where(hash_lists: { hash_type_id: allowed_hash_type_ids }) | ||
|
|
||
| scope = scope.where(campaigns: { project_id: agent.project_ids }) if agent.project_ids.present? | ||
|
|
||
| scope.order("campaigns.priority DESC, attacks.complexity_value, attacks.created_at") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Task#compatible_agent? follows the same project filtering convention
# Search for the compatible_agent? method implementation
ast-grep --pattern $'def compatible_agent?($$$)
$$$
end'
# Also search for project-related filtering in Task model
rg -n -C5 'project_ids' --type ruby -g 'app/models/task.rb'Repository: unclesp1d3r/CipherSwarm
Length of output: 376
Add REASONING block to document method extraction and design decisions.
The refactored available_attacks method correctly implements the same project-filtering convention as Task#compatible_agent?: agents without project assignments can work on any project. The conditional filtering pattern matches find_unassigned_paused_task, and the ordering strategy is preserved.
However, per coding guidelines, all service objects require a formal REASONING block explaining why this extraction was made, alternatives considered, decision rationale, performance implications, and future considerations. The current comment documents the ordering strategy and convention but lacks the required REASONING structure.
Add a REASONING comment block before the method definition with the required sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/task_assignment_service.rb` around lines 306 - 323, Add a
REASONING comment block immediately before the available_attacks method
explaining: intent (why we extracted this logic into TaskAssignmentService),
alternatives considered (e.g., keeping logic on Task model or controller),
decision rationale (consistency with Task#compatible_agent? and
find_unassigned_paused_task, readability, testability), performance implications
(DB joins, indexes, and filtering by allowed_hash_type_ids / project_ids), and
future considerations (caching, pagination, and potential edge cases). Keep the
block concise, use clear headings for each section, and reference the method
name available_attacks and related behaviors (Task#compatible_agent?,
find_unassigned_paused_task) so reviewers can see the linkage.
| Use exponential backoff with the `recommended_retry` parameters: | ||
|
|
||
| ```text | ||
| delay = min(initial_delay * 2^attempt, max_delay) + random(0, delay * 0.5) | ||
| ``` |
There was a problem hiding this comment.
Fix the retry jitter pseudocode.
The expression uses delay on the right-hand side before it exists, so clients copying this literally cannot compute the jitter bound.
✏️ Suggested fix
-```text
-delay = min(initial_delay * 2^attempt, max_delay) + random(0, delay * 0.5)
-```
+```text
+base_delay = min(initial_delay * 2^attempt, max_delay)
+delay = base_delay + random(0, base_delay * 0.5)
+```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/api-reference-agent-auth.md` around lines 631 - 635, The pseudocode for
retry jitter uses delay on the right-hand side before it's defined; change it to
compute a base_delay first as base_delay = min(initial_delay * 2^attempt,
max_delay) and then compute delay from that base as delay = base_delay +
random(0, base_delay * 0.5), replacing the old single-line expression that
referenced delay on the right-hand side; ensure the symbols initial_delay,
attempt, max_delay, base_delay, and delay are used exactly as shown.
| "status": { | ||
| "type": "string", | ||
| "description": "Overall health status (ok or degraded)" | ||
| }, | ||
| "api_version": { | ||
| "type": "integer", | ||
| "description": "API version" | ||
| }, | ||
| "timestamp": { | ||
| "type": "string", | ||
| "format": "date-time", | ||
| "description": "Server timestamp" | ||
| }, | ||
| "database": { | ||
| "type": "string", | ||
| "description": "Database health (healthy or unhealthy)" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "status", | ||
| "api_version", | ||
| "timestamp", | ||
| "database" | ||
| ] |
There was a problem hiding this comment.
Tighten the health response contract.
Both responses currently accept any string, and the 200 schema even describes degraded as valid. Pin the 200 response to status: "ok" / database: "healthy" and the 503 response to status: "degraded" / database: "unhealthy" so generated clients can branch on stable enums instead of free-form text.
Also applies to: 1330-1353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@swagger/v1/swagger.json` around lines 1295 - 1318, Update the health endpoint
response schemas so the 200 and 503 responses use explicit enum values instead
of free-form strings: for the 200 response schema (where "status" and "database"
are currently defined) change "status" to an enum with only "ok" and "database"
to an enum with only "healthy"; for the 503 response schema change "status" to
an enum with only "degraded" and "database" to an enum with only "unhealthy".
Locate and modify the JSON objects that define the response schemas for HTTP 200
and 503 (the properties "status" and "database" in the health response
definitions) to use "enum" arrays with those specific values and keep other
fields (api_version, timestamp) unchanged.
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 10 seconds in the queue, including 1 second running CI. Required conditions to merge
ReasonPull request #701 has been dequeued, merge conditions unmatch:
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. |
unclesp1d3r
deleted the
464-agent-clients-lack-timeout-and-retry-logic-hang-indefinitely-when-server-is-unresponsive
branch
This pull request introduces new agent definitions and configuration updates to support agent-native architecture reviews, Ankane-style README generation, and architectural strategy analysis. It also adds configuration for Tessl-managed files and MCP server integration across multiple tools. The most important changes are grouped below by theme.
Agent Definitions and Reviewers:
.github/agents/agent-native-reviewer.agent.mdfor reviewing agent-native architecture, ensuring parity between agent and user actions, context access, and shared workspace principles..github/agents/ankane-readme-writer.agent.mdfor generating Ruby gem READMEs in Ankane style, enforcing concise, imperative documentation with strict formatting rules..github/agents/architecture-strategist.agent.mdfor architectural analysis of code changes, focusing on pattern compliance, design integrity, and system evolution.Tessl and MCP Server Configuration:
.claude/settings.jsonto enablelayered-rails@palkan-skillsandcompound-engineering@compound-engineering-pluginplugins..codex/config.toml,.cursor/mcp.json, and.gemini/settings.jsonto configure MCP server integration for Tessl, standardizing agent communication via thetesslcommand. [1] [2]Tessl-Managed File Patterns:
.gitignoreentries in.claude/skills,.codex/skills,.cursor/skills, and.gemini/skillsto ignore Tessl-managed files and patterns, ensuring clean repository state.These changes collectively enhance agent functionality, documentation quality, and system architecture review capabilities.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores