Build 956: Audit Remediation#31
Conversation
…ks across all entry modules - FFMA, MOMO, OR, RMA (ExecuteRMAEntry/Custom/TrendSplit), Trend (ExecuteTRENDEntry/ManualEntry): deferred activePositions/entryOrders writes to AFTER null check on SubmitOrderUnmanaged; wrapped both assignments in lock(stateLock); added clean rollback + return on null submit. - Trailing.cs: wrapped three stopOrders[entryName] write sites in lock(stateLock). - Orders.Management.cs: wrapped two stopOrders[entryName] write sites in lock(stateLock). - Symmetry.cs: moved pos.EntryFilled = true inside existing lock(stateLock) block so EntryFilled and RemainingContracts are mutated atomically. Resolves Build 960 audit defects: Area 1 item 1 and Area 2 item 1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A1-2: Added StampReaperMoveGrace() before Cancel in follower target replacements to open a 5-second REAPER suppression window, matching the established PropagateMasterTargetMove pattern: - Symmetry.cs SymmetryGuardReplaceExistingFollowerTarget: guard before Cancel, wrapped dict[fleetEntryName] write in lock(stateLock). - Trailing.cs MoveSpecificTarget: guard before follower target Cancel. A1-3: Wrapped the FSM PendingCancel->Submitting state transition snapshot in lock(stateLock) inside OnAccountOrderUpdate. This closes the TOCTOU window where PropagateFollowerEntryReplace (holding stateLock) could write to PendingQty/PendingPrice simultaneously with the callback thread reading them. Resolves Build 960 audit defects: Area 1 items 2 and 3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ncel expectedPositions rollback A2-2: Added PendingCleanup and FlattenAttemptCount fields to PositionInfo. Replaced immediate activePositions.TryRemove after RequestStopCancelLifecycleSafe in the final-target and trim-close flows with pos.PendingCleanup = true. Added deferred TryRemove logic to both HandleOrderCancelled (master stop) and OnAccountOrderUpdate (follower stop) -- purge fires only on broker-confirmed stop terminal state. A2-3: Removed DeltaExpectedPositionLocked from SymmetryGuardCascadeFollowerCleanup (was firing immediately on cancel request). Moved the confirmed-cancel delta rollback into the non-FSM follower entry cancelled branch in OnAccountOrderUpdate -- rollback now fires only after OrderState.Cancelled confirmed by the broker, eliminating the microsecond fill-race desync between the master cancel request and broker confirmation. Resolves Build 960 audit defects: Area 2 items 2 and 3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A3-1 (V12_002.SIMA.cs): - PumpFleetDispatch: hard guard drains queue if SIMA disabled or flatten running - ApplySimaState disable path: drain _pendingFleetDispatches after Reaper stop A3-2 (V12_002.REAPER.cs): - ExecuteReaperRepair: isFlattenRunning guard as first statement - Background REAPER thread: _repairInFlight.Add moved before TriggerCustomEvent to prevent double-enqueue in next audit cycle A3-3 (V12_002.Trailing.cs): - UpdateStopOrder null-stop path: FlattenAttemptCount circuit breaker (cap at 3) - UpdateStopOrder catch path: same circuit breaker guards emergency flatten - Reset FlattenAttemptCount to 0 on successful stop submission Build 960 Phase 2 Omni-Audit -- Area 3 complete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR introduces a multi-agent collaboration protocol update ("The Loop" workflow: Repair Mission → Sonnet Handoff → Branch/Code/Push → PR Re-Audit Matrix → Merge) across configuration and manifest files. It consolidates guidance documents to reference a unified standards manifesto, adds GitHub Actions audit workflows, and augments the trading strategy codebase with extensive stateLock-based synchronization guards across entry, order callback, and position management code paths to prevent race conditions. Changes
Sequence DiagramsequenceDiagram
participant LE as Lead Engineer<br/>(Claude/Sonnet)
participant Branch as Branch<br/>Operations
participant PR as PR System
participant Matrix as Re-Audit Matrix<br/>(Opus/Gemini)
participant Merge as Merge
LE->>LE: Repair Mission<br/>Identified
LE->>LE: Sonnet Handoff<br/>Ready
LE->>Branch: Checkout<br/>Branch
LE->>Branch: Code<br/>Implementation
LE->>Branch: Push<br/>Changes
Branch->>PR: Create PR
PR->>Matrix: Trigger Phase 3<br/>Omni-Audit
Matrix->>Matrix: Opus Reviews
Matrix->>Matrix: Sonnet Reviews
Matrix->>Matrix: Gemini Reviews
Matrix->>PR: Post Review<br/>Comments
PR->>LE: Request<br/>Human Sign-off
LE->>Merge: Approve &<br/>Merge
Merge->>Merge: Integrated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
 |
|
Overall Grade Focus Area: Complexity |
Security Reliability Complexity Hygiene |
Feedback
- Monolithic handler mixing concerns
- The large HandleMatchedFollowerOrder bundles parsing, business rules and side-effects, inflating branches and duplicating logic; splitting into validation, decision, and effect layers (pure functions + thin orchestrator) will cut paths and ease testing.
- Branch-driven logic instead of declarative rules
- Many nested if/switches implement permutations; moving to table-driven rules, strategy objects or a state machine reduces case explosion and collapses cyclomatic complexity into data-driven evaluations.
- Near-duplicate conditionals scattered across modules
- Repeated occurrences suggest duplicated control logic across handlers; extract a single domain service with small, well-tested rule evaluators to stop copy‑paste and ensure fixes lower complexity globally.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 9, 2026 2:17a.m. | Review ↗ |
| Secrets |  |
Mar 9, 2026 2:17a.m. | Review ↗ |
🤖 Antigravity Auto-Auditor (Gemini 2.5 Pro via Vertex AI)Greetings. I am the Gemini CLI, Antigravity Auto-Auditor. I have completed my analysis of this pull request against the Verdict: REJECTEDThis pull request introduces severe violations of project standards and cannot be approved. While the intent appears to be the addition of audit tooling and documentation, the execution is fundamentally flawed. The PR commits numerous temporary artifacts and log files, directly violating repo hygiene rules. Critically, it also includes a third-party audit report ( Systematic Review of Violations1. Standard: Repo Hygiene & Binary GuardVIOLATION: This PR commits a large number of temporary, generated, or artifact files. This is a direct violation of the The following committed files are in violation:
Required Action: These files must be removed from the commit history. They should be added to 2. Standard: Concurrency (
|
|
──────────────────────────────────────────────────────────────────────────────── https://aider.chat/HISTORY.html#release-notes Initial repo scan can be slow in larger repos, but only happens once. https://platform.openai.com/docs/guides/error-codes/api-errors |
There was a problem hiding this comment.
Pull request overview
Implements Build 956/960 audit remediations to harden multi-threaded order/state management (stateLock discipline, deferred cleanup, and circuit breakers), plus adds/updates repo agent/audit tooling artifacts.
Changes:
- Harden shared state mutations by wrapping key dictionary writes/removes in
lock (stateLock)and tightening FSM/replace race windows. - Add deferred metadata cleanup (
PendingCleanup) and an emergency-flatten circuit breaker (FlattenAttemptCount) to reduce desync/loop risk. - Add/adjust audit automation/docs and commit several PR/audit artifacts (logs/diffs/json) into the repo.
Reviewed changes
Copilot reviewed 25 out of 30 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test-vertex.js | Adds a small Vertex AI model test harness used to validate model/location behavior. |
| src/V12_002.cs | Extends PositionInfo with audit remediation fields for deferred cleanup and circuit breaking. |
| src/V12_002.Trailing.cs | Moves stop order dictionary writes under stateLock, adds emergency-flatten circuit breaker, stamps REAPER grace. |
| src/V12_002.Symmetry.cs | Adds REAPER grace stamping before cancel and wraps shared dict writes and position flags in stateLock. |
| src/V12_002.SIMA.cs | Gates fleet dispatch pump during flatten/SIMA disabled and drains queue on shutdown. |
| src/V12_002.REAPER.cs | Marks repairs in-flight earlier and aborts repairs during flatten. |
| src/V12_002.Orders.Management.cs | Wraps shared dictionary removals and replacement counters under stateLock and adjusts cleanup flows. |
| src/V12_002.Orders.Callbacks.cs | Wraps shared dictionary mutates under stateLock, adds deferred cleanup purge on stop terminal, and tightens follower FSM snapshotting. |
| src/V12_002.Entries.Trend.cs | Defers registering activePositions/entryOrders until entry submit succeeds; adds null-abort rollbacks. |
| src/V12_002.Entries.RMA.cs | Same null-abort/rollback + stateLock registration pattern for RMA/TrendSplit. |
| src/V12_002.Entries.OR.cs | Same null-abort/rollback + stateLock registration for OR entries. |
| src/V12_002.Entries.MOMO.cs | Same null-abort/rollback + stateLock registration for MOMO entries. |
| src/V12_002.Entries.FFMA.cs | Same null-abort/rollback + stateLock registration for FFMA entries. |
| prdiff.txt | Adds a large “diff of diffs” artifact rather than source changes. |
| pr_checks.json | Adds a PR checks status artifact. |
| pr24_audit_utf8.json | Adds an audit export artifact containing tool output. |
| gh_log.txt | Adds CI log output artifact (includes failure details). |
| codex_audit.md | Adds an audit report document. |
| GEMINI.md | Updates standards doc with an “Autonomous Repair Loop” bullet. |
| .github/copilot-instructions.md | Replaces detailed Copilot guidance with a pointer to the manifesto. |
| .cursorrules | Same as above, but includes a machine-local absolute path. |
| .claude/worktrees/sleepy-jepsen | Adds a worktree pointer file (subproject commit line). |
| .claude/worktrees/affectionate-franklin | Adds a worktree pointer file (subproject commit line). |
| .claude.json | Adds Claude custom instructions pointing to the manifesto. |
| .agent/standards_manifesto.md | Updates internal agent operation protocol text and autonomy steps. |
| // A3-2: Mark in-flight BEFORE TriggerCustomEvent to block double-enqueue in next audit cycle (Build 960 audit fix) | ||
| lock (stateLock) { _repairInFlight.Add(repairKey); } | ||
| _reaperRepairQueue.Enqueue(acct.Name); | ||
| try { TriggerCustomEvent(o => ProcessReaperRepairQueue(), null); } catch { } |
There was a problem hiding this comment.
_repairInFlight is now set before TriggerCustomEvent, but the catch { } swallows failures without removing the in-flight key. If TriggerCustomEvent fails (or the queue never runs), this repairKey can remain stuck forever and block future repairs. Suggested fix: in the catch block, remove repairKey from _repairInFlight under stateLock (and consider logging the failure).
| try { TriggerCustomEvent(o => ProcessReaperRepairQueue(), null); } catch { } | |
| try | |
| { | |
| TriggerCustomEvent(o => ProcessReaperRepairQueue(), null); | |
| } | |
| catch (Exception ex) | |
| { | |
| // If triggering the repair queue fails, clear the in-flight marker to avoid blocking future repairs. | |
| lock (stateLock) | |
| { | |
| _repairInFlight.Remove(repairKey); | |
| } | |
| if (shouldLog) | |
| Print($"[REAPER] ERROR: Failed to TriggerCustomEvent for repair queue on account {acct.Name}: {ex}"); | |
| } |
| // A3-2: Abort immediately if a flatten is in progress (Build 960 audit fix) | ||
| if (isFlattenRunning) | ||
| { | ||
| Print("[REAPER REPAIR] Aborted -- flatten in progress."); | ||
| return; | ||
| } | ||
|
|
||
| string repairKey = accountName + "_" + Instrument.FullName; |
There was a problem hiding this comment.
This early-return can leak the _repairInFlight flag because in-flight is set before scheduling repair execution (see AuditSingleFleetAccount). If flatten is running, you likely still need to clear the in-flight marker (and potentially dequeue/skip) so repairs can resume after flatten. Suggested fix: compute repairKey before this guard and remove it from _repairInFlight (under stateLock) before returning.
| // A3-2: Abort immediately if a flatten is in progress (Build 960 audit fix) | |
| if (isFlattenRunning) | |
| { | |
| Print("[REAPER REPAIR] Aborted -- flatten in progress."); | |
| return; | |
| } | |
| string repairKey = accountName + "_" + Instrument.FullName; | |
| string repairKey = accountName + "_" + Instrument.FullName; | |
| // A3-2: Abort immediately if a flatten is in progress (Build 960 audit fix) | |
| if (isFlattenRunning) | |
| { | |
| Print("[REAPER REPAIR] Aborted -- flatten in progress."); | |
| // Build 960/935: Ensure we clear the in-flight marker if we abort due to flatten. | |
| lock (stateLock) | |
| { | |
| _repairInFlight?.Remove(repairKey); | |
| } | |
| return; | |
| } |
| lock (stateLock) { dict.TryRemove(kvp.Key, out _); } | ||
| return true; |
There was a problem hiding this comment.
TryRemoveTargetReferenceByOrder now returns true even if dict.TryRemove(...) fails (e.g., another thread already removed the key). That can cause callers to believe the reference was removed when it wasn't, skipping fallback cleanup paths. Suggested fix: capture the boolean result of TryRemove and return that value instead of unconditional true.
| lock (stateLock) { dict.TryRemove(kvp.Key, out _); } | |
| return true; | |
| bool removed; | |
| lock (stateLock) | |
| { | |
| removed = dict.TryRemove(kvp.Key, out _); | |
| } | |
| if (removed) | |
| return true; |
| // A3-3: Circuit breaker -- cap consecutive flatten attempts to 3 (Build 960 audit fix) | ||
| PositionInfo cbPos; | ||
| if (activePositions.TryGetValue(entryName, out cbPos) && cbPos != null) | ||
| { | ||
| cbPos.FlattenAttemptCount++; | ||
| if (cbPos.FlattenAttemptCount > 3) | ||
| { | ||
| Print(string.Format("[CIRCUIT BREAKER] Emergency flatten halted after 3 consecutive failures for {0}. Manual intervention required.", entryName)); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
FlattenAttemptCount is mutated on a shared PositionInfo instance without stateLock. Given the stated goal of preventing torn reads by REAPER/other threads, this counter should be incremented/checked under lock (stateLock) (or use Interlocked.Increment on the counter field). Apply the same protection for the reset path and the exception path increment.
| PositionInfo closedPos; | ||
| if (activePositions.TryGetValue(entryName, out closedPos) && closedPos != null) | ||
| closedPos.PendingCleanup = true; | ||
| else |
There was a problem hiding this comment.
PendingCleanup is a shared PositionInfo flag used by later cancellation/terminal handlers, but it is set outside stateLock. For consistency with the audit fixes (and to avoid readers observing stale values), set closedPos.PendingCleanup = true inside lock (stateLock).
| PositionInfo closedPos; | |
| if (activePositions.TryGetValue(entryName, out closedPos) && closedPos != null) | |
| closedPos.PendingCleanup = true; | |
| else | |
| PositionInfo closedPos = null; | |
| lock (stateLock) | |
| { | |
| if (activePositions.TryGetValue(entryName, out closedPos) && closedPos != null) | |
| closedPos.PendingCleanup = true; | |
| } | |
| if (closedPos == null) |
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | ||
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | ||
| { | ||
| lock (stateLock) { activePositions.TryRemove(kvp.Key, out _); } |
There was a problem hiding this comment.
This new purge path reads cleanupPos.PendingCleanup and cleanupPos.RemainingContracts without stateLock, then removes from activePositions under lock. Since these fields are used as a gating condition for the purge, they should be read under the same lock to avoid races (e.g., purging while another path is adjusting RemainingContracts or toggling PendingCleanup). Suggested fix: take stateLock, re-check the condition, then remove/purge within the same critical section.
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | |
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | |
| { | |
| lock (stateLock) { activePositions.TryRemove(kvp.Key, out _); } | |
| bool didPurge = false; | |
| lock (stateLock) | |
| { | |
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | |
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | |
| { | |
| activePositions.TryRemove(kvp.Key, out _); | |
| didPurge = true; | |
| } | |
| } | |
| if (didPurge) | |
| { |
|
|
||
| async function testModel(modelName, location) { | ||
| try { | ||
| const vertex_ai = new VertexAI({project: 'project-263b6139-6893-4788-995', location: location}); |
There was a problem hiding this comment.
Hardcoding a GCP project identifier in-repo risks accidental disclosure and makes the script non-portable. Suggested fix: read the project ID from an environment variable (with a clear error if missing) and keep any sensitive identifiers out of committed source.
| const vertex_ai = new VertexAI({project: 'project-263b6139-6893-4788-995', location: location}); | |
| const projectId = process.env.GCP_PROJECT_ID; | |
| if (!projectId) { | |
| throw new Error('GCP_PROJECT_ID environment variable is not set'); | |
| } | |
| const vertex_ai = new VertexAI({project: projectId, location: location}); |
|
|
||
| Follow these rules for ALL code modifications. Reference **[.agent/standards_manifesto.md](file:///.agent/standards_manifesto.md)** for permanent safety standards. | ||
| **CRITICAL INSTRUCTION FOR CURSOR AI:** | ||
| You must read, ingest, and strictly adhere to the permanent project standards defined in the `c:\WSGTA\universal-or-strategy\.agent\standards_manifesto.md` file before making any code suggestions or architectural decisions. |
There was a problem hiding this comment.
The rules reference an absolute Windows path, which won’t apply for other contributors/CI and can become stale. Suggested fix: replace it with a repo-relative path (e.g., .agent/standards_manifesto.md) and avoid machine-local locations in committed docs.
| You must read, ingest, and strictly adhere to the permanent project standards defined in the `c:\WSGTA\universal-or-strategy\.agent\standards_manifesto.md` file before making any code suggestions or architectural decisions. | |
| You must read, ingest, and strictly adhere to the permanent project standards defined in the `.agent/standards_manifesto.md` file before making any code suggestions or architectural decisions. |
| codex-audit Post Audit Results to PR ´╗┐2026-03-07T00:40:24.0405745Z ##[group]Run if [ -f audit_output.md ]; then | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406101Z [36;1mif [ -f audit_output.md ]; then[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406414Z [36;1m gh pr comment 26 --body-file audit_output.md[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406709Z [36;1melse[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406912Z [36;1m echo "Audit output file not found."[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0407214Z [36;1m # Fallback if Aider stdout'd it instead[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0407783Z [36;1m gh pr comment 26 --body "The OpenAI audit finished, but the markdown file was not saved correctly. Please check action logs."[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0408303Z [36;1mfi[0m | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458098Z shell: /usr/bin/bash -e {0} | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458350Z env: | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458608Z pythonLocation: /opt/hostedtoolcache/Python/3.11.14/x64 | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0459055Z PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.14/x64/lib/pkgconfig | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0459704Z Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460135Z Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460547Z Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460931Z LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.14/x64/lib | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0461503Z GH_TOKEN: *** | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0461692Z ##[endgroup] | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0533500Z Audit output file not found. | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:26.1321535Z GraphQL: Resource not accessible by integration (addComment) | ||
| codex-audit Post Audit Results to PR 2026-03-07T00:40:26.1357370Z ##[error]Process completed with exit code 1. |
There was a problem hiding this comment.
CI logs should not be committed into the repository (they’re ephemeral, noisy, and can accidentally include sensitive context). Suggested fix: remove gh_log.txt (and similar artifacts like pr_checks.json, pr24_audit_utf8.json, prdiff.txt) from version control and add them to .gitignore or store them as CI artifacts instead.
| codex-audit Post Audit Results to PR ´╗┐2026-03-07T00:40:24.0405745Z ##[group]Run if [ -f audit_output.md ]; then | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406101Z �[36;1mif [ -f audit_output.md ]; then�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406414Z �[36;1m gh pr comment 26 --body-file audit_output.md�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406709Z �[36;1melse�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0406912Z �[36;1m echo "Audit output file not found."�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0407214Z �[36;1m # Fallback if Aider stdout'd it instead�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0407783Z �[36;1m gh pr comment 26 --body "The OpenAI audit finished, but the markdown file was not saved correctly. Please check action logs."�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0408303Z �[36;1mfi�[0m | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458098Z shell: /usr/bin/bash -e {0} | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458350Z env: | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0458608Z pythonLocation: /opt/hostedtoolcache/Python/3.11.14/x64 | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0459055Z PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.14/x64/lib/pkgconfig | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0459704Z Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460135Z Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460547Z Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.14/x64 | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0460931Z LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.14/x64/lib | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0461503Z GH_TOKEN: *** | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0461692Z ##[endgroup] | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:24.0533500Z Audit output file not found. | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:26.1321535Z GraphQL: Resource not accessible by integration (addComment) | |
| codex-audit Post Audit Results to PR 2026-03-07T00:40:26.1357370Z ##[error]Process completed with exit code 1. | |
| # This file is intentionally kept free of CI logs. | |
| # CI logs like gh_log.txt should not be committed; configure your CI to store them as artifacts | |
| # and add this path to .gitignore instead of checking in actual log output. |
Greptile SummaryThis PR (Build 956) implements the forensic audit remediations from The majority of individual fixes are clean and materially improve the concurrency safety of the strategy. However, two logic bugs were introduced in the dual-leg null-abort paths, and the A2-2 deferred cleanup blocks are incomplete:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Entry Method\n(FFMA/OR/MOMO/TREND/RMA)"] --> B["CreateTRENDPosition / ApplyTargetLadderGuard"]
B --> C{"TREND/RMA dual-leg?\n(linkedTRENDEntries set here)"}
C -->|Yes| D["SubmitOrderUnmanaged E1"]
C -->|No| E["SubmitOrderUnmanaged"]
D --> F{E1 == null?}
F -->|Yes| G["Roll back masterDeltaE1\nPrint ENTRY_ABORT\nreturn\n⚠️ linkedTRENDEntries NOT cleaned"]
F -->|No| H["lock(stateLock)\nactivePositions + entryOrders = E1"]
H --> I["SubmitOrderUnmanaged E2"]
I --> J{E2 == null?}
J -->|Yes| K["Roll back masterDeltaE2\nPrint ENTRY_ABORT\nreturn\n⚠️ E1 stays in dict, link to ghost E2"]
J -->|No| L["lock(stateLock)\nactivePositions + entryOrders = E2"]
E --> M{Order == null?}
M -->|Yes| N["Roll back expectedPositions\nPrint ENTRY_ABORT\nreturn — Clean"]
M -->|No| O["lock(stateLock)\nactivePositions + entryOrders"]
L --> P["SubmitBracketOrders on fill\nA2-2: set PendingCleanup on final target"]
O --> P
P --> Q{Stop terminal?}
Q -->|Cancelled A2-2| R["lock(stateLock)\nactivePositions.TryRemove\n⚠️ stopOrders NOT removed"]
Q -->|Filled| S["lock(stateLock)\nFull CleanupPosition"]
Last reviewed commit: 9c86228 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (entryOrder1 == null) | ||
| { | ||
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE1); | ||
| Print("[ERROR][1102Y-V3] TREND E1 SubmitOrderUnmanaged NULL for " + entry1Name + " -- rolled back."); | ||
| Print("[ENTRY_ABORT] TREND E1 SubmitOrderUnmanaged NULL for " + entry1Name + " -- rolled back."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
linkedTRENDEntries orphaned on E1 null abort
linkedTRENDEntries[entry1Name] = entry2Name and linkedTRENDEntries[entry2Name] = entry1Name are written at lines 215–216, before the E1 submit. When E1 returns null and the function exits here, both keys are left in linkedTRENDEntries with no backing activePositions or entryOrders entries. On the next signal cycle the same trendGroupId names are reused, but any stale iteration over linkedTRENDEntries (e.g. in target-fill or stop-fill callbacks) will find the ghost pair and attempt to process a non-existent partner position.
| if (entryOrder1 == null) | |
| { | |
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE1); | |
| Print("[ERROR][1102Y-V3] TREND E1 SubmitOrderUnmanaged NULL for " + entry1Name + " -- rolled back."); | |
| Print("[ENTRY_ABORT] TREND E1 SubmitOrderUnmanaged NULL for " + entry1Name + " -- rolled back."); | |
| return; | |
| } | |
| if (entryOrder1 == null) | |
| { | |
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE1); | |
| Print("[ENTRY_ABORT] TREND E1 SubmitOrderUnmanaged NULL for " + entry1Name + " -- rolled back."); | |
| linkedTRENDEntries.TryRemove(entry1Name, out _); | |
| linkedTRENDEntries.TryRemove(entry2Name, out _); | |
| return; | |
| } |
| if (entryOrder2 == null) | ||
| { | ||
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE2); | ||
| Print("[ERROR][1102Y-V3] TREND E2 SubmitOrderUnmanaged NULL for " + entry2Name + " -- rolled back."); | ||
| Print("[ENTRY_ABORT] TREND E2 SubmitOrderUnmanaged NULL for " + entry2Name + " -- rolled back."); | ||
| return; | ||
| } | ||
| entryOrders[entry2Name] = entryOrder2; | ||
| lock (stateLock) { activePositions[entry2Name] = pos2; entryOrders[entry2Name] = entryOrder2; } |
There was a problem hiding this comment.
E2 null abort leaves E1 registered with a broken cross-link
When E2 submit fails, the function returns, but at this point:
activePositions[entry1Name]andentryOrders[entry1Name]are already committed insidestateLock.linkedTRENDEntries[entry1Name] = entry2Namestill points to a name that has noactivePositions,entryOrders, orexpectedPositionsentry.
When E1 later fills, OnOrderUpdate/HandleOrderFilled will find linkedTRENDEntries[entry1Name] = entry2Name and attempt to look up E2 in activePositions, finding nothing. Depending on the callback's null-handling for the linked leg, this can leave E1 permanently stuck in a half-linked state (e.g. stop sizing based on a two-leg group but only one leg exists).
The abort path should clean up the orphaned linkedTRENDEntries entries and roll back masterDeltaE1 (the E1 order is real, but a full two-leg group was the intent):
| if (entryOrder2 == null) | |
| { | |
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE2); | |
| Print("[ERROR][1102Y-V3] TREND E2 SubmitOrderUnmanaged NULL for " + entry2Name + " -- rolled back."); | |
| Print("[ENTRY_ABORT] TREND E2 SubmitOrderUnmanaged NULL for " + entry2Name + " -- rolled back."); | |
| return; | |
| } | |
| entryOrders[entry2Name] = entryOrder2; | |
| lock (stateLock) { activePositions[entry2Name] = pos2; entryOrders[entry2Name] = entryOrder2; } | |
| if (entryOrder2 == null) | |
| { | |
| AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaE2); | |
| Print("[ENTRY_ABORT] TREND E2 SubmitOrderUnmanaged NULL for " + entry2Name + " -- rolled back."); | |
| linkedTRENDEntries.TryRemove(entry1Name, out _); | |
| linkedTRENDEntries.TryRemove(entry2Name, out _); | |
| return; | |
| } |
Note: Whether to also cancel the already-submitted E1 order here is a product decision, but at minimum the cross-link should be cleared to prevent the callback path from attempting to process a ghost E2.
| // A2-2: Deferred PendingCleanup purge -- master stop terminal (Build 960 audit fix). | ||
| // If no pendingStopReplacement matched, check if this stop cancel completes a | ||
| // final-target/trim close where activePositions was intentionally kept alive. | ||
| if (!handled) | ||
| { | ||
| foreach (var kvp in stopOrders.ToArray()) | ||
| { | ||
| if (kvp.Value == order) | ||
| { | ||
| PositionInfo cleanupPos; | ||
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | ||
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | ||
| { | ||
| lock (stateLock) { activePositions.TryRemove(kvp.Key, out _); } | ||
| SymmetryGuardForgetEntry(kvp.Key); | ||
| Print("[A2-2] Deferred PendingCleanup purge (master stop cancel): " + kvp.Key); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
A2-2 deferred cleanup never removes the stopOrders entry
When the PendingCleanup condition is satisfied, the code removes the key from activePositions and calls SymmetryGuardForgetEntry, but it never calls stopOrders.TryRemove(kvp.Key, out _). The stop order reference (which is in a terminal/cancelled state at this point) therefore persists in stopOrders indefinitely.
Downstream, any path that iterates stopOrders.ToArray() (e.g. CleanupPosition, HandleOrderCancelled, reverse-audit sweeps) will encounter the stale mapping and may invoke CreateNewStopOrder, attempt re-submission, or simply generate spurious log noise. The same defect exists in the follower stop terminal variant at lines 681–697.
| // A2-2: Deferred PendingCleanup purge -- master stop terminal (Build 960 audit fix). | |
| // If no pendingStopReplacement matched, check if this stop cancel completes a | |
| // final-target/trim close where activePositions was intentionally kept alive. | |
| if (!handled) | |
| { | |
| foreach (var kvp in stopOrders.ToArray()) | |
| { | |
| if (kvp.Value == order) | |
| { | |
| PositionInfo cleanupPos; | |
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | |
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | |
| { | |
| lock (stateLock) { activePositions.TryRemove(kvp.Key, out _); } | |
| SymmetryGuardForgetEntry(kvp.Key); | |
| Print("[A2-2] Deferred PendingCleanup purge (master stop cancel): " + kvp.Key); | |
| } | |
| break; | |
| } | |
| } | |
| } | |
| } | |
| if (activePositions.TryGetValue(kvp.Key, out cleanupPos) && cleanupPos != null | |
| && cleanupPos.PendingCleanup && cleanupPos.RemainingContracts <= 0) | |
| { | |
| lock (stateLock) | |
| { | |
| activePositions.TryRemove(kvp.Key, out _); | |
| stopOrders.TryRemove(kvp.Key, out _); | |
| } | |
| SymmetryGuardForgetEntry(kvp.Key); | |
| Print("[A2-2] Deferred PendingCleanup purge (master stop cancel): " + kvp.Key); | |
| } |
| // A2-2: Deferred PendingCleanup purge -- follower stop terminal (Build 960 audit fix). | ||
| if (order.Name.StartsWith("Stop_") || order.Name.StartsWith("S_")) | ||
| { | ||
| foreach (var _sc in stopOrders.ToArray()) | ||
| { | ||
| if (_sc.Value == order) | ||
| { | ||
| PositionInfo _scPos; | ||
| if (activePositions.TryGetValue(_sc.Key, out _scPos) && _scPos != null | ||
| && _scPos.PendingCleanup && _scPos.RemainingContracts <= 0) | ||
| { | ||
| lock (stateLock) { activePositions.TryRemove(_sc.Key, out _); } | ||
| SymmetryGuardForgetEntry(_sc.Key); | ||
| Print("[A2-2] Deferred PendingCleanup purge (follower stop terminal): " + _sc.Key); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Follower A2-2 deferred cleanup also omits stopOrders removal
Same defect as the master-stop A2-2 block above: activePositions.TryRemove is called but stopOrders.TryRemove(_sc.Key, out _) is never issued. The stop order entry for the follower position will remain in stopOrders after cleanup, creating a ghost reference that can trigger spurious re-submission or replacement attempts on a position that no longer exists.
| // A2-2: Deferred PendingCleanup purge -- follower stop terminal (Build 960 audit fix). | |
| if (order.Name.StartsWith("Stop_") || order.Name.StartsWith("S_")) | |
| { | |
| foreach (var _sc in stopOrders.ToArray()) | |
| { | |
| if (_sc.Value == order) | |
| { | |
| PositionInfo _scPos; | |
| if (activePositions.TryGetValue(_sc.Key, out _scPos) && _scPos != null | |
| && _scPos.PendingCleanup && _scPos.RemainingContracts <= 0) | |
| { | |
| lock (stateLock) { activePositions.TryRemove(_sc.Key, out _); } | |
| SymmetryGuardForgetEntry(_sc.Key); | |
| Print("[A2-2] Deferred PendingCleanup purge (follower stop terminal): " + _sc.Key); | |
| } | |
| break; | |
| if (activePositions.TryGetValue(_sc.Key, out _scPos) && _scPos != null | |
| && _scPos.PendingCleanup && _scPos.RemainingContracts <= 0) | |
| { | |
| lock (stateLock) | |
| { | |
| activePositions.TryRemove(_sc.Key, out _); | |
| stopOrders.TryRemove(_sc.Key, out _); | |
| } | |
| SymmetryGuardForgetEntry(_sc.Key); | |
| Print("[A2-2] Deferred PendingCleanup purge (follower stop terminal): " + _sc.Key); | |
| } |
| # Cursor Rules: Universal OR Strategy V12 | ||
|
|
||
| Follow these rules for ALL code modifications. Reference **[.agent/standards_manifesto.md](file:///.agent/standards_manifesto.md)** for permanent safety standards. | ||
| **CRITICAL INSTRUCTION FOR CURSOR AI:** | ||
| You must read, ingest, and strictly adhere to the permanent project standards defined in the `c:\WSGTA\universal-or-strategy\.agent\standards_manifesto.md` file before making any code suggestions or architectural decisions. | ||
|
|
||
| - **Language**: C# 8.0 / .NET Framework 4.8 (NinjaTrader 8). | ||
| - **Concurrency**: All state mutations (activePositions, expectedPositions) MUST be guarded by lock(stateLock). | ||
| - **Lifecycle**: Semaphores (_simaToggleSem) MUST be released in finally blocks. | ||
| - **Refactoring**: Prefer explicit FirstOrDefault logic for instrument lookups (Reaper parity). | ||
| - **Style**: Use PascalCase for methods, camelCase for local variables. Avoid dense one-liners; prioritize "Metabolic Elegance." | ||
|
|
||
| ## 🛡️ Protocol Hardening (V12.Phase7) | ||
|
|
||
| ### 1. Scope Control | ||
| - **Surgical Edits**: AI agents MUST restrict code modifications to the specific files requested in the Mission Brief. NEVER refactor unrelated files without explicit Director authorization. | ||
| - **Zero-Trust Planning**: Always generate an `implementation_plan.md` before applying code changes to local files. | ||
|
|
||
| ### 2. WPF/UI Guardrails | ||
| - **Escalation**: If a UI layout or positioning task enters a loop (more than 2 failed attempts), the agent MUST halt and escalate to the Director for manual layout review. | ||
| - **Headless Mode**: Prefer headless execution for batch logic updates; do not attempt complex UI re-styling without a visual brief. | ||
|
|
||
| ### 3. Path & Deployment Management | ||
| - **Source Truth**: All primary NinjaScript logic resides in `src/`. | ||
| - **Deployment**: Local builds MUST be synced to `C:\Users\Mohammed Khalid\Documents\NinjaTrader 8\bin\Custom\Strategies\` using the `/deploy` skill. | ||
|
|
||
| ## 🕹️ Director Commands ($) | ||
|
|
||
| - **$PLAN_AUDIT**: Use `read_terminal` on the active Claude/Antigravity PID to ingest Sonnet's implementation plan. Perform a forensic logic audit before recommending approval to the Director. | ||
| - **$MISSION**: Initialize a new project phase via a Mission Brief artifact. | ||
| - **$AUDIT**: Trigger the `/audit` skill to scan the `src/` directory. | ||
|
|
||
| ## Agent Synchronization | ||
| AI Agents (Anthropic, Codex, Antigravity, Cursor, Gemini) MUST follow the **[.agent/standards_manifesto.md](file:///.agent/standards_manifesto.md)** as the primary source of truth for architectural standards and safety protocols. | ||
|
|
||
|
|
||
| ## CRITICAL: ASCII-Only in All C# String Literals | ||
| - NEVER use emoji, curly quotes, em-dashes, Unicode arrows, or box-drawing in Print() or any string literal. | ||
| - Non-ASCII inside C# strings breaks the NinjaTrader compiler with 300+ cascading errors (Build 936 incident). | ||
| - Allowed substitutions: (!) not emoji, -- not em-dash, -> not arrow, straight " not curly " " | ||
| - See .agent/standards_manifesto.md Section 7 for the full rule table and emergency fix sequence. | ||
| Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. |
There was a problem hiding this comment.
Critical inline rules removed — AI reviewers lose key guardrails
The previous .cursorrules contained several project-specific rules that are not derivable from a general read of the manifesto, including:
- ASCII-only string literals — Unicode in
Print()causes 300+ NinjaTrader compile errors (Build 936 incident). stateLockrequirement for allactivePositions/expectedPositionsmutations.- Auto-reject conditions (e.g.
Account.Flatten()on background threads, stateless IPC decoder, blanketexpectedPositions[key] = 0). - Naming conventions for signals (BMad Protocol).
Cursor AI reads .cursorrules directly as its active context — it does not automatically chase transitive references to other files. Replacing all inline rules with a single-line pointer means any Cursor session operating outside this repository root (or without the manifesto pre-loaded) will have no actionable constraints. The high-severity rules that caused real production incidents (ASCII-only, stateLock) should stay inline, even if abbreviated.
| PositionInfo cancelledFollowerPos; | ||
| if (activePositions.TryGetValue(matchedEntry, out cancelledFollowerPos) && cancelledFollowerPos != null) | ||
| { | ||
| string cancelAcctKey = cancelledFollowerPos.ExecutingAccount != null | ||
| ? cancelledFollowerPos.ExecutingAccount.Name : Account.Name; | ||
| int cancelDelta = (cancelledFollowerPos.Direction == MarketPosition.Long) | ||
| ? -cancelledFollowerPos.TotalContracts : cancelledFollowerPos.TotalContracts; | ||
| DeltaExpectedPositionLocked(ExpKey(cancelAcctKey), cancelDelta); |
There was a problem hiding this comment.
A2-3 reads TotalContracts outside stateLock
cancelledFollowerPos is fetched via TryGetValue without holding stateLock, and cancelledFollowerPos.TotalContracts is then read outside the lock. Per the project's concurrency contract (.agent/standards_manifesto.md), shared PositionInfo field reads that feed into delta calculations should be snapshotted under stateLock. TotalContracts can be updated by HandleOrderUpdated (broker-confirmed quantity sync) from a background thread, so a torn read is possible.
Consider snapshotting the account key and the delta inside one lock(stateLock) block, similar to the pattern already used at line 455 (lock (stateLock) { _totalContracts = kvp.Value.TotalContracts; }) elsewhere in this same file.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c86228f57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| FleetDispatchRequest stale; | ||
| while (_pendingFleetDispatches.TryDequeue(out stale)) { } | ||
| Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained."); |
There was a problem hiding this comment.
This new abort branch drains _pendingFleetDispatches and returns before any of the per-request cleanup runs. Each queued request normally clears dispatch-sync state, rolls back ReservedDelta, and decrements _pendingFleetDispatchCount in finally; draining here skips all of that, so flatten/SIMA-off windows can leave expected positions and counters stuck, causing persistent desync and blocked later dispatch/repair behavior.
Useful? React with 👍 / 👎.
| if (isFlattenRunning) | ||
| { | ||
| Print("[REAPER REPAIR] Aborted -- flatten in progress."); | ||
| return; |
There was a problem hiding this comment.
Repairs are now marked in-flight before enqueue, but this early return exits before the try/finally that removes _repairInFlight. If isFlattenRunning is true when the queued repair executes, the in-flight marker is leaked and future audits will keep skipping that account as already repairing, so automated re-entry repair can stop permanently for that account/instrument key.
Useful? React with 👍 / 👎.
| RequestStopCancelLifecycleSafe(entryName); | ||
| activePositions.TryRemove(entryName, out _); | ||
| SymmetryGuardForgetEntry(entryName); | ||
| PositionInfo closedPos; | ||
| if (activePositions.TryGetValue(entryName, out closedPos) && closedPos != null) | ||
| closedPos.PendingCleanup = true; |
There was a problem hiding this comment.
On full target close, this path now sets PendingCleanup and defers purge to a later stop-terminal callback. But RequestStopCancelLifecycleSafe returns immediately when stopOrders has no entry, so if the stop reference was already removed (e.g., cancel/update arrived first), no subsequent event will clear this position and the strategy keeps a zero-contract ghost in activePositions.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/V12_002.Entries.RMA.cs (1)
297-315:⚠️ Potential issue | 🟠 MajorRollback the master reservation on thrown submit failures too.
Both methods reserve
expectedPositionsbefore callingSubmitOrderUnmanaged, but only thenullreturn path undoes it. If the submit throws, the outer catch logs and leaves a phantom master reservation behind, which REAPER will treat as a real desync.Also applies to: 416-431
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/V12_002.Entries.RMA.cs` around lines 297 - 315, The code reserves a master expected position via AddExpectedPositionDeltaLocked(ExpKey(Account.Name), masterDeltaRMA) before calling SubmitOrderUnmanaged but only rolls back on a NULL return; if SubmitOrderUnmanaged throws the reservation is left behind. Surround the SubmitOrderUnmanaged call in a try/catch that on any exception calls AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaRMA) to undo the reservation, logs the error (include exception details) and mirrors the NULL-path behavior (Print and Draw.Text) then return/abort; apply the same try/catch+rollback change to the other symmetric submit block (the similar code around the other entry/exit that also reserves expectedPositions).src/V12_002.Orders.Management.cs (1)
521-531:⚠️ Potential issue | 🔴 CriticalValidate follower stop creation before calling
Submit().In the follower branch,
Submit()runs before the null guard at Line 545. IfCreateOrder()returnsnull, orSubmit()throws, control skips the flatten path and the position can stay unprotected.Suggested fix
- newStop = pos.ExecutingAccount.CreateOrder(Instrument, exitAction, - OrderType.StopMarket, TimeInForce.Gtc, quantity, 0, stopPrice, _b950OcoId, sigName, null); - pos.ExecutingAccount.Submit(new[] { newStop }); + newStop = pos.ExecutingAccount.CreateOrder(Instrument, exitAction, + OrderType.StopMarket, TimeInForce.Gtc, quantity, 0, stopPrice, _b950OcoId, sigName, null); + if (newStop == null) + { + Print(string.Format("[STOP_GUARD] CreateOrder returned null for {0}. Flattening.", entryName)); + FlattenPositionByName(entryName); + return; + } + try + { + pos.ExecutingAccount.Submit(new[] { newStop }); + } + catch (Exception submitEx) + { + Print(string.Format("[STOP_GUARD] Submit failed for {0}: {1}. Flattening.", entryName, submitEx.Message)); + FlattenPositionByName(entryName); + return; + }Also applies to: 545-558
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/V12_002.Orders.Management.cs` around lines 521 - 531, In the follower branch where you call pos.ExecutingAccount.CreateOrder(...) and pos.ExecutingAccount.Submit(...), validate that newStop is not null before calling Submit, and wrap the Submit call in a try/catch; on null or any exception, invoke the same fallback/flatten-protection logic used for the non-follower path so the position remains protected (and log the failure), otherwise proceed with Submit; update the code around symbols activePositions, pos, CreateOrder, newStop and Submit accordingly.src/V12_002.Trailing.cs (1)
1011-1031:⚠️ Potential issue | 🟠 MajorFollower target replacement uses banned Cancel+Submit pattern instead of two-phase FSM.
Lines 1016-1023 perform raw
Cancel()followed immediately bySubmit()for fleet follower target orders. Per coding guidelines and retrieved learnings, follower order cancel+resubmit must use the two-phase Replace FSM (PendingCancel→ wait forOnAccountOrderUpdateconfirm →Submitting→SubmitFollowerReplacement) with the_followerReplaceSpecsdictionary.While
StampReaperMoveGrace()at line 1015 helps suppress REAPER false positives during the gap, it does not prevent ghost orders if the cancel confirmation arrives after the new order is already submitted.Consider implementing a target replacement FSM similar to the stop replacement pattern, or deferring the new order submission until the cancel is broker-confirmed.
Based on learnings: "Raw
Cancel()followed immediately bySubmit()for follower orders is BANNED; use the Replace FSM instead to prevent ghost orders" and "Follower order cancel+resubmit must use the two-phase Replace FSM with states: PendingCancel -> wait for OnAccountOrderUpdate confirm -> Submitting -> SubmitFollowerReplacement."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/V12_002.Trailing.cs` around lines 1011 - 1031, Replace the raw Cancel()+Submit() follower flow in the block that calls StampReaperMoveGrace(), pos.ExecutingAccount.Cancel(...), CreateOrder(...) and pos.ExecutingAccount.Submit(...) with the two‑phase follower Replace FSM: instead of immediately creating/submitting newFollowerOrder, record a replacement spec in the _followerReplaceSpecs dictionary keyed by targetNum/entryName (include desired newTargetPrice, quantity, targetOrderName, etc.), call pos.ExecutingAccount.Cancel(...) and return to PendingCancel, then perform the CreateOrder + Submit step from the handler that processes the broker cancel confirmation (OnAccountOrderUpdate) by transitioning the spec to Submitting and invoking the existing SubmitFollowerReplacement (or equivalent) routine which must update GetTargetOrdersDictionary(entryName) to the new order and increment movedCount/print the log; keep the StampReaperMoveGrace() call before Cancel.
🧹 Nitpick comments (3)
test-vertex.js (1)
21-33: Test lacks summary output and exit code.The test iterates through models and locations but doesn't aggregate results or set an exit code for CI integration. Consider tracking pass/fail counts and exiting with non-zero on failures.
Proposed enhancement
async function runTests() { const models = [ 'gemini-3.1-pro-preview', 'gemini-3.1-pro', ]; const locations = ['global', 'us-central1']; + let passed = 0, failed = 0; for (const model of models) { for (const loc of locations) { - await testModel(model, loc); + if (await testModel(model, loc)) passed++; else failed++; } } + console.log(`\nResults: ${passed} passed, ${failed} failed`); + process.exit(failed > 0 ? 1 : 0); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-vertex.js` around lines 21 - 33, The runTests function currently invokes testModel for each model/location but doesn't collect results or set an exit code; update runTests to track pass and fail counts (e.g., variables passed=0, failed=0), wrap each await testModel(model, loc) call in try/catch to increment passed on success and failed on error (and log per-test outcome), print a concise summary of totals after the loops, and call process.exit(1) if failed>0 else process.exit(0) so CI can detect failures; reference the runTests, testModel, models, and locations symbols when making these changes.prdiff.txt (1)
179-179: Corrupted Unicode characters in error message.Line 179 contains garbled characters
"ÔÜá´©Å"which appear to be encoding corruption (likely intended emoji). While the ASCII-only guideline specifically targets C# code, this will render poorly in PR comments if the Vertex AI call fails.Consider using plain ASCII:
- reviewText = "ÔÜá´©Å **Vertex AI Audit Failed to Execute**: " + error.message; + reviewText = "(!) **Vertex AI Audit Failed to Execute**: " + error.message;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prdiff.txt` at line 179, The string assigned to reviewText contains corrupted Unicode ("ÔÜá´©Å")—replace that fragment with a plain ASCII message such as "[Vertex AI Audit Failed to Execute]" (or similar clear ASCII-only prefix) so reviewText = "[Vertex AI Audit Failed to Execute] : " + error.message; ensure you update the assignment to the reviewText variable where it currently concatenates error.message.src/V12_002.Trailing.cs (1)
685-686: RedundantstopOrdersassignment.Line 686 writes
stopOrders[entryName] = newStopafter the order is already assigned at either line 638 (ExecutingAccount path) or line 650 (SubmitOrderUnmanaged path). This duplicate write is harmless but unnecessary since control flow ensures one of those earlier assignments already executed.Consider removing this line to avoid confusion about when the dictionary is actually updated.
Proposed fix
- // A1-1: stopOrders final write inside stateLock (Build 960 audit fix) - lock (stateLock) { stopOrders[entryName] = newStop; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/V12_002.Trailing.cs` around lines 685 - 686, Remove the redundant assignment inside the lock: delete the line that sets stopOrders[entryName] = newStop within lock(stateLock). The dictionary is already updated earlier on the ExecutingAccount and SubmitOrderUnmanaged paths, so keep the lock (stateLock) block if other synchronization is needed but remove the duplicate stopOrders write; verify stopOrders, entryName and newStop remain correctly assigned by the earlier code paths (ExecutingAccount / SubmitOrderUnmanaged) before this lock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agent/standards_manifesto.md:
- Around line 27-29: The manifesto removed the mandatory Antigravity audit from
Sonnet's handoff -> implementation loop; restore an explicit step requiring
Sonnet to send its implementation plan to Antigravity for review before creating
branches or PRs by updating the text around "Lead Engineer (Claude/Sonnet)",
"The Loop", and the handoff artifacts ("implementation_plan.md",
"session_handoff.md", "Mission Brief") to state: Sonnet must paste its plan to
Antigravity for audit and receive approval (or documented findings) before
branch creation/PR submission; apply the same insertion to the corresponding
block referenced (lines ~168-173) so the audit gate is present in both places.
In @.cursorrules:
- Line 6: Remove the duplicated word "absolute" in the manifesto sentence that
currently reads "absolute absolute single source of truth" by editing the phrase
to "absolute single source of truth" (locate the occurrence of "absolute
absolute" and delete the extra "absolute").
In @.github/copilot-instructions.md:
- Line 6: Fix the duplicated word in the manifesto sentence by removing the
extra "absolute" so the phrase reads "the absolute single source of truth";
locate the string "absolute absolute single source of truth" in the text and
update it to "absolute single source of truth" to eliminate the typo.
In `@codex_audit.md`:
- Around line 14-27: The markdown table row for F2 in codex_audit.md has one
extra cell causing a column-count mismatch; edit the F2 row (the line containing
"F2 | Critical | Area 1 | protocol drift |
`src/V12_002.Orders.Management.cs:832-843` | ...") so that it has exactly the
same 8 pipe-separated columns as the header (remove or merge the stray "|" or
extra cell text), ensure the backticked file:line cell stays single-cell, and
preview in GitHub to confirm the table renders correctly.
In `@pr_checks.json`:
- Line 1: The file pr_checks.json contains a leading UTF-8 BOM character
(U+FEFF) at the start which can break JSON parsers; remove the BOM by re-saving
pr_checks.json as "UTF-8 without BOM" or strip the leading '\uFEFF' character
from the file start so the first character is '['; ensure any tooling or commit
hook that emits this file writes UTF-8 without BOM going forward.
In `@prdiff.txt`:
- Around line 314-329: SendCommand is sending IPC commands directly to tcpStream
without client-side validation; before writing to tcpStream in SendCommand,
check that the command is non-null/non-empty, its length <= IpcMaxCommandLength,
and that the action token is present in AllowedIpcActions (parse the command to
extract the action name the same way the server expects). If validation fails,
log or Print a rejection and return without acquiring tcpLock or calling
tcpStream.Write; only acquire tcpLock and write after the allowlist and length
checks pass to prevent sending invalid/oversized IPC payloads.
In `@src/V12_002.cs`:
- Around line 361-369: PendingCleanup and FlattenAttemptCount on PositionInfo
are mutated without synchronization; wrap every write/read-modify of these
fields with lock(stateLock) to ensure thread-safety. Locate all assignments like
PendingCleanup = true and increments/assignments to FlattenAttemptCount (e.g.,
in Orders.Callbacks and Trailing code paths) and change them to use
lock(stateLock) { /* mutate PendingCleanup or FlattenAttemptCount here */ } so
all state mutations on PositionInfo (and any related
activePositions/expectedPositions updates) are performed inside the critical
section guarded by stateLock.
In `@src/V12_002.Entries.FFMA.cs`:
- Around line 172-185: The POSITION_ENTERED notification is sent before the
order is actually submitted; move the SendResponseToRemote call so it only
executes after a successful SubmitOrderUnmanaged and only when entryOrder is
non-null (and after catching any exceptions from SubmitOrderUnmanaged).
Specifically, wrap the SubmitOrderUnmanaged call in a try/catch, set entryOrder
from SubmitOrderUnmanaged, check entryOrder != null, and then call
SendResponseToRemote("POSITION_ENTERED|FFMA|{0}", contracts); otherwise perform
the existing rollback/return logic (use the existing Print and return paths) to
ensure the UI is only notified after a confirmed order submission.
In `@src/V12_002.Entries.RMA.cs`:
- Around line 114-130: If SubmitOrderUnmanaged for entryOrder2 returns null,
abort the rest of the split-entry workflow: undo any mutations made for entry2
(remove linkedTRENDEntries[entry1Name] and linkedTRENDEntries[entry2Name],
remove entry2Name from masterEntryNames, do not add entryOrders[entry2Name] or
activePositions[entry2Name]), and recompute or adjust finalTotalQty and
weightedEntryPrice to reflect only E1 (or skip updating them). Also ensure the
downstream SIMA dispatch logic is skipped for this split (return/short-circuit
after rolling back) so follower sizing isn't reserved/dispatched against a
non-existent E2; apply these changes around the entryOrder2 null-check in the
block handling linkedTRENDEntries/entryOrders.
In `@src/V12_002.Entries.Trend.cs`:
- Around line 245-252: When SubmitOrderUnmanaged returns null for E2, the code
only rolls back E2 and leaves E1 active; fix by making the operation atomic:
inside the null-check for entryOrder2 (where AddExpectedPositionDeltaLocked is
called) also cancel and remove the paired E1 (use linkedTRENDEntries to find the
partner), under stateLock cancel the live broker order for E1, remove E1 from
activePositions and entryOrders, adjust expected deltas (via
AddExpectedPositionDeltaLocked) for the cancelled E1, and log the abort;
alternatively, if you prefer monitoring, register the linked pair so
OnAccountOrderUpdate will detect and clean orphaned entries by removing entries
from activePositions/entryOrders and cancelling orders when their partner
fails—ensure all modifications happen under stateLock and use the same helper
methods (SubmitOrderUnmanaged, AddExpectedPositionDeltaLocked,
linkedTRENDEntries, OnAccountOrderUpdate, activePositions, entryOrders) for
consistency.
In `@src/V12_002.Orders.Callbacks.cs`:
- Around line 1035-1045: In OnExecutionUpdate, do not unconditionally remove the
stopOrders entry on the first execution callback; instead, inside the stateLock
only remove stopOrders and decrement pendingReplacementCount
(pendingStopReplacements) when the stop order is in a terminal state or fully
filled (e.g., order.IsTerminal or remainingAfterStop <= 0 /
order.RemainingQuantity == 0). If the execution is a partial fill, leave
stopOrders in place (and do not decrement pendingReplacementCount) so the
partially filled stop remains tracked; still handle activePositions and
entryOrders removal only when the stop is truly finished. Reference:
OnExecutionUpdate, stopOrders, pendingStopReplacements, pendingReplacementCount,
remainingAfterStop, activePositions, entryOrders, stateLock.
- Around line 630-640: The confirmed-cancel path updates expectedPositions via
DeltaExpectedPositionLocked but never clears the dispatch-sync barrier, so add
logic around this rollback to set and then clear the account's entry in
_dispatchSyncPendingExpKeys (use the same ExpKey(cancelAcctKey) key) so the
barrier is not left stuck; specifically, before calling
DeltaExpectedPositionLocked(ExpKey(cancelAcctKey), ...) ensure the key is marked
pending in _dispatchSyncPendingExpKeys and after the rollback clear/remove that
key from _dispatchSyncPendingExpKeys so REAPER's syncPending is released for
cancelledFollowerPos/ cancelAcctKey/matchedEntry.
In `@src/V12_002.Orders.Management.cs`:
- Around line 301-305: CancelOrder(existingOrder) is asynchronous, so removing
the entry from targetDict right after calling CancelOrder lets a
delayed/rejected cancel leave an active target untracked; instead, keep the
targetDict entry until the broker-confirmed terminal callback clears it. Change
the flow around CancelOrder/targetDict: call CancelOrder(existingOrder) but do
not call lock(stateLock) { targetDict.TryRemove(entryName, out _); } here; rely
on the existing broker-confirmed terminal callback (the method that currently
removes entries on terminal state) to perform targetDict.TryRemove(entryName,
out _) under stateLock once the cancel is confirmed terminal, ensuring
entryName, existingOrder and stateLock are used unmodified until that callback
runs.
In `@src/V12_002.REAPER.cs`:
- Around line 287-291: The code marks repairKey in _repairInFlight under
stateLock before scheduling but can return or swallow exceptions without
clearing that guard, causing permanent “in-flight” state; update the submit path
around _repairInFlight, _reaperRepairQueue.Enqueue(acct.Name) and
TriggerCustomEvent(ProcessReaperRepairQueue) so that: (1) if the flatten guard
causes an early return you still re-enqueue the account and then remove
repairKey from _repairInFlight under stateLock; (2) wrap TriggerCustomEvent in a
try/catch/finally and in the catch/finally ensure you remove repairKey from
_repairInFlight under stateLock if the submit failed; and (3) apply the same fix
to the equivalent block that handles the other repair submission (the similar
pattern referenced in the review) so the guard is always cleared on every
scheduling or pre-submit abort path.
In `@src/V12_002.SIMA.cs`:
- Around line 640-647: When draining _pendingFleetDispatches in the abort path,
you must rollback the reserved deltas for each discarded FleetDispatchRequest to
avoid leaving stale ReservedDelta amounts; for each dequeued request (stale)
call AddExpectedPositionDeltaLocked(stale.Key, -stale.ReservedDelta) (using the
same key/ReservedDelta members on FleetDispatchRequest) so the expectedPositions
reservation is reversed before continuing, ensuring you perform this rollback
inside the same lock/context that AddExpectedPositionDeltaLocked expects.
- Around line 829-834: The SIMA disable shutdown block that drains
_pendingFleetDispatches currently discards FleetDispatchRequest items without
rolling back ReservedDelta or clearing _dispatchSyncPendingExpKeys; update that
loop in the ApplySimaState/disable path to replicate the rollback used by
PumpFleetDispatch: for each dequeued FleetDispatchRequest, undo any
ReservedDelta increments (decrement the same counters/entries updated when the
request was queued) and remove its keys from _dispatchSyncPendingExpKeys (or
call the existing helper used by PumpFleetDispatch if one exists), then continue
draining and log the cleared queue as before.
In `@test-vertex.js`:
- Around line 5-8: The code is using an invalid Vertex AI model identifier;
update the model passed to VertexAI.preview.getGenerativeModel (the call that
creates generativeModel) to a valid preview model name (e.g., replace
'gemini-3.1-pro' with 'gemini-3.1-pro-preview' or another supported preview name
like 'gemini-3-pro-preview' or 'gemini-3-flash-preview'), or remove the invalid
model reference entirely so modelName in test-vertex.js refers only to a
supported model.
---
Outside diff comments:
In `@src/V12_002.Entries.RMA.cs`:
- Around line 297-315: The code reserves a master expected position via
AddExpectedPositionDeltaLocked(ExpKey(Account.Name), masterDeltaRMA) before
calling SubmitOrderUnmanaged but only rolls back on a NULL return; if
SubmitOrderUnmanaged throws the reservation is left behind. Surround the
SubmitOrderUnmanaged call in a try/catch that on any exception calls
AddExpectedPositionDeltaLocked(ExpKey(Account.Name), -masterDeltaRMA) to undo
the reservation, logs the error (include exception details) and mirrors the
NULL-path behavior (Print and Draw.Text) then return/abort; apply the same
try/catch+rollback change to the other symmetric submit block (the similar code
around the other entry/exit that also reserves expectedPositions).
In `@src/V12_002.Orders.Management.cs`:
- Around line 521-531: In the follower branch where you call
pos.ExecutingAccount.CreateOrder(...) and pos.ExecutingAccount.Submit(...),
validate that newStop is not null before calling Submit, and wrap the Submit
call in a try/catch; on null or any exception, invoke the same
fallback/flatten-protection logic used for the non-follower path so the position
remains protected (and log the failure), otherwise proceed with Submit; update
the code around symbols activePositions, pos, CreateOrder, newStop and Submit
accordingly.
In `@src/V12_002.Trailing.cs`:
- Around line 1011-1031: Replace the raw Cancel()+Submit() follower flow in the
block that calls StampReaperMoveGrace(), pos.ExecutingAccount.Cancel(...),
CreateOrder(...) and pos.ExecutingAccount.Submit(...) with the two‑phase
follower Replace FSM: instead of immediately creating/submitting
newFollowerOrder, record a replacement spec in the _followerReplaceSpecs
dictionary keyed by targetNum/entryName (include desired newTargetPrice,
quantity, targetOrderName, etc.), call pos.ExecutingAccount.Cancel(...) and
return to PendingCancel, then perform the CreateOrder + Submit step from the
handler that processes the broker cancel confirmation (OnAccountOrderUpdate) by
transitioning the spec to Submitting and invoking the existing
SubmitFollowerReplacement (or equivalent) routine which must update
GetTargetOrdersDictionary(entryName) to the new order and increment
movedCount/print the log; keep the StampReaperMoveGrace() call before Cancel.
---
Nitpick comments:
In `@prdiff.txt`:
- Line 179: The string assigned to reviewText contains corrupted Unicode
("ÔÜá´©Å")—replace that fragment with a plain ASCII message such as "[Vertex AI
Audit Failed to Execute]" (or similar clear ASCII-only prefix) so reviewText =
"[Vertex AI Audit Failed to Execute] : " + error.message; ensure you update the
assignment to the reviewText variable where it currently concatenates
error.message.
In `@src/V12_002.Trailing.cs`:
- Around line 685-686: Remove the redundant assignment inside the lock: delete
the line that sets stopOrders[entryName] = newStop within lock(stateLock). The
dictionary is already updated earlier on the ExecutingAccount and
SubmitOrderUnmanaged paths, so keep the lock (stateLock) block if other
synchronization is needed but remove the duplicate stopOrders write; verify
stopOrders, entryName and newStop remain correctly assigned by the earlier code
paths (ExecutingAccount / SubmitOrderUnmanaged) before this lock.
In `@test-vertex.js`:
- Around line 21-33: The runTests function currently invokes testModel for each
model/location but doesn't collect results or set an exit code; update runTests
to track pass and fail counts (e.g., variables passed=0, failed=0), wrap each
await testModel(model, loc) call in try/catch to increment passed on success and
failed on error (and log per-test outcome), print a concise summary of totals
after the loops, and call process.exit(1) if failed>0 else process.exit(0) so CI
can detect failures; reference the runTests, testModel, models, and locations
symbols when making these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bcc3a2eb-da60-44a3-9380-bf7d4315c0d5
📒 Files selected for processing (30)
.agent/standards_manifesto.md.claude.json.claude/worktrees/affectionate-franklin.claude/worktrees/sleepy-jepsen.cursorrules.github/copilot-instructions.md.github/workflows/claude-code-review.yml.disabledGEMINI.mdcodex_audit.mddiff.txtgh_log.txthelp.txtpr24_audit.jsonpr24_audit_utf8.jsonpr26_diff.txtpr_checks.jsonprdiff.txtsrc/V12_002.Entries.FFMA.cssrc/V12_002.Entries.MOMO.cssrc/V12_002.Entries.OR.cssrc/V12_002.Entries.RMA.cssrc/V12_002.Entries.Trend.cssrc/V12_002.Orders.Callbacks.cssrc/V12_002.Orders.Management.cssrc/V12_002.REAPER.cssrc/V12_002.SIMA.cssrc/V12_002.Symmetry.cssrc/V12_002.Trailing.cssrc/V12_002.cstest-vertex.js
| - **Lead Engineer (Claude/Sonnet):** The **"Execution Specialist."** Use **Sonnet** (latest) for all implementation. Sonnet is functionally autonomous: it creates its own branches, executes repairs, and **calls for automated audits** by opening PRs. | ||
| - **The Loop:** Repair Mission -> Sonnet Handoff -> Branch/Code/Push -> PR Re-Audit Matrix (Opus/Gemini) -> Merge. | ||
| - **The Bridge:** Handoffs are managed via `implementation_plan.md`, `session_handoff.md`, and **Mission Brief** artifacts. |
There was a problem hiding this comment.
Put the Antigravity plan-audit gate back into the default loop.
The new loop and autonomy text jump from Sonnet handoff straight into implementation and PR creation. In a live-trading repo, that teaches agents to skip the explicit Antigravity review step that is supposed to catch mistakes before code lands.
Based on learnings, Paste Sonnet's plan to Antigravity for audit before approving any live trading fix. Sonnet may catch brief errors.
Also applies to: 168-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agent/standards_manifesto.md around lines 27 - 29, The manifesto removed
the mandatory Antigravity audit from Sonnet's handoff -> implementation loop;
restore an explicit step requiring Sonnet to send its implementation plan to
Antigravity for review before creating branches or PRs by updating the text
around "Lead Engineer (Claude/Sonnet)", "The Loop", and the handoff artifacts
("implementation_plan.md", "session_handoff.md", "Mission Brief") to state:
Sonnet must paste its plan to Antigravity for audit and receive approval (or
documented findings) before branch creation/PR submission; apply the same
insertion to the corresponding block referenced (lines ~168-173) so the audit
gate is present in both places.
| - Non-ASCII inside C# strings breaks the NinjaTrader compiler with 300+ cascading errors (Build 936 incident). | ||
| - Allowed substitutions: (!) not emoji, -- not em-dash, -> not arrow, straight " not curly " " | ||
| - See .agent/standards_manifesto.md Section 7 for the full rule table and emergency fix sequence. | ||
| Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. |
There was a problem hiding this comment.
Typo: "absolute absolute" (duplicate word).
Line 6 contains a repeated word.
✏️ Proposed fix
-Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity.
+Do not deviate from those rules. The manifesto is the absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity.📝 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.
| Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. | |
| Do not deviate from those rules. The manifesto is the absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.cursorrules at line 6, Remove the duplicated word "absolute" in the
manifesto sentence that currently reads "absolute absolute single source of
truth" by editing the phrase to "absolute single source of truth" (locate the
occurrence of "absolute absolute" and delete the extra "absolute").
| - `IsReaperFillGraceActive()` without an account key argument (global grace bug) | ||
| - `Encoding.UTF8.GetString(buffer, ...)` in the IPC receive loop (stateless decoder) | ||
| - Any IPC command processed without passing through `AllowedIpcActions` check | ||
| Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. |
There was a problem hiding this comment.
Typo: Duplicated word "absolute".
The phrase "absolute absolute single source of truth" contains a duplicated word.
Proposed fix
-Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity.
+Do not deviate from those rules. The manifesto is the absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity.📝 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.
| Do not deviate from those rules. The manifesto is the absolute absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. | |
| Do not deviate from those rules. The manifesto is the absolute single source of truth for architecture, locking, repo hygiene, and multi-agent parity. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/copilot-instructions.md at line 6, Fix the duplicated word in the
manifesto sentence by removing the extra "absolute" so the phrase reads "the
absolute single source of truth"; locate the string "absolute absolute single
source of truth" in the text and update it to "absolute single source of truth"
to eliminate the typo.
| | ID | Severity | Area | Class | File:Line | Evidence | Failure mode | Hardening direction | | ||
| | --- | --- | --- | --- | --- | --- | --- | --- | | ||
| | F1 | Critical | Area 1 | protocol drift | `src/V12_002.Entries.FFMA.cs:172`, `src/V12_002.Entries.MOMO.cs:146`, `src/V12_002.Entries.OR.cs:206`, `src/V12_002.Entries.RMA.cs:93`, `src/V12_002.Entries.Trend.cs:207`, `src/V12_002.Orders.Callbacks.cs:961`, `src/V12_002.SIMA.cs:661`, `src/V12_002.Trailing.cs:637` | Confirmed: `activePositions`, `entryOrders`, `stopOrders`, and target dictionaries are still written and removed outside `lock(stateLock)`. | REAPER, callbacks, and UI/IPC flows can observe half-committed state across multiple dictionaries. | Move all shared-state mutations and multi-dictionary teardown under one lock protocol. | | ||
| | F2 | Critical | Area 1 | protocol drift | `src/V12_002.Orders.Management.cs:832-843` | Confirmed: follower CIT still does raw `Cancel()` -> `CreateOrder()` -> `Submit()` and then swaps `entryOrders`. | Direct ghost-order window on follower entry replacement; violates `GEMINI.md` Build 947 rule. | Route CIT follower nudges through the same two-phase replace FSM as entry move-sync. | | ||
| | F3 | Critical | Area 3 | systemic architecture flaw | `src/V12_002.SIMA.cs:503-521`, `src/V12_002.SIMA.cs:551-559`, `src/V12_002.SIMA.cs:638-676`, `src/V12_002.SIMA.cs:815-822` | Confirmed: `_pendingFleetDispatches` survives SIMA disable; `PumpFleetDispatch()` has no `EnableSIMA`, `State`, or flatten guard. | Queued follower orders can be submitted after `SET_SIMA|OFF` or termination, after handlers and REAPER are stopped. | Drain or cancel pending dispatches on lifecycle shutdown and gate the pump before submit. | | ||
| | F4 | High | Area 1 | protocol drift | `src/V12_002.Orders.Callbacks.cs:1334-1349`, `src/V12_002.Symmetry.cs:555-577`, `src/V12_002.Trailing.cs:617-637`, `src/V12_002.Trailing.cs:979-990` | Confirmed: several follower stop/target move paths still cancel and resubmit directly. | Build 947 FSM is not the repo-wide truth; follower order behavior differs by call site. | Either broaden FSM coverage to all follower replace classes or document separate safe FSMs per order type. | | ||
| | F5 | High | Area 3 | race/backpressure | `src/V12_002.REAPER.cs:253-289`, `src/V12_002.REAPER.cs:313-320`, `src/V12_002.REAPER.cs:429-523`, `src/V12_002.REAPER.cs:635-636` | Confirmed: REAPER enqueues repairs before `_repairInFlight` is set and enqueues flattens with no in-flight dedupe at all. | Strategy-thread stalls can accumulate duplicate repair or flatten waves for the same account. | Set in-flight state at enqueue time and dedupe flatten queue entries. | | ||
| | F6 | High | Area 2 | null/cleanup defect | `src/V12_002.Entries.MOMO.cs:146-163`, `src/V12_002.Entries.FFMA.cs:172-184`, `src/V12_002.Entries.RMA.cs:400-415` | Confirmed: several entry paths pre-register state, then continue after `SubmitOrderUnmanaged()` returns `null`; MOMO still writes `entryOrders[entryName] = entryOrder` after a null rollback. | `ConcurrentDictionary` null writes can throw, leaving orphaned `activePositions` with no live order. | Abort and fully roll back all staged state on null submit before any dictionary assignment. | | ||
| | F7 | High | Area 2 | null/cleanup defect | `src/V12_002.Entries.FFMA.cs:172-184`, `src/V12_002.Entries.FFMA.cs:307-313`, `src/V12_002.Entries.FFMA.cs:446-452`, `src/V12_002.REAPER.cs:310-320`, `src/V12_002.REAPER.cs:398-405` | Confirmed: FFMA entry paths do not reserve `expectedPositions` before submit. | Legitimate FFMA fills can appear to REAPER as `actual != 0 && expected == 0`, which is treated as critical desync. | Reserve master expected state consistently across all entry modes. | | ||
| | F8 | High | Area 2 | null/cleanup defect | `src/V12_002.Orders.Callbacks.cs:96-128`, `src/V12_002.Orders.Callbacks.cs:1020-1029`, `src/V12_002.Orders.Callbacks.cs:1070-1079`, `src/V12_002.Orders.Callbacks.cs:784-793` | Confirmed: final-target and trim-close paths drop `activePositions` immediately after requesting lifecycle-safe stop cancel. | If stop cancel is delayed or rejected, later callbacks lose `PositionInfo` context and cleanup degrades to ghost handling. | Keep metadata until broker-confirmed terminal cleanup completes. | | ||
| | F9 | High | Area 3 | race/backpressure | `src/V12_002.cs:124`, `src/V12_002.cs:127`, `src/V12_002.UI.Compliance.cs:426-477`, `src/V12_002.Orders.Callbacks.cs:463-499` | Confirmed: account execution and account order queues are unbounded and reschedule themselves aggressively during flatten/backlog. | Large stale-event backlogs can replay after the live position state has moved on. | Bound the queues, coalesce wakeups, and stop re-enqueue churn during flatten. | | ||
| | F10 | Medium-High | Area 2 | race/backpressure | `src/V12_002.Orders.Management.cs:401-450`, `src/V12_002.Trailing.cs:501-568`, `src/V12_002.Orders.Callbacks.cs:368-383`, `src/V12_002.Orders.Callbacks.cs:599-611` | Confirmed: `pendingStopReplacements` values are mutated in place and read on other paths without immutable snapshots. | Mixed stop quantity/price/target snapshots can generate stale or malformed replacement stops under bursty ticks. | Make pending replacement records immutable per generation or guard all reads/writes under the same lock. | | ||
| | F11 | Medium-High | Area 2 | race/backpressure | `src/V12_002.Orders.Management.cs:407-414`, `src/V12_002.Orders.Management.cs:421-450` | Confirmed: `UpdateStopQuantity()` uses `ContainsKey()` and then indexes `stopOrders[entryName]` while other paths remove the same key. | TOCTOU can throw inside the stop-resize path and leave the position temporarily unprotected. | Use one `TryGetValue()` snapshot and keep the stop lifecycle entirely atomic. | | ||
| | F12 | Medium | Area 3 | systemic architecture flaw | `src/V12_002.REAPER.cs:211-214`, `src/V12_002.REAPER.cs:330-334`, `src/V12_002.REAPER.cs:456-474`, `src/V12_002.Orders.Management.cs:1451-1456` | Confirmed: REAPER and orphan sweeps dereference broker objects without full null guards and read live broker collections off-thread. | A partially adopted/manual order can abort scans; false desync and false naked-position decisions remain plausible. | Snapshot broker collections on the strategy thread and null-guard `Instrument` and `Name` before access. | |
There was a problem hiding this comment.
Markdown table formatting issue at Line 18.
The static analysis tool flagged a column count mismatch in the findings table. Row for F2 (Line 18) may have an extra cell causing "Expected: 8; Actual: 9". Verify the table renders correctly in GitHub's markdown preview.
🧰 Tools
🪛 LanguageTool
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...72, src/V12_002.Entries.MOMO.cs:146, src/V12_002.Entries.OR.cs:206, src/V12_002.Entries.RMA.cs...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~16-~16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...:146, src/V12_002.Entries.OR.cs:206, src/V12_002.Entries.RMA.cs:93, src/V12_002.Entries.Trend....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... src/V12_002.Entries.FFMA.cs:307-313, src/V12_002.Entries.FFMA.cs:446-452, `src/V12_002.REAPER.c...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.21.0)
[warning] 18-18: Table column count
Expected: 8; Actual: 9; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@codex_audit.md` around lines 14 - 27, The markdown table row for F2 in
codex_audit.md has one extra cell causing a column-count mismatch; edit the F2
row (the line containing "F2 | Critical | Area 1 | protocol drift |
`src/V12_002.Orders.Management.cs:832-843` | ...") so that it has exactly the
same 8 pipe-separated columns as the header (remove or merge the stray "|" or
extra cell text), ensure the backticked file:line cell stays single-cell, and
preview in GitHub to confirm the table renders correctly.
| @@ -0,0 +1 @@ | |||
| [{"name":"Compile NinjaScript (C# / .NET 4.8)","state":"IN_PROGRESS"},{"name":"codex-audit","state":"IN_PROGRESS"},{"name":"Label PR by changed files","state":"SUCCESS"},{"name":"CodeRabbit","state":"PENDING"},{"name":"DeepSource: C#","state":"PENDING"},{"name":"DeepSource: Secrets","state":"PENDING"}] | |||
There was a problem hiding this comment.
UTF-8 BOM detected at file start.
The leading character is a UTF-8 Byte Order Mark. While some JSON parsers handle this, others may fail or produce unexpected results. Consider removing the BOM for broader compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pr_checks.json` at line 1, The file pr_checks.json contains a leading UTF-8
BOM character (U+FEFF) at the start which can break JSON parsers; remove the BOM
by re-saving pr_checks.json as "UTF-8 without BOM" or strip the leading '\uFEFF'
character from the file start so the first character is '['; ensure any tooling
or commit hook that emits this file writes UTF-8 without BOM going forward.
| try | ||
| { | ||
| CancelOrder(existingOrder); | ||
| targetDict.TryRemove(entryName, out _); | ||
| lock (stateLock) { targetDict.TryRemove(entryName, out _); } | ||
| Print(string.Format("[SYNC_ALL] T{0} {1}: Limit cancelled -> now Runner", targetNum, entryName)); |
There was a problem hiding this comment.
Keep the target ref until the cancel is terminal.
CancelOrder() is asynchronous. Removing targetDict[entryName] immediately means a delayed or rejected cancel leaves a still-working target untracked, which recreates the ghost-order condition this file is trying to eliminate. Let the broker-confirmed terminal callback clear the dictionary instead.
Suggested fix
- CancelOrder(existingOrder);
- lock (stateLock) { targetDict.TryRemove(entryName, out _); }
+ CancelOrder(existingOrder);
+ // Keep the target ref until OnOrderUpdate confirms a terminal state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/V12_002.Orders.Management.cs` around lines 301 - 305,
CancelOrder(existingOrder) is asynchronous, so removing the entry from
targetDict right after calling CancelOrder lets a delayed/rejected cancel leave
an active target untracked; instead, keep the targetDict entry until the
broker-confirmed terminal callback clears it. Change the flow around
CancelOrder/targetDict: call CancelOrder(existingOrder) but do not call
lock(stateLock) { targetDict.TryRemove(entryName, out _); } here; rely on the
existing broker-confirmed terminal callback (the method that currently removes
entries on terminal state) to perform targetDict.TryRemove(entryName, out _)
under stateLock once the cancel is confirmed terminal, ensuring entryName,
existingOrder and stateLock are used unmodified until that callback runs.
| if (shouldLog) Print($"[REAPER] * REPAIR CANDIDATE: {acct.Name} is Flat, expected={expectedQty}. Enqueuing repair."); | ||
| // A3-2: Mark in-flight BEFORE TriggerCustomEvent to block double-enqueue in next audit cycle (Build 960 audit fix) | ||
| lock (stateLock) { _repairInFlight.Add(repairKey); } | ||
| _reaperRepairQueue.Enqueue(acct.Name); | ||
| try { TriggerCustomEvent(o => ProcessReaperRepairQueue(), null); } catch { } |
There was a problem hiding this comment.
Do not drop queued repairs with _repairInFlight still set.
The key is marked in-flight before scheduling, but the new flatten guard returns before the later cleanup path, and the TriggerCustomEvent catch also leaves the guard set. Once that happens, later audits keep seeing "repair already in-flight" and stop re-enqueueing this account. Requeue while flattening, and clear the guard on every scheduling or pre-submit abort path.
Based on learnings, _repairInFlight guard must be set before and cleared after every REAPER repair submit.
Also applies to: 532-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/V12_002.REAPER.cs` around lines 287 - 291, The code marks repairKey in
_repairInFlight under stateLock before scheduling but can return or swallow
exceptions without clearing that guard, causing permanent “in-flight” state;
update the submit path around _repairInFlight,
_reaperRepairQueue.Enqueue(acct.Name) and
TriggerCustomEvent(ProcessReaperRepairQueue) so that: (1) if the flatten guard
causes an early return you still re-enqueue the account and then remove
repairKey from _repairInFlight under stateLock; (2) wrap TriggerCustomEvent in a
try/catch/finally and in the catch/finally ensure you remove repairKey from
_repairInFlight under stateLock if the submit failed; and (3) apply the same fix
to the equivalent block that handles the other repair submission (the similar
pattern referenced in the review) so the guard is always cleared on every
scheduling or pre-submit abort path.
| // A3-1: Abort and drain queue if SIMA is disabled or flatten is running (Build 960 audit fix) | ||
| if (isFlattenRunning || !EnableSIMA) | ||
| { | ||
| FleetDispatchRequest stale; | ||
| while (_pendingFleetDispatches.TryDequeue(out stale)) { } | ||
| Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Missing delta rollback when draining dispatch queue.
The abort path drains _pendingFleetDispatches but does not roll back the ReservedDelta that was already applied to expectedPositions for each discarded request. This leaves stale reserved quantities that may cause REAPER to misfire or prevent future dispatches.
As per coding guidelines: "Reserved quantities must be rolled back via AddExpectedPositionDeltaLocked(key, -delta) on any fleet Submit exception."
🐛 Proposed fix to roll back reserved deltas during queue drain
// A3-1: Abort and drain queue if SIMA is disabled or flatten is running (Build 960 audit fix)
if (isFlattenRunning || !EnableSIMA)
{
FleetDispatchRequest stale;
- while (_pendingFleetDispatches.TryDequeue(out stale)) { }
+ while (_pendingFleetDispatches.TryDequeue(out stale))
+ {
+ // Roll back reserved expectedPositions delta for discarded request
+ if (stale.ReservedDelta != 0)
+ AddExpectedPositionDeltaLocked(stale.ExpectedKey, -stale.ReservedDelta);
+ ClearDispatchSyncPending(stale.ExpectedKey);
+ // Clean up tracking dicts for the aborted entry
+ activePositions.TryRemove(stale.FleetEntryName, out _);
+ entryOrders.TryRemove(stale.FleetEntryName, out _);
+ stopOrders.TryRemove(stale.FleetEntryName, out _);
+ for (int tNum = 1; tNum <= 5; tNum++)
+ {
+ var targetDict = GetTargetOrdersDictionary(tNum);
+ if (targetDict != null)
+ targetDict.TryRemove(stale.FleetEntryName, out _);
+ }
+ }
- Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained.");
+ Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained with rollback.");
return;
}📝 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.
| // A3-1: Abort and drain queue if SIMA is disabled or flatten is running (Build 960 audit fix) | |
| if (isFlattenRunning || !EnableSIMA) | |
| { | |
| FleetDispatchRequest stale; | |
| while (_pendingFleetDispatches.TryDequeue(out stale)) { } | |
| Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained."); | |
| return; | |
| } | |
| // A3-1: Abort and drain queue if SIMA is disabled or flatten is running (Build 960 audit fix) | |
| if (isFlattenRunning || !EnableSIMA) | |
| { | |
| FleetDispatchRequest stale; | |
| while (_pendingFleetDispatches.TryDequeue(out stale)) | |
| { | |
| // Roll back reserved expectedPositions delta for discarded request | |
| if (stale.ReservedDelta != 0) | |
| AddExpectedPositionDeltaLocked(stale.ExpectedKey, -stale.ReservedDelta); | |
| ClearDispatchSyncPending(stale.ExpectedKey); | |
| // Clean up tracking dicts for the aborted entry | |
| activePositions.TryRemove(stale.FleetEntryName, out _); | |
| entryOrders.TryRemove(stale.FleetEntryName, out _); | |
| stopOrders.TryRemove(stale.FleetEntryName, out _); | |
| for (int tNum = 1; tNum <= 5; tNum++) | |
| { | |
| var targetDict = GetTargetOrdersDictionary(tNum); | |
| if (targetDict != null) | |
| targetDict.TryRemove(stale.FleetEntryName, out _); | |
| } | |
| } | |
| Print("[PUMP] Abort: SIMA inactive or flatten running. Queue drained with rollback."); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/V12_002.SIMA.cs` around lines 640 - 647, When draining
_pendingFleetDispatches in the abort path, you must rollback the reserved deltas
for each discarded FleetDispatchRequest to avoid leaving stale ReservedDelta
amounts; for each dequeued request (stale) call
AddExpectedPositionDeltaLocked(stale.Key, -stale.ReservedDelta) (using the same
key/ReservedDelta members on FleetDispatchRequest) so the expectedPositions
reservation is reversed before continuing, ensuring you perform this rollback
inside the same lock/context that AddExpectedPositionDeltaLocked expects.
| // A3-1: Drain ghost dispatch queue on SIMA disable (Build 960 audit fix) | ||
| { | ||
| FleetDispatchRequest ignored; | ||
| while (_pendingFleetDispatches.TryDequeue(out ignored)) { } | ||
| Print("[SIMA] Dispatch queue cleared on shutdown."); | ||
| } |
There was a problem hiding this comment.
Same missing rollback issue on SIMA disable path.
This queue drain also discards requests without rolling back ReservedDelta or clearing _dispatchSyncPendingExpKeys. Apply the same rollback pattern as suggested for PumpFleetDispatch.
🐛 Proposed fix for ApplySimaState queue drain
// A3-1: Drain ghost dispatch queue on SIMA disable (Build 960 audit fix)
{
FleetDispatchRequest ignored;
- while (_pendingFleetDispatches.TryDequeue(out ignored)) { }
- Print("[SIMA] Dispatch queue cleared on shutdown.");
+ while (_pendingFleetDispatches.TryDequeue(out ignored))
+ {
+ if (ignored.ReservedDelta != 0)
+ AddExpectedPositionDeltaLocked(ignored.ExpectedKey, -ignored.ReservedDelta);
+ ClearDispatchSyncPending(ignored.ExpectedKey);
+ activePositions.TryRemove(ignored.FleetEntryName, out _);
+ entryOrders.TryRemove(ignored.FleetEntryName, out _);
+ stopOrders.TryRemove(ignored.FleetEntryName, out _);
+ for (int tNum = 1; tNum <= 5; tNum++)
+ {
+ var targetDict = GetTargetOrdersDictionary(tNum);
+ if (targetDict != null)
+ targetDict.TryRemove(ignored.FleetEntryName, out _);
+ }
+ }
+ Print("[SIMA] Dispatch queue cleared with rollback on shutdown.");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/V12_002.SIMA.cs` around lines 829 - 834, The SIMA disable shutdown block
that drains _pendingFleetDispatches currently discards FleetDispatchRequest
items without rolling back ReservedDelta or clearing
_dispatchSyncPendingExpKeys; update that loop in the ApplySimaState/disable path
to replicate the rollback used by PumpFleetDispatch: for each dequeued
FleetDispatchRequest, undo any ReservedDelta increments (decrement the same
counters/entries updated when the request was queued) and remove its keys from
_dispatchSyncPendingExpKeys (or call the existing helper used by
PumpFleetDispatch if one exists), then continue draining and log the cleared
queue as before.
| const vertex_ai = new VertexAI({project: 'project-263b6139-6893-4788-995', location: location}); | ||
| const generativeModel = vertex_ai.preview.getGenerativeModel({ | ||
| model: modelName, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the current Vertex AI Gemini model names and identifiers?
💡 Result:
Current Vertex AI Gemini model IDs (publisher: Google)
Use these in Vertex AI as either the model ID (e.g., gemini-2.5-pro) or the full resource name projects/{PROJECT}/locations/{LOCATION}/publishers/google/models/{MODEL_ID}. [1]
Text / multimodal (general)
- Gemini 2.5 Pro —
gemini-2.5-pro[2] - Gemini 2.5 Flash —
gemini-2.5-flash(preview variant also shown in docs:gemini-2.5-flash-preview-09-2025) [3] - Gemini 2.5 Flash-Lite —
gemini-2.5-flash-lite(preview variant also shown in docs:gemini-2.5-flash-lite-preview-09-2025) [4] - Gemini 2.0 Flash —
gemini-2.0-flash[5] - Gemini 2.0 Flash-Lite —
gemini-2.0-flash-lite[6]
Live / streaming audio (Gemini Live API)
- Gemini 2.5 Flash (Live, native audio) —
gemini-live-2.5-flash-native-audio(preview variant also shown:gemini-live-2.5-flash-preview-native-audio-09-2025) [7] - Gemini 2.0 Flash Live (preview) —
gemini-2.0-flash-live-preview-04-09[5]
Image generation (Gemini)
- Gemini 2.5 Flash Image —
gemini-2.5-flash-image[8] - Gemini 3.1 Flash Image (preview) —
gemini-3.1-flash-image-preview[9] - Gemini 3 Pro Image (preview) —
gemini-3-pro-image-preview[10]
Gemini 3 (preview)
- Gemini 3.1 Pro (preview) —
gemini-3.1-pro-preview[11] - Gemini 3 Pro (preview) —
gemini-3-pro-preview[12] - Gemini 3 Flash (preview) —
gemini-3-flash-preview[13] - Gemini 3.1 Flash-Lite (preview) —
gemini-3.1-flash-lite-preview[14]
Note on “versioned” identifiers (often required for Provisioned Throughput)
Provisioned Throughput requires specific version IDs (example given by Google: gemini-2.0-flash-001, gemini-2.0-flash-lite-001). [15]
Sources: Google Cloud Vertex AI Gemini model docs and supported-models list (last updated 2026-03-06 UTC). [15]
🏁 Script executed:
git ls-files | grep -E "test-vertex|vertex.*test" | head -20Repository: mkalhitti-cloud/universal-or-strategy
Length of output: 95
🏁 Script executed:
cat -n test-vertex.jsRepository: mkalhitti-cloud/universal-or-strategy
Length of output: 1315
Replace 'gemini-3.1-pro' with a valid Vertex AI model identifier.
The model 'gemini-3.1-pro' (line 24) does not exist in current Vertex AI documentation. According to current Vertex AI APIs, only preview versions of Gemini 3 models are available, such as 'gemini-3.1-pro-preview' (which you already have on line 23), 'gemini-3-pro-preview', or 'gemini-3-flash-preview'. Either remove the invalid model or replace it with a valid alternative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test-vertex.js` around lines 5 - 8, The code is using an invalid Vertex AI
model identifier; update the model passed to VertexAI.preview.getGenerativeModel
(the call that creates generativeModel) to a valid preview model name (e.g.,
replace 'gemini-3.1-pro' with 'gemini-3.1-pro-preview' or another supported
preview name like 'gemini-3-pro-preview' or 'gemini-3-flash-preview'), or remove
the invalid model reference entirely so modelName in test-vertex.js refers only
to a supported model.
mkalhitti-cloud
left a comment
mkalhitti-cloud
left a comment
There was a problem hiding this comment.
🛡️ Zero-Trust Forensic Audit: Ghost State Remediation (PR #31)
Audit Status: ✅ APPROVED (Passes all criteria)
Branch: build/956-audit-remediation
Target Files: Orders.Callbacks.cs, Orders.Management.cs
1. Deadlock Prevention: ✅ PASS
- Scanned all new
lock(stateLock)blocks introduced in the PR. - Verification: Absolutely NO broker API calls (
CancelOrder,SubmitOrder,ChangeOrder,SubmitOrderUnmanaged) or UI thread calls (Print) exist inside anylock(stateLock)block. The separation of state mutation from broker execution is strictly maintained.
2. Atomic Integrity: ✅ PASS
- Verified all dictionary mutations across both files.
- Verification: All
TryRemovecalls foractivePositions,entryOrders,stopOrders, and auxiliary dictionaries (_followerReplaceSpecs,_citNudgedKeys, target dictionaries) are now safely wrapped inlock(stateLock). - Reassignments (e.g.,
stopOrders[entryName] = stopOrder;) and property updates for quantities are also robustly guarded.
3. The Staircase Rule Check: ✅ PASS
- Examined
HandleEntryOrderFilledinOrders.Callbacks.cs. - Verification: Properties on
posare safely mutated under the lock, and the lock is correctly released before the call toApplyTargetLadderGuard(pos). This successfully avoids any nested deadlock risks while satisfying the staircase rule.
Conclusion: The PR successfully remediates the ghost state vulnerabilities and adheres perfectly to the GEMINI.md Zero-Trust locking protocols.
Groups A-H surgical fixes: - A: stateLock guards for PendingCleanup/FlattenAttemptCount writes (Callbacks, Trailing) - B1-B5: null-submit rollback completeness (RMA exception path, RMA/Trend split E1 abort, FFMA premature IPC notify, follower stop null guard) - C1: Two-phase FSM for follower target replace in Trailing (_followerTargetReplaceSpecs dict, SubmitFollowerTargetReplacement, OnAccountOrderUpdate hook) - D1: stopOrders.TryRemove gated on remainingAfterStop <= 0 (terminal only) - D2: _dispatchSyncPendingExpKeys.TryRemove on confirmed-cancel to unblock SIMA barrier - D3: Removed premature targetDict.TryRemove after async CancelOrder - E1: _repairInFlight cleared in try/catch/finally around TriggerCustomEvent - F1-F2: SIMA drain loops (abort + disable) now roll back ReservedDelta and clear dispatch-sync barrier - G: Artifact files deleted, .gitignore patterns added - H1: Antigravity audit gate restored in standards_manifesto.md - H2: Fixed duplicate-word typo in .cursorrules and copilot-instructions.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Superseded by #32 (Build 957: Audit Remediation Loop 1) which addresses all remaining findings from Antigravity, CodeRabbit, and DeepSource audits. Closing this PR. |
Groups A-H surgical fixes: - A: stateLock guards for PendingCleanup/FlattenAttemptCount writes (Callbacks, Trailing) - B1-B5: null-submit rollback completeness (RMA exception path, RMA/Trend split E1 abort, FFMA premature IPC notify, follower stop null guard) - C1: Two-phase FSM for follower target replace in Trailing (_followerTargetReplaceSpecs dict, SubmitFollowerTargetReplacement, OnAccountOrderUpdate hook) - D1: stopOrders.TryRemove gated on remainingAfterStop <= 0 (terminal only) - D2: _dispatchSyncPendingExpKeys.TryRemove on confirmed-cancel to unblock SIMA barrier - D3: Removed premature targetDict.TryRemove after async CancelOrder - E1: _repairInFlight cleared in try/catch/finally around TriggerCustomEvent - F1-F2: SIMA drain loops (abort + disable) now roll back ReservedDelta and clear dispatch-sync barrier - G: Artifact files deleted, .gitignore patterns added - H1: Antigravity audit gate restored in standards_manifesto.md - H2: Fixed duplicate-word typo in .cursorrules and copilot-instructions.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Supersedes PR #30. This PR contains the fresh implementation of the audit findings. Triggering a clean bot sweep.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores