Feat/currency converter - #3
Conversation
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.
There was a problem hiding this comment.
🔴 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.
| * 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']) { |
There was a problem hiding this comment.
🔴 [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:
- https://csrc.nist.gov/pubs/sp/800/132/final — SP 800-132, Recommendation for Password-Based Key Derivation: Part 1: Storage Applications | CSRC You are viewing this page in an unauthorized frame window. This is a potential sec
- https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf — ### NIST Special Publication 800-132 ## Recommendation for Password-Based Key Derivation Part 1: Storage Applications Meltem Sönmez Turan, Elaine Barker, William Burr, and Lily Che
- https://blog.pcisecuritystandards.org/faq-how-does-encrypted-cardholder-data-impact-pci-dss-scope — FAQ: How does Encrypted Cardholder Data Impact PCI DSS Scope? < Return to Blog Home Print # FAQ: How does Encrypted Cardholder Data Impact PCI DSS Scope? Did you know that the F
⚠ 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.
| 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'; |
There was a problem hiding this comment.
🟠 [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'; |
There was a problem hiding this comment.
🟡 [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.
🚦 Pre-merge checks ·
|
| Check | Status | Explanation |
|---|---|---|
| Too many lines changed | This PR changes 2694 lines (limit 1000). Splitting into smaller, focused PRs makes review more reliable. | |
| Mixed concerns (refactor + behavior change) | 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 | 1 potential issue(s) detected (max risk 45/100): scripts/verify-deploy.mjs:55. | |
| Removed observability | 1 potential issue(s) detected (max risk 45/100): scripts/partner-server.mjs:67. | |
| Possible non-atomic read-modify-write | 1 potential issue(s) detected (max risk 45/100): scripts/verify-deploy.mjs:44. | |
| Optimistic locking not detected | 1 potential issue(s) detected (max risk 40/100): scripts/verify-deploy.mjs:102. | |
| Infrastructure misconfiguration | 1 potential issue(s) detected (max risk 60/100): Dockerfile:77. | |
| PII stored without application-level encryption | 1 potential issue(s) detected (max risk 85/100): frontend/src/app/converter/converter-session.spec.ts:84. | |
| Missing linked tracker issue | 1 potential issue(s) detected (max risk 55/100): backend/src/config/env.service.ts:99. | |
| Missing CODEOWNERS reviewer approval | 1 potential issue(s) detected (max risk 70/100): backend/src/config/env.service.ts:99. | |
| Direct env-var access bypasses config module | 1 potential issue(s) detected (max risk 50/100): scripts/verify-deploy.mjs:28. | |
| Comment contradicts or fabricates code behaviour | 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 | 1 potential issue(s) detected (max risk 55/100): backend/src/config/env.service.ts:102. | |
| Established pattern ignored | 1 potential issue(s) detected (max risk 60/100): frontend/src/app/pages/dashboard/dashboard.ts:15. | |
| Runtime error risk | 1 finding(s) on changed lines. | |
| Resource leak risk | 2 finding(s) on changed lines. | |
| Code duplication / DRY violation | 1 finding(s) on changed lines. | |
| Complexity Guard | 1 finding(s) on changed lines. | |
| Bundle Size Monitor | 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.
🔇 6 finding(s) suppressed as likely false positives by Autter's verification passThese 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.
|
There was a problem hiding this comment.
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.
| async rediscover(): Promise<void> { | ||
| const origin = this.converterOrigin(); | ||
| if (!this.isBrowser || this.mounts() === 0 || !origin || this.isSameOrigin()) return; | ||
| await this.copilot.discoverRemoteTools([origin]); |
There was a problem hiding this comment.
🟠 [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.
| async rediscover(): Promise<void> { | ||
| const origin = this.converterOrigin(); | ||
| if (!this.isBrowser || this.mounts() === 0 || !origin || this.isSameOrigin()) return; | ||
| await this.copilot.discoverRemoteTools([origin]); |
There was a problem hiding this comment.
🟠 [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.
| */ | ||
| acquire(): () => void { | ||
| this.mounts.update((n) => n + 1); | ||
| void this.ensureConfig().then(() => this.rediscover()); |
There was a problem hiding this comment.
🟠 [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())}`; |
There was a problem hiding this comment.
🟠 [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 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)
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.
|
| 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' |
There was a problem hiding this comment.
🟠 [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()); |
There was a problem hiding this comment.
🟠 [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()); |
There was a problem hiding this comment.
🟠 [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'); |
There was a problem hiding this comment.
🟠 [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'); |
There was a problem hiding this comment.
🟠 [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'); |
There was a problem hiding this comment.
🟠 [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); |
There was a problem hiding this comment.
🟠 [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'; | |||
There was a problem hiding this comment.
🟠 [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(); |
There was a problem hiding this comment.
🟠 [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(); |
There was a problem hiding this comment.
🟠 [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.
|
|
||
| 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 |
There was a problem hiding this comment.
🟡 [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.
| 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 |
There was a problem hiding this comment.
🟡 [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.
| - 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. |
There was a problem hiding this comment.
🟡 [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.
| - 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. |
There was a problem hiding this comment.
🟡 [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.
| - 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). |
There was a problem hiding this comment.
🟡 [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.
| | 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` | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
| | **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` | |
There was a problem hiding this comment.
🟡 [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.
There was a problem hiding this comment.
🔴 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.
| 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.'); |
There was a problem hiding this comment.
🟡 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.
| 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' |
There was a problem hiding this comment.
🟠 [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:
- https://www.martinfowler.com/eaaCatalog/repository.html — Repository # Repository Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects. A system with a complex domain model oft
- https://www.martinfowler.com/eaaCatalog/serviceLayer.html — Service Layer # Service Layer Defines an application's boundary with a layer of services that establishes a set of available operations and coordinates the application's response i
⚠ 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'; |
There was a problem hiding this comment.
🟡 [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:
- https://www.martinfowler.com/eaaCatalog/repository.html — Repository # Repository Mediates between the domain and data mapping layers using a collection-like interface for accessing domain objects. A system with a complex domain model oft
- https://www.martinfowler.com/eaaCatalog/serviceLayer.html — Service Layer # Service Layer Defines an application's boundary with a layer of services that establishes a set of available operations and coordinates the application's response i
⚠ 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.
| selector: 'app-dashboard', | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| imports: [Card, EmptyState, ErrorState, Skeleton, StatCard], | ||
| imports: [Card, CurrencyConverter, EmptyState, ErrorState, Skeleton, StatCard], |
There was a problem hiding this comment.
🟡 [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 task list
Generated from PR diff, blast radius, and context. Issues found
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 optionsCheck one option and Autter will start a fix run for the unresolved issues above.
Checking a box triggers the fix run immediately — Autter comments back with the issues being fixed and the branch created for each. |
🧪 Autter test runAutter checked Execution summary: 8 checks executed · 8 passed. Project test commands
Autter targeted verification8 tests executed · 8 passed. Declared tests: 53 test file(s) found — 0 ran, 0 not observed in suite output, 53 did not run.
|
| 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 testbuilder 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/agentin 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_MAXexpenses 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 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)
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).
🔴 [ai] Cross-origin tools execute without the required confirmation gate (risk 86/100)
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.
🔴 [ai] Cross-origin tools execute without confirmation (risk 86/100)
A Gemini-selected tool from the configured cross-origin converter is executed without the in-chat confirmation required for mutating or sensitive capabilities.
🔴 [ai] Require confirmation before invoking cross-origin tools (risk 86/100)
The confirmation gate checks only
🟠 [ai] In-flight remote discovery can restore tools after converter teardown (risk 78/100)
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.
🟠 [ai] Discovery can repopulate remote tools after converter teardown (risk 78/100)
The discovery started by
🟠 [ai] In-flight converter discovery can repopulate tools after teardown (risk 78/100)
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.
🟠 [ai] Converter handshake is appended inside URL fragments (risk 62/100)
The frame URL builder chooses
🟠 [ai] SEO verification treats missing or error responses as stamped (risk 58/100)
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.
|
| | **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 | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
| | 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 | |
There was a problem hiding this comment.
🟡 [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.
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
ConverterSessionmanages 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
CurrencyConverterandConverterSessionimplementations with tests for conversion behavior, iframe mounting, reference-counted lifecycle management, handshake configuration, discovery, and teardown./convertroute and embedded converter surfaces on the dashboard, expenses, settings, and agent tools pages.BASE_CURRENCYandCONVERTER_URL, including development defaults and production opt-in behavior.Acceptance Criteria
/convertroute renders the currency converter and can obtain conversion results without sending the user's Gemini API key to Actuo.EXPENSE_PAGE_MAXby fetching all pages.CONVERTER_URL; development retains the documented fallback behavior.Test Plan
ng testbuilder and verify the new converter, session, dashboard, expenses, agent, Copilot, and settings suites pass./convert,/dashboard,/expenses, and/agentin a browser and verify the converter loads, displays rates, and does not alter Actuo expense totals.convertCurrency, the Copilot can invoke it, and the tool disappears after the last converter surface is closed.EXPENSE_PAGE_MAXexpenses and verify dashboard totals and generated CSV reports include all matching rows.Rollback Plan
CONVERTER_URLin production and redeploy; the converter surfaces should remain unavailable without affecting local Copilot tool execution or expense workflows.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.