feat: shooter/sniper genre-specific checks MVP#2
Merged
Conversation
- .sourcery.yaml: auto-review PRs, ignore dist/plugin/json/md - CI workflow: add sonarcloud job with SonarSource scan action Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Switch from sonarqube-scan-action to sonarcloud-github-action@v5 - Add Node.js setup + npm ci + build before SonarCloud scan - Remove redundant args (read from sonar-project.properties) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shipcheck rules (opt-in, genre: shooter): - rbx_shooter_weapon_remote_trust: audit weapon remotes for server-side validation, type checking, and rate limiting - rbx_shooter_spawn_clustering: detect spawn point clustering, team imbalance, and suspicious spawn heights - rbx_shooter_combat_content_maturity: scan scripts and UI for combat keywords affecting content maturity classification Playtester presets: - shooter_weapon_equip: verify weapon Tools exist with config values - shooter_respawn_cycle: validate respawn infrastructure All genre checks are heuristic and pattern-based. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdds three new genre-specific Shipcheck tools for Roblox shooter/sniper games (weapon remote trust, spawn clustering, and combat content maturity), two shooter-focused Playtester presets, and introduces SonarCloud + Sourcery configuration to CI and repo tooling. Sequence diagram for shooter weapon remote trust analysissequenceDiagram
participant ShipcheckTool as WeaponRemoteTrustTool
participant StudioBridgeClient
participant RobloxStudio
ShipcheckTool->>StudioBridgeClient: new StudioBridgeClient(port)
ShipcheckTool->>StudioBridgeClient: ping()
StudioBridgeClient-->>ShipcheckTool: pong
ShipcheckTool->>StudioBridgeClient: searchInstances(RemoteEvent,class)
StudioBridgeClient->>RobloxStudio: query RemoteEvent instances
RobloxStudio-->>StudioBridgeClient: RemoteEvent matches
StudioBridgeClient-->>ShipcheckTool: remoteEventMatches
ShipcheckTool->>StudioBridgeClient: searchInstances(RemoteFunction,class)
StudioBridgeClient->>RobloxStudio: query RemoteFunction instances
RobloxStudio-->>StudioBridgeClient: RemoteFunction matches
StudioBridgeClient-->>ShipcheckTool: remoteFunctionMatches
ShipcheckTool->>ShipcheckTool: filter weaponRemotes by weaponRemotePattern
loop For each weaponRemote
ShipcheckTool->>StudioBridgeClient: searchInstances(remoteName,script_content,root_path ServerScriptService)
StudioBridgeClient->>RobloxStudio: search handler scripts
RobloxStudio-->>StudioBridgeClient: handler matches
StudioBridgeClient-->>ShipcheckTool: handlers
alt No handlers found
ShipcheckTool->>ShipcheckTool: add no_server_handler issue
else Handlers found
loop For each handler script
ShipcheckTool->>StudioBridgeClient: getScriptSource(handler.path)
StudioBridgeClient->>RobloxStudio: read script source
RobloxStudio-->>StudioBridgeClient: script.source
StudioBridgeClient-->>ShipcheckTool: script.source
ShipcheckTool->>ShipcheckTool: append to combinedSource
end
alt check_type_validation enabled and no typeValidationPattern match
ShipcheckTool->>ShipcheckTool: add missing_type_validation issue
end
alt check_rate_limiting enabled and no rateLimitingPattern match
ShipcheckTool->>ShipcheckTool: add no_rate_limiting issue
end
end
end
ShipcheckTool->>ShipcheckTool: compute score from issues
ShipcheckTool-->>ShipcheckTool: ResponseEnvelope(WeaponRemoteTrustResult)
Class diagram for new shooter Shipcheck toolsclassDiagram
class StudioBridgeClient {
+port number
+ping()
+searchInstances(params)
+getProperties(path)
+getChildren(path, depth)
+getScriptSource(path)
}
class ResponseEnvelope_T_ {
+data T
+metadata object
}
class SpawnClusteringIssue {
+severity string
+rule string
+message string
+spawn_path string
+confidence string
}
class SpawnClusteringResult {
+score number
+spawn_count number
+team_distribution Record_string_number_
+avg_spread_studs number
+issues SpawnClusteringIssue[]
}
class CombatContentMaturityIssue {
+severity string
+category string
+rule string
+message string
+element_path string
+confidence string
}
class CombatContentMaturityResult {
+score number
+scripts_scanned number
+ui_elements_scanned number
+findings_by_category Record_category_number_
+issues CombatContentMaturityIssue[]
}
class WeaponRemoteTrustIssue {
+severity string
+remote_path string
+rule string
+message string
+suggestion string
+script_path string
}
class WeaponRemoteTrustResult {
+score number
+weapon_remotes_found number
+issues WeaponRemoteTrustIssue[]
}
class runSpawnClustering {
+runSpawnClustering(input)
}
class runCombatContentMaturity {
+runCombatContentMaturity(input)
}
class runWeaponRemoteTrust {
+runWeaponRemoteTrust(input)
}
class registerToolFn {
+registerTool(config)
}
class ShooterSpawnClusteringTool {
+name rbx_shooter_spawn_clustering
+description string
}
class ShooterCombatContentMaturityTool {
+name rbx_shooter_combat_content_maturity
+description string
}
class ShooterWeaponRemoteTrustTool {
+name rbx_shooter_weapon_remote_trust
+description string
}
runSpawnClustering --> ResponseEnvelope_T_ : returns
runSpawnClustering --> SpawnClusteringResult : payload
runSpawnClustering --> StudioBridgeClient : uses
runCombatContentMaturity --> ResponseEnvelope_T_ : returns
runCombatContentMaturity --> CombatContentMaturityResult : payload
runCombatContentMaturity --> StudioBridgeClient : uses
runWeaponRemoteTrust --> ResponseEnvelope_T_ : returns
runWeaponRemoteTrust --> WeaponRemoteTrustResult : payload
runWeaponRemoteTrust --> StudioBridgeClient : uses
ShooterSpawnClusteringTool ..> runSpawnClustering : handler
ShooterCombatContentMaturityTool ..> runCombatContentMaturity : handler
ShooterWeaponRemoteTrustTool ..> runWeaponRemoteTrust : handler
registerToolFn ..> ShooterSpawnClusteringTool : registers
registerToolFn ..> ShooterCombatContentMaturityTool : registers
registerToolFn ..> ShooterWeaponRemoteTrustTool : registers
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
runWeaponRemoteTrust, server handlers are only searched undergame.ServerScriptService; consider either broadening the search root or making it configurable so projects that colocate server scripts elsewhere don’t produce misleadingno_server_handlerissues. - The hardcoded suspicious spawn height bounds (
y < -10ory > 1000) inspawn-clustering.tscould be exposed as schema options to better fit very tall/vertical maps or unusual level scales. - The keyword lists in
combat-content-maturity.tsare currently fixed in-code; making them configurable (e.g., via tool input or a shared config) would let teams tune content maturity heuristics without code changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `runWeaponRemoteTrust`, server handlers are only searched under `game.ServerScriptService`; consider either broadening the search root or making it configurable so projects that colocate server scripts elsewhere don’t produce misleading `no_server_handler` issues.
- The hardcoded suspicious spawn height bounds (`y < -10` or `y > 1000`) in `spawn-clustering.ts` could be exposed as schema options to better fit very tall/vertical maps or unusual level scales.
- The keyword lists in `combat-content-maturity.ts` are currently fixed in-code; making them configurable (e.g., via tool input or a shared config) would let teams tune content maturity heuristics without code changes.
## Individual Comments
### Comment 1
<location path="src/tools/shipcheck/genre/shooter/spawn-clustering.ts" line_range="88-43" />
<code_context>
+ const client = new StudioBridgeClient({ port: input.studio_port });
+ await client.ping();
+
+ const matches = parseMatches(
+ await client.searchInstances({
+ query: "SpawnLocation",
+ search_type: "class",
+ case_sensitive: false,
+ max_results: 200,
+ }),
+ );
+
+ const issues: SpawnClusteringIssue[] = [];
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle failures from Studio search more explicitly instead of silently returning an empty match set.
`parseMatches` currently uses `safeParse(raw).data ?? []`, so any schema mismatch or API shape change from Studio becomes an empty list of spawns. That conflates “no spawns found” with “unexpected response shape” and can hide regressions. Please check `success` explicitly and either throw or surface/log a diagnostic when parsing fails so callers can distinguish these cases.
Suggested implementation:
```typescript
function parseMatches(raw: unknown) {
const parsed = SpawnSearchResultSchema.safeParse(raw);
if (!parsed.success) {
// Surface a diagnostic so schema/API regressions are visible instead of
// being conflated with "no spawns found".
// This will cause callers to see a failure instead of silently
// continuing with an empty match set.
// eslint-disable-next-line no-console
console.error(
"[spawn-clustering] Unexpected Studio searchInstances response shape",
parsed.error,
);
throw new Error(
"[spawn-clustering] Failed to parse Studio searchInstances response",
);
}
return parsed.data;
}
```
If the actual helper is named slightly differently or has a different return type, adjust the `SEARCH` block to match the current implementation of `parseMatches`.
Also ensure that any higher-level caller of `runSpawnClustering` either allows this new error to propagate (so it is visible in logs/telemetry) or wraps it into the appropriate `ResponseEnvelope` error path, depending on how failures are normally surfaced in this toolchain.
</issue_to_address>
### Comment 2
<location path="src/tools/shipcheck/genre/shooter/weapon-remote-trust.ts" line_range="85-87" />
<code_context>
+ }),
+ );
+
+ const weaponRemotes = uniqueByPath([...remoteEventMatches, ...remoteFunctionMatches]).filter(
+ (match) => weaponRemotePattern.test(match.path),
+ );
+ const issues: WeaponRemoteTrustIssue[] = [];
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Filtering weapon remotes by full path can misclassify unrelated remotes whose ancestors match the pattern.
Because the regex is applied to `match.path`, any remote under a container with matching terms (e.g., `fire`, `damage`) will be treated as a weapon remote, even if the remote name itself isn’t. This risks noisy, inflated `weapon_remotes_found`. Consider matching against the remote’s name (or a specific property) instead, or restricting the regex to the final path segment.
```suggestion
const weaponRemotes = uniqueByPath([...remoteEventMatches, ...remoteFunctionMatches]).filter(
(match) => {
const remoteName = match.path.split(".").pop() ?? match.path;
return weaponRemotePattern.test(remoteName);
},
);
```
</issue_to_address>
### Comment 3
<location path=".github/workflows/ci.yml" line_range="31-35" />
<code_context>
- name: Quality checks
run: npm run check
+
+ sonarcloud:
+ runs-on: ubuntu-latest
+ if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
+
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+
+ - name: Set up Node.js
+ uses: actions/setup-node@v4
+ with:
+ node-version: '20'
+ cache: 'npm'
+
+ - name: Install dependencies
+ run: npm ci
+
+ - name: Build
+ run: npm run build
+
+ - name: SonarCloud Scan
+ uses: SonarSource/sonarcloud-github-action@v5
+ env:
+ SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider declaring explicit permissions for the SonarCloud job to avoid relying on GitHub Defaults.
This job inherits repository-level defaults because it doesn’t declare `permissions`. SonarCloud PR decoration usually needs only `contents: read` and `pull-requests: write`, so adding an explicit `permissions` block on this job will follow least-privilege and avoid breakage if repo defaults change.
```suggestion
sonarcloud:
runs-on: ubuntu-latest
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
permissions:
contents: read
pull-requests: write
steps:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- docs/shooter-checks.md: per-rule documentation with limitations - examples/shooter-report.md: sample shooter audit report - TESTING.md: add shooter genre test results (3 tools + 2 presets) - README: add genre-specific checks section, update test table to 46 tools Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- weapon-remote-trust: match only remote name (not full path) to reduce false positives; add configurable server_script_root param - spawn-clustering: expose min_height/max_height as schema options; throw on parse failure instead of silent empty result - combat-content-maturity: add custom_keywords param to extend built-in keyword lists without code changes - ci: add explicit permissions (contents:read, pull-requests:write) to sonarcloud job for least-privilege Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- lighting-preset: fix unused _ destructure and presetName variable - sonarcloud job: add continue-on-error until project is configured - prettier: auto-format shooter genre files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New Tools
rbx_shooter_weapon_remote_trustrbx_shooter_spawn_clusteringrbx_shooter_combat_content_maturityNew Playtester Presets
shooter_weapon_equip— verify weapon Tools with config valuesshooter_respawn_cycle— validate respawn infrastructureTest plan
npm run buildpassesnpm run checkpasses🤖 Generated with Claude Code
Summary by Sourcery
Add shooter-genre-specific analysis tools and playtester presets, and integrate additional code quality automation.
New Features:
Enhancements:
Build:
CI: