Repository navigation
fix: follow-ups from the cleanup — stale comments, dead deploy paths, 404 masking - #2
Merged
Merged
Conversation
v2/index.ts said "Default disabled (MOMENTUM_CONFIG.ENABLED=false) — flip to true when ready to ship live", but config.ts:257 has ENABLED: true and has since 2026-05-18. Momentum has been trading while the comment said it was off. Also reconciles the two dates the file carried (rebuilt 2026-05-06, enabled 2026-05-18). core/arbitrageEngine.ts: `scan()` is retained as a reference implementation for the eventual rewrite, but being `private` made tsc flag it as unused, which had been silenced with `void this.scan;`. Making it `protected` removes the error honestly — tsc does not unused-check protected members — so the fake reference is gone and the method is no longer bait for deletion. types/services.d.ts: document that the wildcard specifier form can match an unintended module ending in the same suffix, and why that risk was accepted. Comments and member visibility only. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX
deploy.ps1: docker mode built from canuck-trader-pro/backend/Dockerfile, which was deleted along with the Python backend; the docker-compose.yml bot service it targeted is also gone. Removed the mode entirely (ValidateSet, $DockerImage/$DockerImageFile vars, rollback branch, deploy block, help text) — archive and git modes untouched. setup-vps.sh: removed the unreachable 'elif systemctl is-active --quiet trading-bot' branch in the post-receive hook. No systemd unit named trading-bot is created anywhere in the script (inline unit generation was removed with the Python stack), so this branch could never fire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX
…tall step setup-vps.sh: - Removed help text pointing at deploy.ps1 -Mode docker (deleted last commit) and the 'Or with Docker: docker-compose up -d / logs -f bot' hint (the bot compose service was removed with the Python backend). docker-compose.yml itself is untouched. - Removed the Python 3.12 install step: no .py files remain tracked in the repo, so nothing installs or uses it. Removed PYTHON_VERSION and PYTHON_BIN vars and the Python summary line. - Renumbered the provisioning steps 1-10/10 (previously 1-8,10-12/12, with step 9 already missing before this change) so printed step counts are contiguous and correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX
The catch-all `app.get('*')` answered every unmatched path with 200 +
dist/index.html, including /api/*. Callers expecting JSON got HTML, res.json()
threw, and component catch blocks swallowed it — so a missing API route was
indistinguishable from a working one. 66 of the 79 distinct /api endpoints the
frontend calls are not served by this V2 server; all of them looked like 200 OK.
Adds an /api guard ahead of the catch-all returning {error, path} with 404.
Verified by smoke test: /api/health, /api/v2/status and /api/config/flags still
200; /api/sentiment/dashboard now 404 JSON; /monitor and SPA routes unaffected.
Also fixes deploy/setup-vps.sh:312, which started the bot with
`pm2 start server.js` — server.js was deleted. Now starts from
ecosystem.config.cjs, which carries the --experimental-strip-types flag the
TypeScript entrypoint requires, plus the env and restart policy.
No trading behaviour change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX
manijose1919
pushed a commit
that referenced
this pull request
Jul 29, 2026
Records PR #2. Notes the finding that 66 of 79 frontend /api endpoints are unserved because routes/ is never mounted — flagged as needing a decision, not fixed. Stats baseline reset: no. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the follow-ups logged during the repository cleanup, plus one bug found while investigating them.
The significant one:
/api/*was masking missing routesserverV2.ts's catch-allapp.get('*')answered every unmatched path with200+dist/index.html— including/api/*. Callers expecting JSON received HTML,res.json()threw, and componentcatchblocks swallowed it. A missing API route was indistinguishable from a working one.66 of the 79 distinct
/apiendpoints the frontend calls are not served by this V2 server (the entireroutes/directory, 17 files, is never mounted). All of them looked like200 OK.This PR does not implement the missing endpoints — that is a separate decision. It makes the problem visible: unmatched
/api/*now returns404 {"error":"Not found","path":"..."}.Smoke-tested:
/api/health,/api/v2/status,/api/config/flagsstill200;/api/sentiment/dashboardnow404JSON;/monitorand SPA routes unaffected. The guard is registered after every real API route and before the catch-all, so nothing is shadowed.The rest
v2/index.ts— the comment claimed momentum was disabled (ENABLED=false, "flip to true when ready to ship live") whileconfig.ts:257has hadENABLED: truesince 2026-05-18. Momentum has been trading while the code said it was off. Backtest figures preserved.core/arbitrageEngine.ts—scan()is kept as a reference implementation, but beingprivatemade tsc flag it unused, which had been silenced withvoid this.scan;. Nowprotected: the error goes away honestly and the method is no longer bait for deletion. Engine remains a no-op.deploy/deploy.ps1— removed thedockermode; it builtcanuck-trader-pro/backend/Dockerfile, deleted in the cleanup.archiveandgitmodes untouched.deploy/setup-vps.sh— removed an unreachablesystemctlbranch, stale docker help text, and a Python 3.12 install step for a stack that no longer exists; provisioning steps renumbered to a contiguous 1-10 (they were already mislabelled, skipping 9). Fixedpm2 start server.js→pm2 start ecosystem.config.cjs, sinceserver.jswas deleted and the ecosystem file carries the--experimental-strip-typesflag the TypeScript entrypoint needs.types/services.d.ts— documented the accepted over-match risk of the wildcard specifier form.No trading behaviour change. No stats baseline reset.
Known, not fixed
The 66 unimplemented endpoints. Options are to mount
routes/(integration work — those files were written for the deletedserver.js) or to trim the dashboard to what V2 actually serves.🤖 Generated with Claude Code
https://claude.ai/code/session_01B5r4UPkQVsGY2DPB5ak1LX