Skip to content

Feat/currency converter - #3

Merged
theprogrammersingh merged 3 commits into
mainfrom
feat/currency-converter
Sep 3, 2026
Merged

Feat/currency converter#3
theprogrammersingh merged 3 commits into
mainfrom
feat/currency-converter

Conversation

@theprogrammersingh

@theprogrammersingh theprogrammersingh commented Sep 3, 2026

Copy link
Copy Markdown
Owner

View code changes stack in Autter

Summary

Summary generated by Autter.
Adds a cross-origin currency-converter experience that can be embedded across the dashboard, expenses, agent tools, and conversion routes. The converter exposes a WebMCP tool that the Copilot can discover and invoke, while ConverterSession manages shared iframe lifecycle and remote-tool cleanup. The PR also corrects multi-page expense aggregation for dashboards and reports, updates runtime/deployment configuration and documentation, removes the standalone partner demo server, and expands frontend regression coverage.

Changes

  • Added CurrencyConverter and ConverterSession implementations with tests for conversion behavior, iframe mounting, reference-counted lifecycle management, handshake configuration, discovery, and teardown.
  • Added the /convert route and embedded converter surfaces on the dashboard, expenses, settings, and agent tools pages.
  • Connected cross-origin WebMCP discovery to the Copilot and agent tools page, including cleanup when converter surfaces are released.
  • Updated Copilot handling and UI tests for remote tools, tool formatting, and cross-origin invocation behavior.
  • Changed dashboard and report expense retrieval to page through all available results instead of relying on a single capped page.
  • Added expense-page behavior and tests for the updated converter and expense workflows.
  • Updated environment/configuration handling for BASE_CURRENCY and CONVERTER_URL, including development defaults and production opt-in behavior.
  • Removed the legacy partner demo HTML and server scripts, and updated deployment, SEO stamping, SSR, Docker, App Hosting, README, PRD, and design documentation accordingly.
  • Expanded regression coverage across dashboard, settings, agent, Copilot, expenses, configuration, routing, and UI components.

Acceptance Criteria

  • The /convert route renders the currency converter and can obtain conversion results without sending the user's Gemini API key to Actuo.
  • Dashboard, expenses, and agent surfaces can mount the converter without duplicate frames, and the shared session releases the frame and clears remote Copilot tools after the final surface is removed.
  • The agent tools page reports WebMCP and cross-origin support accurately, and the Copilot only offers discovered converter tools while a converter surface is active.
  • Currency conversion requests preserve the configured converter URL and append the Actuo handshake correctly, including URLs containing query parameters or fragments.
  • Dashboard totals and generated reports include expenses beyond EXPENSE_PAGE_MAX by fetching all pages.
  • Production uses an explicitly configured CONVERTER_URL; development retains the documented fallback behavior.
  • The shared build, backend and frontend typechecks, unit tests, backend e2e tests, and full SSR/SEO build complete successfully.

Test Plan

  • Run the repository CI-equivalent install, shared build, backend/frontend typechecks, unit tests, backend e2e tests, and full SSR/SEO build using the documented npm workspace commands.
  • Run the Angular frontend tests through the Angular ng test builder and verify the new converter, session, dashboard, expenses, agent, Copilot, and settings suites pass.
  • Open /convert, /dashboard, /expenses, and /agent in a browser and verify the converter loads, displays rates, and does not alter Actuo expense totals.
  • In a browser with WebMCP cross-origin testing enabled, verify the agent page discovers convertCurrency, the Copilot can invoke it, and the tool disappears after the last converter surface is closed.
  • Configure a converter URL containing both query parameters and a hash fragment, reload the embedded converter, and verify the handshake is received and discovery succeeds.
  • Use an organization with more than EXPENSE_PAGE_MAX expenses and verify dashboard totals and generated CSV reports include all matching rows.
  • Verify the legacy partner-demo assets are no longer required by the build or deployment output.

Rollback Plan

  • Revert this commit to remove the converter route, embedded converter/session integration, remote-tool discovery changes, pagination updates, and associated deployment/configuration changes.
  • If only the cross-origin feature is problematic, unset CONVERTER_URL in production and redeploy; the converter surfaces should remain unavailable without affecting local Copilot tool execution or expense workflows.
  • If pagination or report generation causes regressions, temporarily restore the previous single-page retrieval behavior while retaining the converter changes, then redeploy after isolating the affected service.
  • Restore the deleted partner demo assets from the preceding revision only if an existing deployment or documentation workflow still depends on them.

Related Issues

No linked issue was identified in the supplied branch, commit, or diff context.

Written for commit 229716f. Summary will update on new commits.

Actuo has no FX pass, so `converted_amount` is null for every foreign row
and totals count base-currency rows only, stating what they left out. That
gap is honest but unhelpful: there was nowhere in the app to find out what
an excluded row is actually worth.

A separate converter app is now framed on four surfaces — `/convert`, the
`/agent` panel, beside the dashboard's excluded-rows notice, and on expense
rows filed in another currency. It is a reference a person reads. Nothing it
shows is written to `converted_amount`, folded into `sumSpend()`, or allowed
to change the excluded-rows copy.

That boundary is structural, not a promise. `CurrencyConverter` has no
`output()`, no `postMessage` listener, and never reads a value back out of
the frame, so no converted figure exists in the component tree to be wired
in; adding one would mean first inventing a return channel. The specs assert
the component's inputs and outputs directly, and the dashboard and expenses
specs assert that opening the lookup moves no figure. `core/expense/amount.ts`
and its spec are untouched, which is the point.

`PARTNER_DEMO_ORIGIN` becomes `CONVERTER_URL`, and is now a full URL rather
than a bare origin: the production converter serves at `/` while the local
partner demo serves at `/partner-demo/`, so one value covers both and
consumers take the origin with `new URL(...).origin`. A second path variable
that had to stay in step would be one too many. Non-http(s) values are
rejected before the sanitizer bypass that renders the frame.

Discovery moves out of `/agent` into `ConverterSession`. Page-owned teardown
was correct while one page framed one partner; with four surfaces it cleared
the Copilot's remote tools while a frame was still mounted elsewhere. The
service enforces two rules. Only one frame may be open at a time, because
`getTools()` returns a descriptor per *window* and two live frames publish
two tools of the same name. And discovery is reference counted, because
Angular constructs the incoming component before destroying the outgoing one,
so clear-on-destroy would wipe what the new surface just found.

Three things this turned up on the way:

- The expenses page renders the desktop table and the phone card list into
  the DOM at once and lets CSS pick, so a panel inside the row markup mounted
  TWICE for one open row — two iframes, two loads of a whole separate app.
  Confirmed in the browser before fixing. There is now one converter for the
  page, driven by whichever row is open.
- `Copilot.discoverRemoteTools()` did not dedupe by name. The same page framed
  here and also open in another tab would hand Gemini two identical function
  declarations, which is malformed rather than merely redundant. This was
  already reachable with the partner demo; the fix is not specific to the
  converter.
- The paging specs queued mock responses by call order, so any new request
  from this screen displaced them. They now route by path, as the dashboard
  spec already did.

Verified with the stack running, not only by unit test: one frame across all
four surfaces and when switching rows, the `?actuo=` handshake present on the
frame src, both partner tools discovered cross-origin with `readOnlyHint`,
`executeTool()` returning a real result across origins (`inputSchema` arrived
as a string and the result as a string, both documented quirks), and the
expenses money column unchanged at $200 / $45 / rupee rows through opening
and switching the lookup. Both themes checked.

Gate: typechecks clean, 892 tests green (9 shared, 65 backend, 784 frontend,
34 e2e), build clean with 11 routes prerendered.

Not yet verified: the deployed converter. That needs its `exposedTo` change
released and `CONVERTER_URL` set on the service; until then the surfaces show
their honest "no tools discovered from that origin yet" state.
…verywhere

The Pageturner Books page and its :4201 static server existed for one
reason: to give the cross-origin path a second origin before there was a
real one. There is now. The converter's `exposedTo` change is merged and
live, so a synthetic stand-in is duplication — and worse than duplication,
because it was only ever true on localhost. Dev exercised one thing and
production another, which is how a path stays broken in one of them without
anyone noticing.

So `CONVERTER_URL` defaults to the deployed converter in development too,
and dev and production now run the same code against the same origin. Gone:
`frontend/public/partner-demo/`, `scripts/partner-server.mjs`, the third
`pnpm run dev` pane, the express.static mount in `frontend/src/server.ts`,
the `!/partner-demo/**` service-worker exclusion, and the robots Allow rule.
`pnpm run dev` is two panes and nothing binds :4201.

Production is deliberately still not defaulted. A deploy should name the
converter it trusts rather than inherit one; unset, the surfaces say so.
That is a different reason from the old one (a localhost default would have
pointed at each visitor's own machine), so the comments that gave the old
reason are rewritten rather than moved.

`CONVERTER_URL` stays a full URL, but the justification for it changed and
had to be replaced rather than left standing: the old one was that the two
targets sat at different paths, and there are no longer two targets. What
survives is that a converter need not sit at the root of its host — a GitHub
Pages project site is `<user>.github.io/<repo>/` — and the `?actuo=`
handshake is appended to it regardless. The spec that pinned the old reason
now pins the new one instead of being deleted.

Running it turned up a real bug the suite could not see. Going offline
released the frame, which tore the session down, which cleared the open
surface — so the panel vanished instead of rendering "live rates need a
connection", and coming back online left it closed because nothing was
asking for it any more. `openSurface` is the user's intent and the mount
count is a resource; coupling them was the mistake. Two specs now cover the
transition, not just the initial state, which is what let it through.

Verified with the stack running, in Chrome 151 with the flag, against the
deployed converter framed from localhost:4200 — a genuine origin boundary:
all seven of its tools discovered, badges correct (four read-only, three
mutating, matching the split shipped on the other side),
`typeof inputSchema === 'string'` and `executeTool()` resolving to a string
(both documented quirks, re-measured here rather than assumed),
`convertCurrency({amount:200,from:'EUR',to:'INR'})` returning
`200 EUR = 22,018.00 INR (1 EUR = 110.09 INR, 2 Sep 2026)` with the embedded
widget moving to match. One frame across all four surfaces and when
switching rows. Money untouched throughout: `$200 / $45 / ₹840 / ₹6,450` and
the dashboard total and excluded-rows notice byte-identical through opening
and switching. Offline drops the frame, keeps the panel, explains itself,
and restores on reconnect.

The PRD §7 claim is reworded rather than quietly inherited. It used to rest
on a "deliberately unrelated site" that this repo authored; it now rests on
a separately built, independently deployed app we do not own, working over a
real origin boundary in dev as well as on a deploy. That is better evidence,
and unlike the old page it survives a deploy.

Also corrects three things the previous commit left stale: CLAUDE.md still
named `EnvService.partnerOrigin` (renamed to `converterUrl`), still cited
`PARTNER_DEMO_ORIGIN`, and claimed Chrome 152 where this machine reports
151. `/convert` was missing from robots.txt and is now disallowed with the
other authenticated routes.

Gate: typechecks clean, 892 tests green (9 shared, 65 backend, 786 frontend,
34 e2e), build clean with 11 routes prerendered.

Still not verified: the path from a *deployed* Actuo. That needs
CONVERTER_URL set on the Render service.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 1 finding(s) as review threads below (🔴 1). Each carries a copy-paste AI fix prompt.

* makes that bypass defensible.
*/
it('refuses a URL whose scheme is not http or https', async () => {
for (const bad of ['javascript:alert(1)', 'data:text/html,x', 'file:///etc/passwd']) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 [deterministic] PII stored without application-level encryption — Risk: 85/100

A sensitive PII column/field (SSN, payment token, secret, API key, password, bank account, etc.) is declared without application-level encryption. Blast radius — exposing this data cascades to the downstream consumers that depend on this code: dependent files frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, backend/src/config/env.service.spec.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/pages/dashboard/dashboard.spec.ts.

References:

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, backend/src/config/env.service.spec.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/pages/dashboard/dashboard.spec.ts
🛠 AI fix prompt (copy & paste into your coding agent)
In `frontend/src/app/converter/converter-session.spec.ts` around line 84, a sensitive column/field (SSN, payment token, credit card, secret, API key, password, bank account, etc.) is declared with a plain-text type and no application-level encryption — OR is paired with a broken/weak primitive (MD5/SHA1, low bcrypt cost, low PBKDF2 iterations, DES/RC4/ECB, Math.random for secrets). Store it in an encrypted column (pgp_sym_encrypt / app-level AES-256-GCM via KMS / Vault transit / a dedicated encrypted-field type) and protect passwords with a strong KDF (bcrypt cost ≥ 12, Argon2id with timeCost ≥ 3 and memoryCost ≥ 65536, or PBKDF2-SHA256 with ≥ 600k iterations). Keep the plaintext out of logs, snapshots, and analytics. Blast radius — exposing this data cascades to the downstream consumers that depend on this code: dependent files `frontend/src/app/pages/agent/agent.ts`, `frontend/src/app/ui/showcase/showcase.ts`, `backend/src/config/config.controller.ts`, `frontend/src/app/app.routes.ts`, `backend/src/config/env.service.spec.ts`, `frontend/src/app/copilot/copilot.spec.ts`, `frontend/src/app/pages/agent/agent.spec.ts`, `frontend/src/app/pages/dashboard/dashboard.spec.ts`.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 2 finding(s) as review threads below (🟠 1 · 🟡 1). Each carries a copy-paste AI fix prompt.

import { formatDate, formatMoney } from '../../core/format/money.js';
import { expenseAmount, expenseCurrency } from '../../core/expense/amount.js';
import { expenseAmount, expenseCurrency, isConverted } from '../../core/expense/amount.js';
import { CurrencyConverter } from '../../converter/currency-converter.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Currency converter is eagerly included in the expenses route chunk — Risk: 60/100

Because CurrencyConverter is statically imported and declared in Expenses.imports, the converter implementation is bundled when the expenses route loads, although the panel is rendered only after a user opens it. Consider lazy-loading or otherwise splitting the converter if keeping it out of the route's initial chunk is important.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: describeFailure, Expenses.run, Expenses.remove, Expenses.disarmDelete, Expenses, ClientConfig, EnvService, Showcase
  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, backend/src/config/env.service.spec.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/pages/dashboard/dashboard.spec.ts
  • Scopes: frontend
🛠 AI fix prompt (copy & paste into your coding agent)
Wrap the converter panel in Angular @defer (on interaction), or dynamically load a standalone converter component, and remove the eager CurrencyConverter import from the expenses page's component imports.

Flagged by Autter security & observability checks.


import { ApiClient } from '../../core/api/api-client.js';
import { Card, EmptyState, ErrorState, Skeleton, StatCard } from '../../ui';
import { CurrencyConverter } from '../../converter/currency-converter.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [ai] Currency converter is eagerly included in the dashboard route chunk — Risk: 45/100

The static CurrencyConverter and ConverterSession imports make the converter code part of the dashboard component's bundle even though the lookup UI is conditional and only relevant after the user chooses it. Consider deferring the converter implementation if dashboard bundle size is important. This affects the dashboard route chunk, not necessarily the application's global initial chunk.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: Dashboard, ClientConfig, EnvService, Showcase, Settings, Agent, ConfigController, Copilot
  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, backend/src/config/env.service.spec.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/pages/dashboard/dashboard.spec.ts
  • Scopes: frontend
🛠 AI fix prompt (copy & paste into your coding agent)
Use Angular @defer (on interaction) or a dynamically loaded standalone converter component so CurrencyConverter and its implementation are split from the dashboard initial bundle; keep only the minimal trigger/session wiring eagerly loaded.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown

🚦 Pre-merge checks · ⚠️ 19 warning, ✅ 158 passed

Needs attention

Check Status Explanation
Too many lines changed ⚠️ Warning This PR changes 2694 lines (limit 1000). Splitting into smaller, focused PRs makes review more reliable.
Mixed concerns (refactor + behavior change) ⚠️ Warning The PR combines new converter and expense-aggregation behavior with retirement of the legacy partner demo and related no-behavior cleanup. Split legacy demo/runtime cleanup into a separate PR from converter and aggregation behavior. Review high-fan-in changes such as backend/src/config/env.service.ts (fanIn 9) and fron ... [truncated 94 chars]
Log injection ⚠️ Warning 1 potential issue(s) detected (max risk 45/100): scripts/verify-deploy.mjs:55.
Removed observability ⚠️ Warning 1 potential issue(s) detected (max risk 45/100): scripts/partner-server.mjs:67.
Possible non-atomic read-modify-write ⚠️ Warning 1 potential issue(s) detected (max risk 45/100): scripts/verify-deploy.mjs:44.
Optimistic locking not detected ⚠️ Warning 1 potential issue(s) detected (max risk 40/100): scripts/verify-deploy.mjs:102.
Infrastructure misconfiguration ⚠️ Warning 1 potential issue(s) detected (max risk 60/100): Dockerfile:77.
PII stored without application-level encryption ⚠️ Warning 1 potential issue(s) detected (max risk 85/100): frontend/src/app/converter/converter-session.spec.ts:84.
Missing linked tracker issue ⚠️ Warning 1 potential issue(s) detected (max risk 55/100): backend/src/config/env.service.ts:99.
Missing CODEOWNERS reviewer approval ⚠️ Warning 1 potential issue(s) detected (max risk 70/100): backend/src/config/env.service.ts:99.
Direct env-var access bypasses config module ⚠️ Warning 1 potential issue(s) detected (max risk 50/100): scripts/verify-deploy.mjs:28.
Comment contradicts or fabricates code behaviour ⚠️ Warning 2 potential issue(s) detected (max risk 60/100): backend/src/config/env.service.ts:92, backend/.env.example:23.
Code style differs from rest of codebase ⚠️ Warning 1 potential issue(s) detected (max risk 55/100): backend/src/config/env.service.ts:102.
Established pattern ignored ⚠️ Warning 1 potential issue(s) detected (max risk 60/100): frontend/src/app/pages/dashboard/dashboard.ts:15.
Runtime error risk ⚠️ Warning 1 finding(s) on changed lines.
Resource leak risk ⚠️ Warning 2 finding(s) on changed lines.
Code duplication / DRY violation ⚠️ Warning 1 finding(s) on changed lines.
Complexity Guard ⚠️ Warning 1 finding(s) on changed lines.
Bundle Size Monitor ⚠️ Warning 1 finding(s) on changed lines.
✅ Passed checks (158)
Check Status Explanation
Too many files changed ✅ Passed Changed 46 file(s), within the limit of 50.
Too many unrelated chapters ✅ Passed 6 chapter(s) detected, within the limit of 6.
Generated files hiding real changes ✅ Passed Generated-file volume (0 lines) does not obscure the 2694 hand-written line(s).
Missing PR context ✅ Passed PR context looks sufficient.
Migration + app logic + UI combined in one PR ✅ Passed No database migration files are present; the PR contains application and UI changes but no migration.
Sensitive data in logs ✅ Passed No sensitive data in logs issues detected.
Missing audit logging ✅ Passed No missing audit logging issues detected.
Silent exception swallowing ✅ Passed No silent exception swallowing issues detected.
Unhandled promise rejection ✅ Passed No unhandled promise rejection issues detected.
Circuit breaker not detected ✅ Passed No circuit breaker not detected issues detected.
Stack trace leakage ✅ Passed No stack trace leakage issues detected.
Multi-write without detected transaction ✅ Passed No multi-write without detected transaction issues detected.
Possible TOCTOU in critical path ✅ Passed No possible toctou in critical path issues detected.
Idempotency key not detected ✅ Passed No idempotency key not detected issues detected.
Rate limiting not detected ✅ Passed No rate limiting not detected issues detected.
Rate limiting removed ✅ Passed No rate limiting removed issues detected.
Batch size limit not detected ✅ Passed No batch size limit not detected issues detected.
Pagination not detected ✅ Passed No pagination not detected issues detected.
Publicly exposed storage ✅ Passed No publicly exposed storage issues detected.
Over-permissive IAM policy ✅ Passed No over-permissive iam policy issues detected.
Security group open to the internet ✅ Passed No security group open to the internet issues detected.
Unencrypted storage at rest ✅ Passed No unencrypted storage at rest issues detected.
Infrastructure missing access logging ✅ Passed No infrastructure missing access logging issues detected.
Hardcoded secret in IaC ✅ Passed No hardcoded secret in iac issues detected.
Deprecated Kubernetes API version ✅ Passed No deprecated kubernetes api version issues detected.
Compound IaC attack chain ✅ Passed No compound iac attack chain issues detected.
Prompt injection risk ✅ Passed No LLM/AI-integration code touched by this diff.
LLM output used in a dangerous sink ✅ Passed No LLM/AI-integration code touched by this diff.
Sensitive data in prompt or system-prompt leakage ✅ Passed No LLM/AI-integration code touched by this diff.
Over-privileged LLM tool / excessive agency ✅ Passed No LLM/AI-integration code touched by this diff.
Missing validation on an LLM-driven decision ✅ Passed No LLM/AI-integration code touched by this diff.
Unbounded LLM usage (denial-of-wallet) ✅ Passed No LLM/AI-integration code touched by this diff.
Table exposed without row-level security ✅ Passed No row-level-security-related code touched by this diff.
Over-broad row-level security policy ✅ Passed No row-level-security-related code touched by this diff.
Code path that bypasses row-level security ✅ Passed No row-level-security-related code touched by this diff.
Privileged database credential reachable from the client ✅ Passed No row-level-security-related code touched by this diff.
Privileged query without row-level scoping ✅ Passed No row-level-security-related code touched by this diff.
Template-default gradient styling ✅ Passed No added frontend pages or design-slop markers in this diff.
Interchangeable AI marketing copy ✅ Passed No added frontend pages or design-slop markers in this diff.
Placeholder content shipped to users ✅ Passed No added frontend pages or design-slop markers in this diff.
Emoji standing in for an icon system ✅ Passed No added frontend pages or design-slop markers in this diff.
Call-to-action that goes nowhere ✅ Passed No added frontend pages or design-slop markers in this diff.
Templated page composition ✅ Passed No added frontend pages or design-slop markers in this diff.
Merge-blocking marker left in the change ✅ Passed No pending-work markers added by this diff.
Known-defect marker shipped in code ✅ Passed No pending-work markers added by this diff.
Untracked TODO without an issue reference ✅ Passed No pending-work markers added by this diff.
Test disabled or left pending ✅ Passed No pending-work markers added by this diff.
PII in logs ✅ Passed No pii in logs issues detected.
PII or internals leaked in error response ✅ Passed No pii or internals leaked in error response issues detected.
User data stored without retention controls ✅ Passed No user data stored without retention controls issues detected.
PII sent to external / cross-border destination ✅ Passed No pii sent to external / cross-border destination issues detected.
Lockfile resolution / integrity tampered ✅ Passed No lockfile resolution / integrity tampered issues detected.
Dependency runs install-time lifecycle script ✅ Passed No dependency runs install-time lifecycle script issues detected.
Possible dependency-confusion attack ✅ Passed No possible dependency-confusion attack issues detected.
Lockfile resolves a dependency the manifest does not declare ✅ Passed No lockfile resolves a dependency the manifest does not declare issues detected.
Checked-in build artefact modified without source change ✅ Passed No checked-in build artefact modified without source change issues detected.
Dockerfile build-step is insecure ✅ Passed No dockerfile build-step is insecure issues detected.
External artefact pulled in without integrity pinning ✅ Passed No external artefact pulled in without integrity pinning issues detected.
Changed export, importer not updated ✅ Passed No changed export with an un-updated importer detected.
Missing security-team review on sensitive path ✅ Passed No missing security-team review on sensitive path issues detected.
Source changes without matching tests ✅ Passed No source changes without matching tests issues detected.
Migration missing rollback / down step ✅ Passed No migration missing rollback / down step issues detected.
Frontend importing database client directly ✅ Passed No frontend importing database client directly issues detected.
Route handler bypassing service layer ✅ Passed No route handler bypassing service layer issues detected.
Backend service importing UI module ✅ Passed No backend service importing ui module issues detected.
Cross-context internals import ✅ Passed No cross-context internals import issues detected.
Workspace package rule violation ✅ Passed No workspace package rule violation issues detected.
Inconsistent logging pattern ✅ Passed No inconsistent logging pattern issues detected.
Inconsistent error handling ✅ Passed No inconsistent error handling issues detected.
Endpoint missing input validation ✅ Passed No endpoint missing input validation issues detected.
Multi-write without transaction wrapper ✅ Passed No multi-write without transaction wrapper issues detected.
New feature shipped without feature flag ✅ Passed No new feature shipped without feature flag issues detected.
Module placed in the wrong workspace package ✅ Passed No module placed in the wrong workspace package issues detected.
Hallucinated import (package not installed) ✅ Passed No hallucinated import (package not installed) issues detected.
Nonexistent package (not found in registry) ✅ Passed No nonexistent package (not found in registry) issues detected.
Call to function that does not exist ✅ Passed No call to function that does not exist issues detected.
Generic placeholder identifier in production logic ✅ Passed No generic placeholder identifier in production logic issues detected.
Repetitive boilerplate (duplicated block) ✅ Passed No repetitive boilerplate (duplicated block) issues detected.
Overbroad try/catch swallowing all exceptions ✅ Passed No overbroad try/catch swallowing all exceptions issues detected.
TODO / FIXME on critical path ✅ Passed No todo / fixme on critical path issues detected.
Abstraction defined but never used ✅ Passed No abstraction defined but never used issues detected.
Unhandled edge case (null / empty / zero / boundary) ✅ Passed No unhandled edge case (null / empty / zero / boundary) issues detected.
Doc-copy code with insecure defaults ✅ Passed No doc-copy code with insecure defaults issues detected.
Dead code (defined but never referenced) ✅ Passed No dead code (defined but never referenced) issues detected.
Deprecated API call ✅ Passed No deprecated api call issues detected.
API pattern from wrong library version ✅ Passed No api pattern from wrong library version issues detected.
API endpoint removed ✅ Passed No api endpoint removed issues detected.
HTTP method changed (GET ↔ POST etc.) ✅ Passed No http method changed (get ↔ post etc.) issues detected.
New required field added to request ✅ Passed No new required field added to request issues detected.
Field removed from response schema ✅ Passed No field removed from response schema issues detected.
Response field type changed ✅ Passed No response field type changed issues detected.
HTTP status code changed ✅ Passed No http status code changed issues detected.
Auth requirement added / removed / changed ✅ Passed No auth requirement added / removed / changed issues detected.
Error response shape changed ✅ Passed No error response shape changed issues detected.
Pagination behaviour changed ✅ Passed No pagination behaviour changed issues detected.
Outbound webhook payload schema changed ✅ Passed No outbound webhook payload schema changed issues detected.
GraphQL field removed without deprecation ✅ Passed No graphql field removed without deprecation issues detected.
GraphQL enum value removed ✅ Passed No graphql enum value removed issues detected.
Vendor API consumer ✅ Passed This PR does not touch call sites of an open vendor API change.
SQL injection ✅ Passed No sql injection issues detected.
Cross-site scripting (XSS) ✅ Passed No cross-site scripting (xss) issues detected.
Path traversal ✅ Passed No path traversal issues detected.
Command injection ✅ Passed No command injection issues detected.
Insecure deserialization ✅ Passed No insecure deserialization issues detected.
Weak cryptography ✅ Passed No weak cryptography issues detected.
Hardcoded secret ✅ Passed No hardcoded secret issues detected.
Insecure randomness for security material ✅ Passed No insecure randomness for security material issues detected.
Unsafe file upload ✅ Passed No unsafe file upload issues detected.
Missing input validation ✅ Passed No missing input validation issues detected.
Unsafe CORS configuration ✅ Passed No unsafe cors configuration issues detected.
Unsafe / open redirect ✅ Passed No unsafe / open redirect issues detected.
Missing CSRF protection ✅ Passed No missing csrf protection issues detected.
Unsafe cookie / session settings ✅ Passed No unsafe cookie / session settings issues detected.
Sensitive data exposure ✅ Passed No sensitive data exposure issues detected.
API key in source ✅ Passed No api key in source detected.
Access token in source ✅ Passed No access token in source detected.
Private key in source ✅ Passed No private key in source detected.
Database connection URL with embedded credentials ✅ Passed No database connection url with embedded credentials detected.
Cloud credential in source ✅ Passed No cloud credential in source detected.
Webhook signing secret in source ✅ Passed No webhook signing secret in source detected.
OAuth client secret in source ✅ Passed No oauth client secret in source detected.
JWT signing secret in source ✅ Passed No jwt signing secret in source detected.
Hardcoded password ✅ Passed No hardcoded password detected.
Dependency has known CVE ✅ Passed No dependency has known cve findings.
Malware package in dependency ✅ Passed No malware package in dependency findings.
Possible typosquat dependency ✅ Passed No possible typosquat dependency findings.
Newly introduced dependency ✅ Passed No newly introduced dependency findings.
Unpinned production dependency ✅ Passed No unpinned production dependency findings.
Deprecated dependency ✅ Passed No deprecated dependency findings.
License allow-list violation ✅ Passed No license allow-list violation findings.
Auth middleware removed from route ✅ Passed No auth middleware removed from route issues detected.
Route protection changed (protected → public) ✅ Passed No route protection changed (protected → public) issues detected.
Permission / RBAC check removed ✅ Passed No permission / rbac check removed issues detected.
Required role weakened ✅ Passed No required role weakened issues detected.
Admin-only route exposed to lower privilege ✅ Passed No admin-only route exposed to lower privilege issues detected.
Token validation skipped in middleware chain ✅ Passed No token validation skipped in middleware chain issues detected.
JWT verification weakened or changed ✅ Passed No jwt verification weakened or changed issues detected.
Session expiration / TTL changed ✅ Passed No session expiration / ttl changed issues detected.
Password reset flow changed ✅ Passed No password reset flow changed issues detected.
OAuth callback / redirect handling changed ✅ Passed No oauth callback / redirect handling changed issues detected.
Webhook endpoint missing signature verification ✅ Passed No webhook endpoint missing signature verification issues detected.
Public route touches private/PII data ✅ Passed No public route touches private/pii data issues detected.
Frontend performance issue ✅ Passed No additional explanation was reported.
Frontend security issue ✅ Passed No additional explanation was reported.
Frontend correctness issue ✅ Passed No additional explanation was reported.
Accessibility issue ✅ Passed No additional explanation was reported.
Frontend maintainability issue ✅ Passed No additional explanation was reported.
Code correctness issue ✅ Passed No additional explanation was reported.
Data integrity risk ✅ Passed No additional explanation was reported.
Maintainability issue ✅ Passed No additional explanation was reported.
Co-change coupling ✅ Passed No additional explanation was reported.
Redundant alias / duplicate import ✅ Passed No additional explanation was reported.
Redundant type construct ✅ Passed No additional explanation was reported.
Simplifiable code ✅ Passed No additional explanation was reported.
Unnecessary type assertion ✅ Passed No additional explanation was reported.
Module smell ✅ Passed No additional explanation was reported.
Excessive complexity ✅ Passed No additional explanation was reported.
Dead export (no callers) ✅ Passed No additional explanation was reported.

This comment is updated automatically whenever Autter reviews a new PR revision.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown
🔇 6 finding(s) suppressed as likely false positives by Autter's verification pass

These were flagged by a detector but a second, full-file verification judged them not to be real issues. Listed here for transparency — review if you disagree.

  • 🟡 Possible non-atomic read-modify-write (risk 1/100) — scripts/verify-deploy.mjs:44 — This script only performs read-only HTTP smoke checks and appends failed check messages to a process-local array. It does not read-modify-write any persisted counter, balance, quota, or shared resource, so atomic updates or row locks are irrelevant.
  • 🟡 Optimistic locking not detected (risk 1/100) — scripts/verify-deploy.mjs:102 — The apparent updates are local control-flow state (home and the failures array) while verifying deployment responses. No shared resource is overwritten and there is no persistence operation that would require optimistic locking, versioning, ETags, or affected-row guards.
  • 🟠 Direct env-var access bypasses config module (risk 50/100) — scripts/verify-deploy.mjs:28 — The access is in a standalone deployment smoke-check CLI, not application code. DEPLOY_URL is an intentional command-line fallback for selecting the target URL, while the backend's EnvService governs runtime application configuration; routing this script through the Nest config module would not be applicable.
  • 🟠 Comment contradicts or fabricates code behaviour (risk 60/100) — backend/src/config/env.service.ts:92 — The full comment explicitly states that the default applies outside production and that production is deliberately not defaulted; the implementation matches that behavior by returning an empty string when NODE_ENV is production and CONVERTER_URL is unset.
  • 🟠 Comment contradicts or fabricates code behaviour (risk 50/100) — backend/.env.example:23 — The full documentation explicitly states that CONVERTER_URL is optional, defaults only outside production, and that an unset production value disables converter surfaces. The runtime EnvService implements exactly that behavior, so the comment does not leave production's unset behavior ambiguous.
  • 🟡 Code duplication / DRY violation (risk 35/100) — frontend/src/app/ui/showcase/showcase.ts:111 — The changed label is static showcase/demo copy, not runtime converter configuration. The other occurrence in badge.ts is only a JSDoc example, while EnvService.converterUrl is independently environment-configured; Settings does not display or define this host. No demonstrated production consistency contract requires the showcase label to track CONVERTER_URL.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter completed PR review for #3: 8 finding(s) remain below the merge-blocking bar, so this review stays neutral rather than approving. (Also detected: 9 finding(s) dismissed as likely false positives by verification.) See the findings below; the task checklist follows as the review's final comment.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

async rediscover(): Promise<void> {
const origin = this.converterOrigin();
if (!this.isBrowser || this.mounts() === 0 || !origin || this.isSameOrigin()) return;
await this.copilot.discoverRemoteTools([origin]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] In-flight discovery can repopulate tools after the last frame is released — Risk: 78/100

This asynchronous caller chain is not safe on teardown: rediscover() checks mounts() > 0 and then awaits Copilot.discoverRemoteTools(), while the last release() can synchronously call teardown() and clearRemoteTools(). If the discovery promise resolves after that clear, discoverRemoteTools() writes the stale cross-origin descriptors back into the Copilot even though the iframe/document that owns them has already been destroyed. Subsequent Copilot turns can offer and invoke convertCurrency against a gone frame. The session needs cancellation or a generation/mount check after the await before committing discovery results.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/currency-converter.ts, frontend/src/app/converter/converter-session.ts, frontend/src/app/copilot/copilot.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Make ConverterSession invalidate in-flight discovery when mounts reach zero, and prevent a completed discoverRemoteTools result from being committed after teardown or after a newer mount/session has replaced it. Preserve the reference-counted lifecycle.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 1 finding(s) as review threads below (🟠 1). Each carries a copy-paste AI fix prompt.

async rediscover(): Promise<void> {
const origin = this.converterOrigin();
if (!this.isBrowser || this.mounts() === 0 || !origin || this.isSameOrigin()) return;
await this.copilot.discoverRemoteTools([origin]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] In-flight discovery can restore tools after the last converter frame is gone — Risk: 78/100

This awaits Copilot discovery without preserving the mount generation that started it. A frame load, toolchange, or acquire() schedules rediscover(), and getTools() can still be pending when the final release runs teardown() and clears remoteTools; when that pending request resolves, line 183 repopulates the Copilot with descriptors for a document that has already been destroyed. The Copilot can then offer convertCurrency and invoke a stale iframe, producing failed calls until another discovery or mount corrects the state. Route changes and iframe teardown make this reachable in normal operation, not only on an artificial concurrent call.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Guard discovery results with a mount/session generation or cancellation token, and discard any result that completes after teardown or after a newer discovery; retain the clear-on-last-release behavior.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 2 finding(s) as review threads below (🟠 2). Each carries a copy-paste AI fix prompt.

*/
acquire(): () => void {
this.mounts.update((n) => n + 1);
void this.ensureConfig().then(() => this.rediscover());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] In-flight discovery can repopulate tools after the converter has been torn down — Risk: 78/100

The discovery started here is not cancellable and its completion is not tied to the lifetime of the mount that started it. If getTools() is still pending when the last converter surface closes, teardown() clears the Copilot's remote tools, but the pending request can later resolve and discoverRemoteTools() writes the stale descriptors back into the Copilot. The app then offers convertCurrency to Gemini even though no converter iframe remains, so the next call fails instead of being omitted; a subsequent toolchange or navigation can make this intermittent and difficult to diagnose. Use a generation/lifetime check or abort the discovery before applying its result.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts, frontend/src/app/copilot/copilot.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Make ConverterSession cancel or invalidate every discovery started by a mount. Before applying a resolved discovery result, verify that the session still has a mounted converter and that the request belongs to the current discovery generation; otherwise discard it. Ensure teardown invalidates the generation (and aborts any supported underlying request). Add a regression test that holds discovery pending, releases the final mount, resolves discovery, and verifies remote tools remain cleared.

Flagged by Autter security & observability checks.

const base = this.url();
if (!base || !this.isAvailable()) return null;
const separator = base.includes('?') ? '&' : '?';
return `${base}${separator}actuo=${encodeURIComponent(this.selfOrigin())}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Handshake is appended inside the fragment for converter URLs with a hash — Risk: 62/100

The URL builder chooses only between ? and & based on whether the base contains a query, but it never accounts for a fragment. For a valid configured URL such as https://converter.example/app#rates, line 114 produces https://converter.example/app#rates?actuo=...; everything after # is a fragment and is not sent to the converter as query data. The framed converter therefore cannot read actuo, will not expose its tools to Actuo, and the cross-origin demo silently discovers nothing even though the URL passes the HTTP(S) validation. The same defect occurs with a URL that has both query and fragment.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts
🛠 AI fix prompt (copy & paste into your coding agent)
Build the handshake with `new URL(base)`, set `searchParams.set('actuo', selfOrigin)`, and serialize the URL so the parameter is inserted before any hash fragment.

Flagged by Autter security & observability checks.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown

Autter found 1 issue(s) it could not attach to the current diff (the anchor line is not part of a diff hunk, or the PR advanced during the review):

🔴 [ai] Remote WebMCP tools bypass mutation confirmation (risk 86/100)

frontend/src/app/copilot/copilot.ts:212 · sec_missing_input_validation

This is not safe to ship because the new runtime-configured converter is an executable cross-origin tool surface, but its WebMCP descriptor metadata is not used to enforce the confirmation policy. runTool() sets mutates to false for every tool without a local contract and only waits for confirmation when contract?.requiresConfirmation is true; remote descriptors are never contracts. Consequently, any tool exposed by the configured converter, including a mutating or sensitive tool, can be selected by Gemini and executed via invokeCrossOrigin() without the in-chat Confirm/Cancel gate, while the UI labels it as read-only. A compromise, misconfiguration, or future change to the trusted converter can therefore cause an unreviewed cross-origin action in the user's session.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/copilot/copilot.ts, frontend/src/app/webmcp/tool-registry.ts, shared/src/tools.ts

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 14 finding(s) as review threads below (🟠 14). Each carries a copy-paste AI fix prompt.

const configured = this.optional('CONVERTER_URL');
if (configured) return configured;
return process.env['NODE_ENV'] === 'production' ? '' : 'http://localhost:4201';
return process.env['NODE_ENV'] === 'production'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/complexity/useLiteralKeys — Risk: 55/100

The computed expression can be simplified without the use of a string literal.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/complexity/useLiteralKeys` issue at backend/src/config/env.service.ts:102: The computed expression can be simplified without the use of a string literal.

Flagged by Autter security & observability checks.

await settle();

expect(api.get).toHaveBeenCalledTimes(1);
releases.forEach((release) => release());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/complexity/noForEach — Risk: 55/100

Prefer for...of instead of forEach.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/complexity/noForEach` issue at frontend/src/app/converter/converter-session.spec.ts:129: Prefer for...of instead of forEach.

Flagged by Autter security & observability checks.

await settle();

expect(registry.onToolChange).toHaveBeenCalledTimes(1);
releases.forEach((release) => release());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/complexity/noForEach — Risk: 55/100

Prefer for...of instead of forEach.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/complexity/noForEach` issue at frontend/src/app/converter/converter-session.spec.ts:242: Prefer for...of instead of forEach.

Flagged by Autter security & observability checks.


const frame = iframe();
expect(frame).not.toBeNull();
expect(frame!.getAttribute('allow')).toBe('tools');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/converter/currency-converter.spec.ts:101: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

const frame = iframe();
expect(frame).not.toBeNull();
expect(frame!.getAttribute('allow')).toBe('tools');
expect(frame!.getAttribute('referrerpolicy')).toBe('no-referrer');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/converter/currency-converter.spec.ts:102: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

expect(frame).not.toBeNull();
expect(frame!.getAttribute('allow')).toBe('tools');
expect(frame!.getAttribute('src')).toContain(`${PARTNER}/partner-demo/`);
expect(frame!.getAttribute('referrerpolicy')).toBe('no-referrer');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/pages/agent/agent.spec.ts:123: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

expect(frame!.getAttribute('allow')).toBe('tools');
expect(frame!.getAttribute('src')).toContain(`${PARTNER}/partner-demo/`);
expect(frame!.getAttribute('referrerpolicy')).toBe('no-referrer');
expect(frame!.getAttribute('src')).toContain(CONVERTER_URL);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/pages/agent/agent.spec.ts:124: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

@@ -0,0 +1,78 @@
import { ChangeDetectionStrategy, Component, OnInit, inject } from '@angular/core';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/useImportType — Risk: 55/100

Some named imports are only used as types.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/useImportType` issue at frontend/src/app/pages/convert/convert.ts:1: Some named imports are only used as types.

Flagged by Autter security & observability checks.

(b.textContent ?? '').includes("won't change the total"),
) as HTMLButtonElement | undefined;
expect(trigger).toBeDefined();
trigger!.click();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/pages/dashboard/dashboard.spec.ts:318: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

(b.textContent ?? '').includes('What is this in'),
) as HTMLButtonElement | undefined;
expect(trigger).toBeDefined();
trigger!.click();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [deterministic] Biome: lint/style/noNonNullAssertion — Risk: 55/100

Forbidden non-null assertion.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noNonNullAssertion` issue at frontend/src/app/pages/expenses/expenses.spec.ts:266: Forbidden non-null assertion.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 14 finding(s) as review threads below (🟡 14). Each carries a copy-paste AI fix prompt.

Comment thread CLAUDE.md

1. **`inputSchema` type varies by Chrome version.** Chrome 149–153 (most of the origin-trial
population, including the Chrome 152 on this machine) return it as a **serialized JSON
population, including the Chrome 151 on this machine) return it as a **serialized JSON

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 89

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at CLAUDE.md:218: Line length: Expected: 80; Actual: 89

Flagged by Autter security & observability checks.

Comment thread CLAUDE.md
URL can change without a release on the other side.

**`ConverterSession` owns the discovery lifecycle, not any page.** It used to
belong to `/agent`, which was fine while one page framed one other origin. With four

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 84

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at CLAUDE.md:282: Line length: Expected: 80; Actual: 84

Flagged by Autter security & observability checks.

Comment thread docs/Actuo-Design-Doc.md
- A small colored dot indicating `readOnlyHint` (blue = safe/read-only) vs a mutating action (amber = changed something)
4. **Sensitive actions** (submit/delete/approve) trigger a native confirmation step **inside the chat itself** — a card with "Confirm" / "Cancel" buttons — never a silent execution.
5. **Cross-origin calls** get a distinct badge showing the origin the tool came from (e.g. "via partner-demo.app") — this is a great subtle way to make the cross-origin feature *visible* to a judge/demo audience instead of invisible plumbing.
5. **Cross-origin calls** get a distinct badge showing the origin the tool came from (e.g. "via cambiaro.programmersingh.dev") — this is a great subtle way to make the cross-origin feature *visible* to a judge/demo audience instead of invisible plumbing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 254

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at docs/Actuo-Design-Doc.md:115: Line length: Expected: 80; Actual: 254

Flagged by Autter security & observability checks.

Comment thread docs/Actuo-PRD.md
- Store amounts in original currency + converted base-currency value
- Live FX rate fetch (cached daily) via a public FX API
- Historical rate lock at time of entry
- **Embedded currency converter (advisory):** a separate converter app is framed on `/convert`, on `/agent`, beside the dashboard's excluded-rows notice, and on expense rows filed in another currency. It is a *reference*, not a rate source — nothing it shows is written to `converted_amount`, folded into a total, or allowed to change the excluded-rows copy. Until the FX pass above exists, a total that says what it left out stays the honest answer; a rate looked up today is not the historical rate locked at entry.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 517

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at docs/Actuo-PRD.md:110: Line length: Expected: 80; Actual: 517

Flagged by Autter security & observability checks.

Comment thread docs/Actuo-PRD.md
- Live FX rate fetch (cached daily) via a public FX API
- Historical rate lock at time of entry
- **Embedded currency converter (advisory):** a separate converter app is framed on `/convert`, on `/agent`, beside the dashboard's excluded-rows notice, and on expense rows filed in another currency. It is a *reference*, not a rate source — nothing it shows is written to `converted_amount`, folded into a total, or allowed to change the excluded-rows copy. Until the FX pass above exists, a total that says what it left out stays the honest answer; a rate looked up today is not the historical rate locked at entry.
- The converter is a genuinely separate origin exposing its own WebMCP tools, so it is also how §7's cross-origin row is satisfied on a real deploy (see §6.8).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 159

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at docs/Actuo-PRD.md:111: Line length: Expected: 80; Actual: 159

Flagged by Autter security & observability checks.

Comment thread Progress.md
| Live FX + daily cache | 1 | ⬜ | No FX client, no cache, no rates table |
| Live FX + daily cache | 1 | ⬜ | No FX client, no cache, no rates table. The embedded converter does **not** count — see below |
| Historical rate lock | 1 | ⬜ | No rate column |
| Embedded converter (advisory) | 1 | ✅ | `converter/currency-converter.ts` frames a separate converter app on `/convert`, `/agent`, the dashboard notice and foreign-currency expense rows. One frame at a time, lazily mounted, `CONVERTER_URL` from `GET /api/config` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 266

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:152: Line length: Expected: 80; Actual: 266

Flagged by Autter security & observability checks.

Comment thread Progress.md
| Key-setup flow when no key | 0 | ✅ | Opens into setup rather than failing silently |
| Embeddable via one `<script>` | 3 | ⬜ | It is an Angular component inside the app shell |
| **Cross-origin tool use** | 0 | ✅ | `/agent` embeds the partner page from `PARTNER_DEMO_ORIGIN` (:4201, `scripts/partner-server.mjs`) with `allow="tools"` and calls `discoverRemoteTools()`. Verified in Chrome 151 with the flag: both partner tools discovered, `executeTool()` returned a price cross-origin |
| **Cross-origin tool use** | 0 | ✅ | The converter is framed from `CONVERTER_URL` with `allow="tools"`, and `ConverterSession` owns discovery for all four surfaces. The synthetic partner page and its :4201 server are gone: dev and production now frame the same independently deployed converter, so there is no cross-origin path that is only exercised in one of them. **Verified 2026-09-03 in Chrome 151 with the flag**, framing the deployed converter from `localhost:4200`: all seven of its tools discovered over a real origin boundary, badges correct (4 read-only / 3 mutating), and `executeTool(convertCurrency, {amount:200,from:'EUR',to:'INR'})` returned `200 EUR = 22,018.00 INR` with the embedded widget moving to match. Not yet run from a *deployed* Actuo — see the rough edges |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 786

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:213: Line length: Expected: 80; Actual: 786

Flagged by Autter security & observability checks.

Comment thread Progress.md
| Cancellation (`AbortSignal`) | ✅ | Client aborts, polls stop, server abandons the job mid-fetch and mid-format |
| Cross-origin tools | ✅ | See §6.8. Needs a genuinely second origin — same-origin descriptors are filtered out, which is what made the earlier setup unprovable |
| Security annotations | ✅ | `readOnlyHint` on all five, driving the shell's re-poll and the `/agent` panel. `untrustedContentHint` on `search_expenses` and `approve_expense` — the two that surface *another person's* free text — and on the partner-demo tools; shown as a badge on the tool-call card |
| Cross-origin tools | ✅ | See §6.8. Needs a genuinely second origin — same-origin descriptors are filtered out, which is what made the earlier in-repo page unprovable. It is now a separately built, independently deployed app Actuo does not own, in dev as well as on a deploy |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 277

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:235: Line length: Expected: 80; Actual: 277

Flagged by Autter security & observability checks.

Comment thread Progress.md
| Cross-origin tools | ✅ | See §6.8. Needs a genuinely second origin — same-origin descriptors are filtered out, which is what made the earlier setup unprovable |
| Security annotations | ✅ | `readOnlyHint` on all five, driving the shell's re-poll and the `/agent` panel. `untrustedContentHint` on `search_expenses` and `approve_expense` — the two that surface *another person's* free text — and on the partner-demo tools; shown as a badge on the tool-call card |
| Cross-origin tools | ✅ | See §6.8. Needs a genuinely second origin — same-origin descriptors are filtered out, which is what made the earlier in-repo page unprovable. It is now a separately built, independently deployed app Actuo does not own, in dev as well as on a deploy |
| Security annotations | ✅ | `readOnlyHint` on all five, driving the shell's re-poll and the `/agent` panel. `untrustedContentHint` on `search_expenses` and `approve_expense` — the two that surface *another person's* free text — and on the converter's tools, whose results carry third-party rate data; shown as a badge on the tool-call card |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 342

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:236: Line length: Expected: 80; Actual: 342

Flagged by Autter security & observability checks.

Comment thread README.md
| **Dynamic / state-gated tools** — `approve_expense` registers only while you can actually approve something, and fires `toolchange` as that changes | `frontend/src/app/webmcp/tool-session.ts` |
| **Cancellation** — `generate_report` honours `AbortSignal`; the server abandons the job mid-flight, not just the client | `frontend/src/app/tools/expense-tools.ts`, `backend/src/reports/` |
| **Cross-origin tools** — the Copilot discovers and calls tools published by an unrelated site | `frontend/src/app/pages/agent/agent.ts`, `frontend/public/partner-demo/` |
| **Cross-origin tools** — the Copilot discovers and calls tools published by a separately built, independently deployed app Actuo does not own | `frontend/src/app/converter/`, `frontend/src/app/pages/agent/agent.ts` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 218

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at README.md:29: Line length: Expected: 80; Actual: 218

Flagged by Autter security & observability checks.

The deployed site has been serving a literal `__PUBLIC_ORIGIN__` in its
`canonical` and `og:image`. A crawler reading a malformed URL is worse than
one reading a relative URL, so this is not cosmetic.

`stamp-seo.mjs` walked only `dist/frontend/browser`. Angular keeps its own
copies of the page HTML under `server/` — `index.server.html` and the
`assets-chunks/*.mjs` templates, `index_csr_html.mjs` among them — and those
are what the SSR handler actually serves. It now walks the whole
`dist/frontend` tree, and `.mjs` is stampable because that is where those
templates live. Coverage goes from 28 URLs across 7 files to 63 across 14,
and a build with PUBLIC_ORIGIN set now leaves zero sentinels anywhere in the
output. With it unset everything still falls back to root-relative.

What made this hard to see is worth recording: `sitemap.xml` and
`robots.txt` were correct on the live site the whole time, because they are
served from `browser/`. That is also the evidence the build was fine — the
origin reached it, and only some files got it.

The same look at the running site turned up a second defect this repo
cannot fix from the repo: `/` carries no `ng-server-context`, so Angular is
falling back to client-side rendering in production and discarding the SSR
and structured-data work. It works locally, which is exactly how it went
unnoticed — the same silent failure as 2026-08-29. `NG_ALLOWED_HOSTS` has
to be set on the *service*; declaring it in `render.yaml` is not sufficient
if the service was created by hand rather than from the Blueprint, because
then its envVars were never applied.

So `pnpm run verify:deploy <url>` now exists. Every check in it is there
because that thing broke in production while every test passed and the page
looked fine: the API answers, `/` genuinely server-renders, no sentinel
survives in `/`, `/sitemap.xml` or `/robots.txt`, and the converter is
configured on an origin the app does not serve. Each failure names the fix.
Run against the live deploy it reproduces all three known defects and
correctly passes sitemap and robots — the discriminating signal that led to
the root cause.

Progress.md was believed and it had drifted both ways. It did not know the
app is deployed at all (§12 still described creating the Blueprint), while
"What to fix next" still described an interactive Google login, the project
actuo-2f1f3 and the Blaze plan — all Firebase App Hosting, abandoned in
6b5a3d5. §8.5 claimed SSR was fixed when it is broken in production. The
baseline was one commit stale, §7 named `discoveredTools` where `/agent`
renders `Copilot.crossOriginTools`, the App Hosting rough edge cited
firebase-tools#7478 where the current docs cite #10435, and "Demo video"
was listed twice. All corrected. §12 stays 🟡 rather than going ✅: the
deploy exists and is healthy, and it is also defective.

Five source comments still reasoned about "Firebase App Hosting" as the
runtime. The reasoning survives the move to Render unchanged — single
process, in-memory jobs, per-instance rate limiting, an unauthenticated
health probe — so only the platform name was wrong, and only that changed.

Gate: typechecks clean, 894 tests green (9 shared, 65 backend, 786
frontend, 34 e2e), build clean with 11 routes prerendered and zero
sentinels surviving.

Not fixed here, because it is not in this repo: NG_ALLOWED_HOSTS and
CONVERTER_URL on the Render service, and a redeploy to pick up the stamp
fix — PUBLIC_ORIGIN is a build arg, so a restart cannot carry it.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Autter review in progress — running security, correctness & dependency checks on this PR. Follow live step-by-step progress on the autter/review-gate check in the merge box. Merge is blocked until the gate completes; Autter approves automatically when the review comes back clean, and releases this hold with a neutral review when it finds non-blocking issues.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 1 finding(s) as review threads below (🟡 1). Each carries a copy-paste AI fix prompt.

Comment thread scripts/verify-deploy.mjs
const { status, body } = await get('/api/health');
const ok = status === 200 && JSON.parse(body)?.status === 'ok';
if (ok) pass('/api/health', '200 ok');
else fail('/api/health', `status ${status}, body ${body.slice(0, 120)}`, 'The container is not serving Nest. Check the service logs.');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Log injection — Risk: 45/100

The deployment verifier logs the fetched health-response body directly in the /api/health failure path. A compromised or unexpected deployment response can include CRLF/control characters and forge additional console log lines, affecting CI or operator logs for verify-deploy.mjs. Blast radius — if this logging/observability gap is exploited it affects: dependent files frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts
🛠 AI fix prompt (copy & paste into your coding agent)
In `scripts/verify-deploy.mjs`, sanitize the health response before interpolating it into the failure message: remove or escape carriage returns, newlines, and other control characters, then truncate the sanitized value to the existing length limit. Apply the same protection to any other fetched response or externally supplied value that is logged.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 2 finding(s) as review threads below (🟠 1 · 🟡 1). Each carries a copy-paste AI fix prompt.

const configured = this.optional('CONVERTER_URL');
if (configured) return configured;
return process.env['NODE_ENV'] === 'production' ? '' : 'http://localhost:4201';
return process.env['NODE_ENV'] === 'production'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [ai] Code style differs from rest of codebase — Risk: 55/100

The computed environment access uses a string literal key, which violates the repository's Biome useLiteralKeys rule and differs from the preferred property-access style. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions EnvService.optional, EnvService.partnerOrigin, EnvService.baseCurrency, EnvService, HealthController, ClientConfig, Showcase, Settings; scopes backend; dependent files frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts.

References:

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: EnvService.optional, EnvService.partnerOrigin, EnvService.baseCurrency, EnvService, HealthController, ClientConfig, Showcase, Settings
  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts
  • Scopes: backend
🛠 AI fix prompt (copy & paste into your coding agent)
Replace process.env['NODE_ENV'] with process.env.NODE_ENV while preserving the production check. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `EnvService.optional`, `EnvService.partnerOrigin`, `EnvService.baseCurrency`, `EnvService`, `HealthController`, `ClientConfig`, `Showcase`, `Settings`; scopes `backend`; dependent files `frontend/src/app/pages/agent/agent.ts`, `frontend/src/app/ui/showcase/showcase.ts`, `backend/src/config/config.controller.ts`, `frontend/src/app/app.routes.ts`, `frontend/src/app/copilot/copilot.spec.ts`, `frontend/src/app/pages/agent/agent.spec.ts`, `frontend/src/app/ui/badge.spec.ts`, `backend/src/expenses/expenses.service.ts`.

Flagged by Autter security & observability checks.


import { ApiClient } from '../../core/api/api-client.js';
import { Card, EmptyState, ErrorState, Skeleton, StatCard } from '../../ui';
import { CurrencyConverter } from '../../converter/currency-converter.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [ai] Established pattern ignored — Risk: 45/100

CurrencyConverter and ConverterSession are statically imported into the dashboard, so the converter implementation is included in the dashboard route chunk even though the UI is conditional and only needed after interaction. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions Dashboard, HealthController, ClientConfig, EnvService, Showcase, Settings, RateLimitGuard, Agent; scopes frontend; dependent files frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts.

References:

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: Dashboard, HealthController, ClientConfig, EnvService, Showcase, Settings, RateLimitGuard, Agent
  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts
  • Scopes: frontend
🛠 AI fix prompt (copy & paste into your coding agent)
Defer or dynamically load the standalone converter component, keeping only minimal trigger/session wiring in the dashboard so the converter implementation is split from the initial dashboard route chunk. Blast radius — if this AI-generated slop ships it cascades to the downstream usage that depends on this file: functions `Dashboard`, `HealthController`, `ClientConfig`, `EnvService`, `Showcase`, `Settings`, `RateLimitGuard`, `Agent`; scopes `frontend`; dependent files `frontend/src/app/pages/agent/agent.ts`, `frontend/src/app/ui/showcase/showcase.ts`, `backend/src/config/config.controller.ts`, `frontend/src/app/app.routes.ts`, `frontend/src/app/copilot/copilot.spec.ts`, `frontend/src/app/pages/agent/agent.spec.ts`, `frontend/src/app/ui/badge.spec.ts`, `backend/src/expenses/expenses.service.ts`.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 1 finding(s) as review threads below (🟡 1). Each carries a copy-paste AI fix prompt.

selector: 'app-dashboard',
changeDetection: ChangeDetectionStrategy.OnPush,
imports: [Card, EmptyState, ErrorState, Skeleton, StatCard],
imports: [Card, CurrencyConverter, EmptyState, ErrorState, Skeleton, StatCard],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [ai] Currency converter implementation is eagerly bundled in the dashboard route — Risk: 45/100

The static CurrencyConverter import and component registration place the converter implementation in the dashboard route chunk even though the lookup is conditional and only relevant to users who use it. Consider @defer (on interaction) or dynamically loading a standalone converter component if it should not increase the initial dashboard route payload.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Functions/symbols: Dashboard, HealthController, ClientConfig, EnvService, Showcase, Settings, RateLimitGuard, Agent
  • Dependent files: frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts, backend/src/config/config.controller.ts, frontend/src/app/app.routes.ts, frontend/src/app/copilot/copilot.spec.ts, frontend/src/app/pages/agent/agent.spec.ts, frontend/src/app/ui/badge.spec.ts, backend/src/expenses/expenses.service.ts
  • Scopes: frontend
🛠 AI fix prompt (copy & paste into your coding agent)
Lazy-load the standalone CurrencyConverter with Angular @defer (on interaction) or a dynamically loaded component, keeping only the minimal converter trigger and session wiring eager.

Flagged by Autter security & observability checks.

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter blocked this PR: 1 confirmed correctness/runtime finding(s). See the findings below and the full PR review for details.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown

Autter task list

  • @theprogrammersingh Close all converter and remote-tool resources (frontend/src/app/pages/agent/agent.ts, frontend/src/app/pages/convert/convert.ts, frontend/src/app/converter/converter-session.ts) - Audit Agent.rediscover and the convert-page lifecycle, then add explicit cleanup, abort, and error-path handling for every response, subscription, connection, and iframe-related handle, with regression tests for repeated mount, navigation, and teardown.
  • @theprogrammersingh Remove sensitive data from converter test fixtures (frontend/src/app/converter/converter-session.spec.ts, frontend/src/app/converter/converter-session.ts) - Replace any API keys, payment tokens, or other PII-like values in converter-session fixtures with clearly synthetic placeholders and verify that sensitive values are neither persisted nor logged without approved application-level protection.
  • @theprogrammersingh Split the converter feature from legacy-demo retirement (frontend/src/app/converter/currency-converter.ts, scripts/partner-server.mjs, frontend/public/partner-demo/index.html) - Separate the converter, expense-pagination, and Copilot behavior changes from partner-demo deletion and documentation or cleanup changes into focused follow-up commits or pull requests, preserving focused test and review scope.
  • @theprogrammersingh Fix environment access style and track configuration ownership (backend/src/config/env.service.ts, backend/src/config/config.controller.ts, backend/.env.example) - Change the computed environment lookup in EnvService to the repository-preferred property-access form, add or reference a linked tracker issue for CONVERTER_URL behavior, and establish CODEOWNERS or obtain explicit owner approval for the affected configuration and consumer paths.
  • @theprogrammersingh Harden the deployment image healthcheck (Dockerfile, scripts/verify-deploy.mjs, backend/src/health/health.controller.ts) - Update the Dockerfile healthcheck to satisfy CKV_DOCKER_2 using the supported repository runtime and verify the resulting image health behavior in the deployment validation flow.
  • @theprogrammersingh Sanitize deployment verifier response logging (scripts/verify-deploy.mjs) - Replace raw health-response-body logging with bounded, escaped, and non-control-character output or a fixed diagnostic message before writing failures to CI and operator logs.
  • @theprogrammersingh Lazy-load the dashboard converter (frontend/src/app/pages/dashboard/dashboard.ts, frontend/src/app/converter/currency-converter.ts, frontend/src/app/converter/converter-session.ts) - Remove the static CurrencyConverter and ConverterSession imports from the dashboard route and load or defer the converter only on user interaction, adding a bundle or route test that confirms the initial dashboard chunk excludes converter code.
  • @theprogrammersingh Restore observability for retired partner paths (scripts/partner-server.mjs, frontend/src/app/pages/agent/agent.ts, frontend/src/app/ui/showcase/showcase.ts) - Confirm the partner-server retirement does not remove monitoring needed by active converter, agent, configuration, or deployment flows, and add equivalent structured logging or metrics around cross-origin discovery, handshake failures, and converter availability.

Generated from PR diff, blast radius, and context.

Issues found

  1. Resource leak risk · risk 89/100 · frontend/src/app/pages/agent/agent.ts:258
  2. PII stored without application-level encryption · risk 85/100 · frontend/src/app/converter/converter-session.spec.ts:84
  3. PR changes too many lines — split it · risk 74/100 · backend/src/config/env.service.ts:76
  4. PR mixes refactor and behavior change · risk 55/100 · scripts/partner-server.mjs:1
  5. Code style differs from rest of codebase · risk 55/100 · backend/src/config/env.service.ts:102
  6. Infrastructure misconfiguration · risk 49/100 · Dockerfile:77
  7. Missing linked tracker issue · risk 49/100 · backend/src/config/env.service.ts:99
  8. Missing CODEOWNERS reviewer approval · risk 49/100 · backend/src/config/env.service.ts:99
  9. Resource leak risk · risk 49/100 · frontend/src/app/pages/convert/convert.ts:76
  10. Removed observability · risk 45/100 · scripts/partner-server.mjs:67
  11. Log injection · risk 45/100 · scripts/verify-deploy.mjs:55
  12. Established pattern ignored · risk 45/100 · frontend/src/app/pages/dashboard/dashboard.ts:15
  13. Currency converter implementation is eagerly bundled in the dashboard route · risk 45/100 · frontend/src/app/pages/dashboard/dashboard.ts:75
  14. Currency converter is eagerly bundled in the dashboard route · risk 45/100 · frontend/src/app/pages/dashboard/dashboard.ts:15

Also detected but not listed above: 7 finding(s) dismissed as likely false positives by verification — see the Autter review dashboard for their verdicts.

🛠 Fix options

Check one option and Autter will start a fix run for the unresolved issues above.

  • One PR with all unresolved fixes
  • One independent PR per unresolved issue

Checking a box triggers the fix run immediately — Autter comments back with the issues being fixed and the branch created for each.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown

🧪 Autter test run

Autter checked 229716f2.

Execution summary: 8 checks executed · 8 passed.

Project test commands

Scope Command Result
backend npm run test ⚠️ could not run — missing toolchain
frontend npm run test ⚠️ could not run — missing toolchain
scripts npm run test ⚠️ could not run — missing toolchain

Autter targeted verification

8 tests executed · 8 passed.

Declared tests: 53 test file(s) found — 0 ran, 0 not observed in suite output, 53 did not run.

⚠️ Test cases that did not run
  • backend/src/app.controller.spec.ts (1 case) — its suite was skipped (missing toolchain)
  • backend/src/audit/audit.service.spec.ts (4 cases) — its suite was skipped (missing toolchain)
  • backend/src/budgets/budgets.service.spec.ts (4 cases) — its suite was skipped (missing toolchain)
  • backend/src/config/env.service.spec.ts (8 cases) — its suite was skipped (missing toolchain)
  • backend/src/expenses/expense-state-machine.spec.ts (12 cases) — its suite was skipped (missing toolchain)
  • backend/src/reports/reports.service.spec.ts (11 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/gemini-client.spec.ts (41 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/gemini-protocol.spec.ts (7 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/gemini-schema.spec.ts (22 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/key-privacy.spec.ts (7 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/key-store.spec.ts (17 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ai/models.spec.ts (16 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/app.spec.ts (13 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/converter/converter-session.spec.ts (20 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/converter/currency-converter.spec.ts (15 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/copilot/copilot.spec.ts (13 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/expense/amount.spec.ts (18 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/expense/expense-actions.spec.ts (18 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/pwa/pwa-service.spec.ts (12 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/seo/seo-service.spec.ts (5 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/session/session.spec.ts (29 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/core/theme/theme-service.spec.ts (12 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/add-expense/add-expense.spec.ts (12 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/agent/agent.spec.ts (17 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/auth/login.spec.ts (14 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/auth/signup.spec.ts (10 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/budgets/budget-rollup.spec.ts (20 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/budgets/budgets.spec.ts (30 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/dashboard/dashboard.spec.ts (25 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/dashboard/spend-pace.spec.ts (37 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/expenses/expense-filter.spec.ts (25 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/expenses/expenses.spec.ts (47 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/settings/ai-settings.spec.ts (26 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/settings/key-privacy.spec.ts (8 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/pages/settings/settings.spec.ts (22 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/tools/expense-tools.spec.ts (10 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ui/badge.spec.ts (7 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ui/button.spec.ts (13 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ui/card.spec.ts (6 cases) — its suite was skipped (missing toolchain)
  • frontend/src/app/ui/empty-state.spec.ts (6 cases) — its suite was skipped (missing toolchain)
  • …and 13 more

Change coverage (is each changed file's behavior verified by a test?)

Changed file Related test Result
backend/src/common/rate-limit.guard.ts backend/test/auth.e2e-spec.ts ✅ existing test passes
backend/src/config/config.controller.ts backend/test/rbac.e2e-spec.ts ✅ no declared test — temporary test passed
backend/src/config/env.service.ts backend/src/config/env.service.spec.ts ✅ existing test passes
backend/src/health/health.controller.ts backend/test/routing-contract.e2e-spec.ts ✅ existing test passes
backend/src/reports/reports.service.ts backend/src/reports/reports.service.spec.ts ✅ existing test passes
frontend/src/app/converter/converter-session.ts frontend/src/app/converter/converter-session.spec.ts ✅ existing test passes
backend/test/routing-contract.e2e-spec.ts backend/test/routing-contract.e2e-spec.ts ✅ existing test passes
frontend/src/app/app.routes.ts frontend/src/app/app.spec.ts ✅ no declared test — temporary test passed
🤖 Coverage-check evidence

backend/src/common/rate-limit.guard.ts
Ran: git diff 88f79a4..229716f -- backend/src/common/rate-limit.guard.ts; pnpm --filter backend run test:e2e

Diff is comment-only (Firebase App Hosting -> generic single-process deploy wording); no behavioral change to exercise. The guard's behavior is exercised by auth.e2e-spec.ts:325 'rate-limits repeated login attempts from one client' (asserts 429s). Full e2e run: Test Files 4 passed (4), Tests 34 passed (34).

backend/src/config/config.controller.ts
Ran: pnpm --filter backend run test:e2e; then temp probe: pnpm --filter backend exec vitest run --config .autter/scratch/vitest.backend.config.ts config-controller

rbac.e2e-spec.ts:427 hits GET /api/config but only asserts geminiModels/defaultGeminiModel — the renamed field (partnerOrigin -> converterUrl) is executed yet never asserted, so coverage is inadequate. Temp probe (.autter/scratch/config-controller.temp.spec.ts) instantiates ConfigController and asserts: converterUrl served from CONVERTER_URL, partnerOrigin absent, empty in production when unset, model catalogue intact — 'Test Files 1 passed (1), Tests 3 passed (3)'.

backend/src/config/env.service.ts
Ran: pnpm --filter backend exec vitest run src/config/env.service.spec.ts src/reports/reports.service.spec.ts

The spec's 'converterUrl' describe block exercises exactly this PR's change: configured URL wins (dev and prod), dev default is https://cambiaro.programmersingh.dev/, production default is '', blank treated as unconfigured, path carried through for a non-root converter. Result: Test Files 2 passed (2), Tests 19 passed (19).

backend/src/health/health.controller.ts
Ran: git diff 88f79a4..229716f -- backend/src/health/health.controller.ts; pnpm --filter backend run test:e2e

Diff is comment-only (Firebase App Hosting -> platform health probe); no behavioral change. routing-contract.e2e-spec.ts:38 exercises GET /api/health. Full e2e run: Test Files 4 passed (4), Tests 34 passed (34).

backend/src/reports/reports.service.ts
Ran: git diff 88f79a4..229716f -- backend/src/reports/reports.service.ts; pnpm --filter backend exec vitest run src/config/env.service.spec.ts src/reports/reports.service.spec.ts

Diff is comment-only (App Hosting wording in the ReportJob docblock); no behavioral change. The declared spec for this module passes: Test Files 2 passed (2), Tests 19 passed (19).

frontend/src/app/converter/converter-session.ts
Ran: pnpm --filter frontend exec ng test --no-watch --filter "ConverterSession"

The spec (added in this PR) exercises the new ConverterSession end to end with a stubbed ApiClient/Copilot/ToolRegistry: origin derivation from a path-bearing URL, non-http(s) rejection, same-origin detection, frameUrl ?actuo= handshake, single-surface radio group, reference-counted acquire/release with clearRemoteTools on last release, toolchange rediscovery, config failure fallback. Result: Test Files 1 passed | 44 skipped, Tests 20 passed | 766 skipped.

backend/test/routing-contract.e2e-spec.ts
Ran: git diff 88f79a4..229716f -- backend/test/routing-contract.e2e-spec.ts; pnpm --filter backend run test:e2e

The change to this file is comment-only (docblock wording: Firebase App Hosting -> combined single-process deploy); the assertions are untouched. The spec itself was executed and passes: full e2e run Test Files 4 passed (4), Tests 34 passed (34), including the /api/health + unknown /api/* JSON-404 contract.

frontend/src/app/app.routes.ts
Ran: grep confirmed no declared test imports app.routes (app.spec.ts uses provideRouter([])); temp probe: cd frontend && pnpm exec vitest run --config ../.autter/scratch/vitest.frontend.config.ts

No declared test covers the routes table. Temp probe (.autter/scratch/app-routes.temp.spec.ts) imports the real routes array and asserts the new /convert route: present, canActivate contains authGuard, data.robots === NOT_INDEXABLE, title 'Currency converter · Actuo', lazily loaded (no static component; loadComponent() resolves to the same Convert class as a direct import of pages/convert/convert.js). Result: 'Test Files 2 passed (2), Tests 5 passed (5)'.

Temporary tests are written under .autter/scratch/ for verification only — they are never committed to the repository.

Test plan (from the PR description)

  • ⬜ Run the repository CI-equivalent install, shared build, backend/frontend typechecks, unit tests, backend e2e tests, and full SSR/SEO build using the documented npm workspace commands. — Required CI-equivalent suites were skipped because toolchains were missing.
  • ⬜ Run the Angular frontend tests through the Angular ng test builder and verify the new converter, session, dashboard, expenses, agent, Copilot, and settings suites pass. — Angular ng test was skipped because ng was unavailable.
  • ⬜ Open /convert, /dashboard, /expenses, and /agent in a browser and verify the converter loads, displays rates, and does not alter Actuo expense totals. — Browser verification was not executed.
  • ⬜ In a browser with WebMCP cross-origin testing enabled, verify the agent page discovers convertCurrency, the Copilot can invoke it, and the tool disappears after the last converter surface is closed. — WebMCP browser verification was not executed.
  • ⬜ Configure a converter URL containing both query parameters and a hash fragment, reload the embedded converter, and verify the handshake is received and discovery succeeds. — Embedded converter handshake verification was not executed.
  • ⬜ Use an organization with more than EXPENSE_PAGE_MAX expenses and verify dashboard totals and generated CSV reports include all matching rows. — Large-organization totals and CSV verification was not executed.
  • ⬜ Verify the legacy partner-demo assets are no longer required by the build or deployment output. — Build and deployment asset verification was not executed.

⬜ items could not be verified automatically and still need a manual check.

@autter-dev

autter-dev Bot commented Sep 3, 2026

Copy link
Copy Markdown

Autter found 9 issue(s) it could not attach to the current diff (the anchor line is not part of a diff hunk, or the PR advanced during the review):

🔴 [ai] Remote WebMCP tools bypass the mutation confirmation gate (risk 86/100)

frontend/src/app/copilot/copilot.ts:234 · cross_file_consumer_not_updated

runTool() identifies a remote descriptor at line 211, but mutates is false for every tool without a local contract and the confirmation check here only tests contract?.requiresConfirmation. A configured converter can therefore expose a mutating or sensitive cross-origin tool, have Gemini select it, and reach invokeCrossOrigin() without an in-chat Confirm/Cancel step. Remote metadata must not be treated as a verified safety policy; cross-origin calls should be considered mutating by default and gated before execution (or restricted by a separately verified allowlist).

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/copilot/copilot.ts, frontend/src/app/webmcp/tool-registry.ts, frontend/src/app/pages/agent/agent.ts

🔴 [ai] Cross-origin tools execute without the required confirmation gate (risk 86/100)

frontend/src/app/copilot/copilot.ts:234 · auth_route_protection_changed

The Copilot's confirmation policy is only applied when a local Actuo contract exists and declares requiresConfirmation. A runtime-discovered cross-origin tool has no local contract, so runTool() skips the gate and invokes it immediately at line 246. The converter URL and its exposed WebMCP descriptors are runtime-configured external authority; a configured site can expose a mutating or sensitive tool, and Gemini can execute it without an in-chat Confirm/Cancel step. This weakens the authorization boundary for every remote tool, regardless of its annotations.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/copilot/copilot.ts, frontend/src/app/webmcp/tool-registry.ts

🔴 [ai] Cross-origin tools execute without confirmation (risk 86/100)

frontend/src/app/copilot/copilot.ts:234 · sec_missing_input_validation

A Gemini-selected tool from the configured cross-origin converter is executed without the in-chat confirmation required for mutating or sensitive capabilities. runTool() derives mutates as false for every tool without a local contract, and the only confirmation check is contract?.requiresConfirmation; remote descriptors are not contracts, so any remotely exposed tool reaches invokeCrossOrigin() without user approval. A compromised or unexpectedly changed converter could therefore perform a state-changing action under the user's browser session with no confirmation gate.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/copilot/copilot.ts

🔴 [ai] Require confirmation before invoking cross-origin tools (risk 86/100)

frontend/src/app/copilot/copilot.ts:234 · code_correctness

The confirmation gate checks only contract?.requiresConfirmation, but cross-origin descriptors have no local contract. A tool discovered from the configured converter, including a mutating or sensitive tool, therefore reaches invokeCrossOrigin() immediately with no user confirmation; its descriptor annotations do not enforce the policy. This is reachable whenever discovery returns a remote tool and Gemini selects it, and the UI simultaneously labels such a tool as non-mutating because mutates is false when contract is absent. Cross-origin calls must be treated as mutating by default and gated before execution unless a separately verified allowlist establishes a read-only policy.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/copilot/copilot.ts

🟠 [ai] In-flight remote discovery can restore tools after converter teardown (risk 78/100)

frontend/src/app/converter/converter-session.ts:183 · auth_route_protection_changed

rediscover() awaits an uncancellable Copilot discovery request without retaining the mount generation that authorized it. A discovery started while a converter is mounted can remain pending; the final release then calls teardown() and clearRemoteTools(), but when the request resolves, discoverRemoteTools() writes the descriptors back into the Copilot even though no converter frame remains. The Copilot can subsequently offer and invoke a tool owned by a destroyed document, leaving stale executable authority in the session until another discovery changes it.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts, frontend/src/app/copilot/copilot.ts

🟠 [ai] Discovery can repopulate remote tools after converter teardown (risk 78/100)

frontend/src/app/converter/converter-session.ts:183 · sec_missing_input_validation

The discovery started by rediscover() is not tied to the lifetime of the mount that started it. If discoverRemoteTools() is still pending when the final release() calls teardown() and clears the Copilot's remote tools, the pending promise can later resolve and commit descriptors for the destroyed iframe. The Copilot then offers convertCurrency despite having no live converter document to execute it against, producing stale capability exposure and failed calls until another lifecycle event corrects the state.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts

🟠 [ai] In-flight converter discovery can repopulate tools after teardown (risk 78/100)

frontend/src/app/converter/converter-session.ts:183 · silent_exception_swallowing

This is not safe to ship because discovery is started as fire-and-forget work from acquire(), toolchange, and frame-load callers, but its completion is not tied to the mount that started it. If getTools() is still pending when the last release reaches teardown() and clearRemoteTools(), the await below can later resolve and discoverRemoteTools() writes descriptors back into Copilot even though the converter iframe/document is gone. Subsequent Gemini turns then advertise convertCurrency and attempt to invoke a stale cross-origin tool instead of omitting it. Teardown needs to invalidate a generation and rediscover must verify that the same generation is still mounted before committing the result; supported underlying requests should also be aborted.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts, frontend/src/app/copilot/copilot.ts

🟠 [ai] Converter handshake is appended inside URL fragments (risk 62/100)

frontend/src/app/converter/converter-session.ts:114 · sec_missing_input_validation

The frame URL builder chooses ? or & based only on whether the configured URL contains a query, but it does not account for a fragment. For a valid configured URL such as https://converter.example/app#rates, this produces https://converter.example/app#rates?actuo=...; everything after # is a fragment and is not sent as query data to the converter. The converter consequently cannot read the handshake or expose its tools, so cross-origin discovery silently fails for URLs with hashes.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: frontend/src/app/converter/converter-session.ts

🟠 [ai] SEO verification treats missing or error responses as stamped (risk 58/100)

scripts/verify-deploy.mjs:83 · silent_exception_swallowing

The deployment smoke check can report a broken deploy as healthy for sitemap.xml or robots.txt. In the loop below, get(path) returns status and body, but only the body is inspected; a 404/500 response with any non-empty body that lacks the sentinel falls through to pass(), and an empty body is silently skipped. Thus a missing or failing SEO asset is not surfaced by the required verification command, leaving the production failure path unreported even though the script claims the deploy passed.

⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:

  • Dependent files: scripts/verify-deploy.mjs

@theprogrammersingh
theprogrammersingh merged commit 71c3371 into main Sep 3, 2026
2 of 4 checks passed

@autter-dev autter-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autter posted 4 finding(s) as review threads below (🟡 4). Each carries a copy-paste AI fix prompt.

Comment thread Progress.md
| **WebMCP** | Works in flag-enabled Chrome (`chrome://flags/#enable-webmcp-testing`) **and** still works with the flag off |
| **Money / totals** | The number is right on a dataset larger than one page (100 rows) — truncation shows a wrong figure, not an obvious gap |
| **UI** | Both themes, phone and desktop widths, keyboard reachable |
| **Anything deployed** | `pnpm run verify:deploy <url>`. Local green does not mean deployed correct — SSR fell back to CSR in production while every test passed, and the page looked fine |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 189

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:71: Line length: Expected: 80; Actual: 189

Flagged by Autter security & observability checks.

Comment thread Progress.md
| llms.txt | ✅ | Accurate tool inventory and permission model |
| OG / Twitter | ✅ | 1200×630 `og.png` generated from the brand tokens, plus `og:url`, `og:image:alt`, `twitter:image` and a canonical link |
| SSR on public pages | 🟡 | `app.routes.server.ts` prerenders `**` — including authenticated routes, which land on the app shell and hydrate client-side (correct for a gated view, accidental rather than chosen). **Was silently broken until 2026-08-29:** Angular 21's `Host` allowlist rejected every request and fell back to CSR, discarding the SSR entirely. Fixed via `security.allowedHosts` + `NG_ALLOWED_HOSTS`; the check is that `/` contains `ng-server-context` |
| SSR on public pages | 🟡 ⚠️ | `app.routes.server.ts` prerenders `**` — including authenticated routes, which land on the app shell and hydrate client-side (correct for a gated view, accidental rather than chosen). **Broken on the deployed site as of 2026-09-03:** `/` carries no `ng-server-context`, so Angular is falling back to CSR there and discarding the SSR entirely. It works locally, which is exactly how it went unnoticed — the same silent failure as 2026-08-29. `NG_ALLOWED_HOSTS` must be set on the *service*, not only in `render.yaml` and the Dockerfile. `pnpm run verify:deploy <url>` now checks it |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 614

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:270: Line length: Expected: 80; Actual: 614

Flagged by Autter security & observability checks.

Comment thread Progress.md
| Item | Status | Notes |
|---|---|---|
| **Public deployed URL** | 🟡 | The deploy path is **built and verified locally**: `server.mjs` composes Nest under `/api` with the Angular SSR handler, and a `Dockerfile` builds and runs it. Both builder stages plus the runtime boot were simulated locally — `/api/health` 200, `/api/*` 404 as JSON, `ng-server-context` present. The target is Render via `render.yaml`; Firebase App Hosting was abandoned after three distinct buildpack failures against this workspace monorepo (see README *Why a Dockerfile*). What is left is creating the Render Blueprint, which needs an interactive login and the three secrets |
| **Public deployed URL** | 🟡 | **Live at `https://actuo.onrender.com`** — `/api/health` returns 200. `server.mjs` composes Nest under `/api` with the Angular SSR handler from a committed `Dockerfile`; Firebase App Hosting was abandoned after three distinct buildpack failures against this workspace monorepo (see README *Why a Dockerfile*). Still 🟡, not ✅, because the deployed site is **defective in two ways**: `/` does not server-render (see §8.5) and it served a literal `__PUBLIC_ORIGIN__` in `canonical`/`og:image`. The stamp half is fixed in `scripts/stamp-seo.mjs` and needs a redeploy; the SSR half needs `NG_ALLOWED_HOSTS` set on the service. `pnpm run verify:deploy <url>` reports both |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 701

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:291: Line length: Expected: 80; Actual: 701

Flagged by Autter security & observability checks.

Comment thread Progress.md
| README | ✅ | Root `README.md`: what is WebMCP-specific and where, the flag setup, what works without it, and the deploy steps. Workspace READMEs are still starter boilerplate |
| Demo video | ⬜ | The script is the "What to look at" list in `README.md` |
| Demo video | ⬜ | — |
| Demo video | ⬜ | The script is the "What to look at" list in `README.md`. Worth filming only after the SSR fix lands, or it records the client-rendered site |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [deterministic] markdownlint: MD013 — Risk: 30/100

Line length: Expected: 80; Actual: 160

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the markdownlint `MD013` issue at Progress.md:293: Line length: Expected: 80; Actual: 160

Flagged by Autter security & observability checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant