Feature/share presentation skeleton - #23
JosueBrenes merged 5 commits into
Conversation
|
@KevinMB0220 is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a complete credential-sharing and verification flow for the credit-history app. It adds shared TypeScript interfaces and a mock ChangesCredit History Share & Verify Flow
Sequence DiagramssequenceDiagram
participant User
participant SharePage
participant MockCreditCredentialSource
participant VerifyPage
rect rgba(100, 149, 237, 0.5)
Note over User,MockCreditCredentialSource: Share flow
SharePage->>MockCreditCredentialSource: listCredentials() + getProfileSummary()
MockCreditCredentialSource-->>SharePage: credentials[], profile
SharePage->>SharePage: auto-select valid credentials
User->>SharePage: select credentials + set expiration
User->>SharePage: click Generate
SharePage->>SharePage: JSON-stringify {ids, exp} → base64url encode
SharePage-->>User: /verify/{token} URL shown in CopyField
end
rect rgba(144, 238, 144, 0.5)
Note over User,VerifyPage: Verify flow
User->>VerifyPage: navigate /verify/{token}
VerifyPage->>VerifyPage: base64url-decode → validate ids + exp
VerifyPage->>MockCreditCredentialSource: listCredentials() + getProfileSummary()
MockCreditCredentialSource-->>VerifyPage: credentials[], profile
VerifyPage->>VerifyPage: filter by ids → derive presentationState
VerifyPage-->>User: status banner + profile card + credentials accordion
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/types/src/index.ts (1)
25-25: ⚡ Quick winAvoid
anyin shared contract claims typing.
claims: Record<string, any>leaksanyacross all consumers and removes type-safety right where this package should be strongest.Suggested change
- claims: Record<string, any>; + claims: Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/types/src/index.ts` at line 25, The claims property in the Record type definition uses any as its value type, which weakens type safety throughout all consuming code. Replace the any type with a properly defined type or interface that accurately represents what claims should contain. This ensures type-safety is maintained across the package boundary and provides clear documentation of the claims contract to all consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/credit-history/src/app/page.tsx`:
- Around line 22-30: The page.tsx file contains hardcoded user-facing strings
embedded directly in JSX throughout the component, which bypasses the i18n
translation pipeline and prevents proper localization. Extract all hardcoded
user-facing copy (strings) from the component at
apps/credit-history/src/app/page.tsx lines 22-30 (anchor), 41-44, 47-47, 55-56,
68-72, 75-75, 79-79, 82-82, 91-91, and 96-96 (siblings), and move these strings
to your i18n translation system. Import and use the appropriate translation
function (such as a useTranslation hook or similar) to reference these strings
from the translation files instead of hardcoding them directly in the JSX,
ensuring all user-facing text follows the new translation pipeline.
In `@apps/credit-history/src/app/share/page.tsx`:
- Line 150: Replace all hardcoded user-facing text strings throughout the share
page with i18n key references. In apps/credit-history/src/app/share/page.tsx at
lines 150, 162, 191-201, 212, 264, 296-300, and 310-315, identify each hardcoded
string and replace it with the appropriate i18n key lookup using your
localization library (such as using t() function or similar). Ensure that every
visible user-facing string that was previously hardcoded now flows through the
i18n system to support proper localization across all languages.
- Around line 61-65: The catch blocks at lines 61-65 and 146-153 in the
credentials loading logic only log errors to console without setting any error
state that the UI can display. To fix this, introduce an error state variable
(such as credentialsError or similar) that gets set in both catch blocks with
the error details. Then update the component's render logic to display this
error state visibly to users (for example, with an error message and retry
button) so they can understand the failure and attempt recovery. This ensures
both credential-load failure locations provide users with a proper recoverable
error experience instead of silently failing.
- Line 373: The min attribute on the datetime-local input uses toISOString()
which returns UTC time, but the datetime-local input type expects local time
values, causing a timezone mismatch. Replace the new
Date().toISOString().slice(0, 16) expression with a local time formatter that
constructs the datetime string in YYYY-MM-DDTHH:mm format using the local
timezone (year, month, day, hours, and minutes from the Date object without UTC
conversion), ensuring the min constraint properly reflects the user's local
time.
- Line 95: The code at line 95 does not validate customExpDate before converting
it to a timestamp, which allows invalid or malformed dates to return NaN that
silently becomes null during JSON serialization on line 123, effectively
removing the expiration constraint. Add explicit validation before line 95 to
ensure customExpDate is a valid, finite future timestamp. Check that new
Date(customExpDate).getTime() produces a finite number greater than the current
time, and reject or return an error if the date is invalid or in the past. This
validation must occur before the token is generated, ensuring malformed dates
fail validation rather than silently bypassing expiration.
In `@apps/credit-history/src/app/verify/`[token]/page.tsx:
- Around line 131-133: The verify page component contains hardcoded user-visible
strings that bypass the i18n/dictionary localization system, breaking
localization consistency. Replace the hardcoded string "Running cryptographic
verification..." in the verification status message and "Share a New
Presentation" with dictionary-backed translations using the existing i18n system
to maintain complete localization coverage throughout the page.
- Around line 71-82: The token expiration validation in the expiration check
block is too permissive for untrusted input from public tokens. Replace the
loose truthiness check on payload.exp with strict schema validation that ensures
exp is either null or a finite number (rejecting falsy numeric values like 0,
and non-numeric values). Add validation logic after the initial payload
validation to check that exp is either null or typeof number and
Number.isFinite(payload.exp) before using it. If the exp value fails this strict
validation (is not null and not a finite number), treat it the same as other
schema violations by setting the error message and returning early.
In `@apps/credit-history/src/lib/i18n.ts`:
- Around line 12-13: The code in the replacements iteration loop constructs a
RegExp from the dynamic placeholder key k, which is unsafe because regex-special
characters in k will be misinterpreted. Replace the RegExp-based replacement
approach with a literal-string replacement strategy: instead of using new RegExp
to match {k}, use the split/join pattern where you split the value string on the
literal placeholder string {k} and then join with the replacement value
String(v). This ensures placeholder keys are treated as literal strings
regardless of their content.
In `@packages/acta/src/mock.ts`:
- Around line 101-105: The mock source methods are returning direct references
to shared in-memory objects like mockCreditCredentials, which allows downstream
code to mutate the global singleton state. Modify the listCredentials method
(and the other affected methods at lines 108-113 and 117-120) to return
defensive deep copies of the mock data objects instead of returning the original
references directly. This prevents mutations of returned credentials or profile
data from corrupting the global mock state for subsequent calls.
In `@packages/ui/src/components/copy-field.tsx`:
- Around line 38-54: The Button component in the copy-field.tsx file that
renders only an icon (Check or Copy) lacks an accessible label for screen reader
users. Add an aria-label attribute to the Button element that describes the
button's action. The label should indicate the copy-to-clipboard functionality
and ideally reflect the current state using the copied boolean variable, for
example "Copy to clipboard" when not copied and "Copied to clipboard" when
copied.
- Line 21: The setTimeout that resets the copied state is never cleared, which
can cause a state update warning after the component unmounts. Wrap the
setTimeout in a useEffect hook with a cleanup function that clears the timeout
using clearTimeout when the component unmounts, ensuring the timeout is
cancelled if the component is removed before the 2000ms delay completes.
---
Nitpick comments:
In `@packages/types/src/index.ts`:
- Line 25: The claims property in the Record type definition uses any as its
value type, which weakens type safety throughout all consuming code. Replace the
any type with a properly defined type or interface that accurately represents
what claims should contain. This ensures type-safety is maintained across the
package boundary and provides clear documentation of the claims contract to all
consumers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54f840bf-a098-4c32-b87e-b44e6ede15eb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/credit-history/package.jsonapps/credit-history/src/app/page.tsxapps/credit-history/src/app/share/page.tsxapps/credit-history/src/app/verify/[token]/page.tsxapps/credit-history/src/dictionaries/en.jsonapps/credit-history/src/lib/i18n.tspackages/acta/package.jsonpackages/acta/src/index.tspackages/acta/src/mock.tspackages/acta/src/types.tspackages/types/src/index.tspackages/ui/src/components/badge.tsxpackages/ui/src/components/card.tsxpackages/ui/src/components/checkbox.tsxpackages/ui/src/components/copy-field.tsxpackages/ui/src/components/input.tsxpackages/ui/src/components/label.tsxpackages/ui/src/index.ts
| ACTA Protocol | ||
| </Badge> | ||
| <h1 className="text-4xl md:text-5xl font-extrabold tracking-tight bg-gradient-to-r from-foreground via-muted-foreground to-foreground bg-clip-text text-transparent"> | ||
| Credit History | ||
| </h1> | ||
| <p className="text-muted-foreground text-base md:text-lg"> | ||
| Own, manage, and share your verifiable credit reputation. Port your financial history | ||
| securely without relying on centralized credit bureaus. | ||
| </p> |
There was a problem hiding this comment.
Move landing-page copy into i18n instead of hardcoding strings.
This page bypasses the new translation pipeline by embedding user-facing copy directly in JSX, which diverges from the PR’s i18n objective and makes localization incomplete.
Suggested direction
- import { DID_PKH_STELLAR_PREFIX } from '`@acta-products/acta/did`';
+ import { DID_PKH_STELLAR_PREFIX } from '`@acta-products/acta/did`';
+ import { t } from '`@/lib/i18n`';
...
- ACTA Protocol
+ {t('home.badge')}
- Credit History
+ {t('home.title')}
- Own, manage, and share your verifiable credit reputation...
+ {t('home.subtitle')}
- <CardTitle className="text-xl">Generate Presentation</CardTitle>
+ <CardTitle className="text-xl">{t('home.share.title')}</CardTitle>
- Get Started
+ {t('home.share.cta')}
- <p>Built with Next.js 16, React 19, and Tailwind CSS v4.</p>
+ <p>{t('home.footer.stack')}</p>Also applies to: 41-44, 47-47, 55-56, 68-72, 75-75, 79-79, 82-82, 91-91, 96-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/page.tsx` around lines 22 - 30, The page.tsx file
contains hardcoded user-facing strings embedded directly in JSX throughout the
component, which bypasses the i18n translation pipeline and prevents proper
localization. Extract all hardcoded user-facing copy (strings) from the
component at apps/credit-history/src/app/page.tsx lines 22-30 (anchor), 41-44,
47-47, 55-56, 68-72, 75-75, 79-79, 82-82, 91-91, and 96-96 (siblings), and move
these strings to your i18n translation system. Import and use the appropriate
translation function (such as a useTranslation hook or similar) to reference
these strings from the translation files instead of hardcoding them directly in
the JSX, ensuring all user-facing text follows the new translation pipeline.
| } catch (err) { | ||
| console.error('Failed to load credentials from mock source', err); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
There was a problem hiding this comment.
Credential-load failure is non-recoverable in the UI.
On fetch failure (Line 61-63), the page only logs to console and drops into the normal screen. This leaves users without a visible error state or retry action, which breaks the required recoverable error UX.
Also applies to: 146-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/share/page.tsx` around lines 61 - 65, The catch
blocks at lines 61-65 and 146-153 in the credentials loading logic only log
errors to console without setting any error state that the UI can display. To
fix this, introduce an error state variable (such as credentialsError or
similar) that gets set in both catch blocks with the error details. Then update
the component's render logic to display this error state visibly to users (for
example, with an error message and retry button) so they can understand the
failure and attempt recovery. This ensures both credential-load failure
locations provide users with a proper recoverable error experience instead of
silently failing.
| case '30d': | ||
| return now + 30 * 24 * 60 * 60 * 1000; | ||
| case 'custom': | ||
| return customExpDate ? new Date(customExpDate).getTime() : null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/credit-history/src/app/share/page.tsx | sed -n '85,120p'Repository: ACTA-Team/products-acta
Length of output: 1375
🏁 Script executed:
cat -n apps/credit-history/src/app/share/page.tsx | head -100 | tail -30Repository: ACTA-Team/products-acta
Length of output: 1080
🏁 Script executed:
cat -n apps/credit-history/src/app/share/page.tsx | sed -n '115,140p'Repository: ACTA-Team/products-acta
Length of output: 1140
🏁 Script executed:
rg "customExpDate" apps/credit-history/src/app/share/page.tsx -B 3 -A 3Repository: ACTA-Team/products-acta
Length of output: 880
🏁 Script executed:
node <<'JS'
// Test JSON.stringify behavior with NaN
const payload1 = { exp: NaN };
const payload2 = { exp: null };
const payload3 = { exp: 1234567890 };
console.log("With NaN:", JSON.stringify(payload1));
console.log("With null:", JSON.stringify(payload2));
console.log("With valid number:", JSON.stringify(payload3));
// Also test what new Date with invalid input returns
console.log("\nnew Date('invalid').getTime():", new Date('invalid').getTime());
console.log("new Date('').getTime():", new Date('').getTime());
JSRepository: ACTA-Team/products-acta
Length of output: 216
Validate customExpDate before using—invalid dates silently become "no expiration."
Line 95 can return NaN for malformed customExpDate; when serialized by JSON.stringify() on line 123, that becomes null, effectively removing expiration instead of failing validation. Although the input uses type="datetime-local", this provides only browser-side validation and cannot be relied upon. Add explicit validation to ensure customExpDate is a finite future timestamp before generating the token.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/share/page.tsx` at line 95, The code at line 95
does not validate customExpDate before converting it to a timestamp, which
allows invalid or malformed dates to return NaN that silently becomes null
during JSON serialization on line 123, effectively removing the expiration
constraint. Add explicit validation before line 95 to ensure customExpDate is a
valid, finite future timestamp. Check that new Date(customExpDate).getTime()
produces a finite number greater than the current time, and reject or return an
error if the date is invalid or in the past. This validation must occur before
the token is generated, ensuring malformed dates fail validation rather than
silently bypassing expiration.
| return ( | ||
| <div className="flex flex-1 flex-col items-center justify-center min-h-[500px] gap-4"> | ||
| <RefreshCw className="size-8 animate-spin text-primary" /> | ||
| <p className="text-sm text-muted-foreground animate-pulse">Loading vault data...</p> |
There was a problem hiding this comment.
Share page still has hardcoded user-facing text.
There are multiple non-localized strings (for example Line 150, Line 162, Line 191, Line 264, Line 296, Line 310). This violates the localization objective that all visible copy should flow through i18n keys.
Also applies to: 162-162, 191-201, 212-212, 264-264, 296-300, 310-315
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/share/page.tsx` at line 150, Replace all
hardcoded user-facing text strings throughout the share page with i18n key
references. In apps/credit-history/src/app/share/page.tsx at lines 150, 162,
191-201, 212, 264, 296-300, and 310-315, identify each hardcoded string and
replace it with the appropriate i18n key lookup using your localization library
(such as using t() function or similar). Ensure that every visible user-facing
string that was previously hardcoded now flows through the i18n system to
support proper localization across all languages.
| type="datetime-local" | ||
| value={customExpDate} | ||
| onChange={(e) => setCustomExpDate(e.target.value)} | ||
| min={new Date().toISOString().slice(0, 16)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "page.tsx" -path "*/credit-history/src/app/share/*" 2>/dev/null | head -5Repository: ACTA-Team/products-acta
Length of output: 49
🏁 Script executed:
git ls-files | grep -E "apps/credit-history.*share.*page\.tsx"Repository: ACTA-Team/products-acta
Length of output: 110
🏁 Script executed:
cat -n apps/credit-history/src/app/share/page.tsx | sed -n '360,390p'Repository: ACTA-Team/products-acta
Length of output: 1690
🌐 Web query:
datetime-local HTML input type UTC vs local time toISOString()
💡 Result:
The HTML input type datetime-local represents a "naive" local date and time [1][2]. It does not include, accept, or store time zone information [2][3][4]. When working with datetime-local and JavaScript, you must handle the conversion between this local format and UTC-based formats carefully [3][4]. Key Concepts: 1. Input Format: The datetime-local input expects and provides values in the format YYYY-MM-DDTHH:mm [2][5][6]. It rejects strings that include time zone offsets or indicators like Z [4]. 2. JavaScript toISOString: The toISOString method produces an ISO 8601 string in UTC (e.g., 2026-06-16T10:00:00.000Z) [5][4]. Because this output contains the Z suffix (indicating UTC) and represents the time in UTC rather than the user's local time, it is incompatible with the datetime-local input [4]. Common Pitfalls: - Assigning UTC to Local: Using a UTC-based time (like one from toISOString or a database) directly in a datetime-local input will cause the browser to display the UTC time rather than the intended local time [7][4]. - Format Mismatches: Browsers will ignore or fail to parse any value containing a Z or time zone offset [4]. Best Practices: - To Display/Set Value: Convert your Date object into the YYYY-MM-DDTHH:mm format manually [5][4]. A common approach is extracting the year, month, day, hours, and minutes, then padding them to ensure two digits [5][4]: const pad = (n) => n.toString.padStart(2, '0'); const format = ${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}; - To Retrieve/Store Value: When reading from the input, treat the string as the user's local time [3]. To send this to a backend as UTC, pass the string to the Date constructor—which interprets it in the browser's local time zone—and then call toISOString to convert that local instant into a UTC-based string for storage [7][3].
Citations:
- 1: https://w3c.github.io/html-reference/input.datetime-local.html
- 2: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/datetime-local
- 3: https://playbooks.com/skills/mjunaidca/mjs-agent-skills/datetime-timezone
- 4: https://www.xjavascript.com/blog/assign-javascript-date-to-html5-datetime-local-input/
- 5: https://stackoverflow.com/questions/30166338/setting-value-of-datetime-local-from-date
- 6: https://stackoverflow.com/questions/43329252/how-to-convert-javascript-date-object-to-string-that-is-compatible-with-datetime
- 7: https://stackoverflow.com/questions/71265513/javascript-date-utc-datetime-local
Fix datetime-local input to use local time instead of UTC.
The min attribute on line 373 uses toISOString() which converts to UTC time. The datetime-local input type interprets its min/max values as local time, not UTC. This timezone mismatch blocks valid local date selections or allows unintended past values depending on the timezone offset.
Replace new Date().toISOString().slice(0, 16) with a local time formatter that extracts year, month, day, hours, and minutes without timezone conversion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/share/page.tsx` at line 373, The min attribute on
the datetime-local input uses toISOString() which returns UTC time, but the
datetime-local input type expects local time values, causing a timezone
mismatch. Replace the new Date().toISOString().slice(0, 16) expression with a
local time formatter that constructs the datetime string in YYYY-MM-DDTHH:mm
format using the local timezone (year, month, day, hours, and minutes from the
Date object without UTC conversion), ensuring the min constraint properly
reflects the user's local time.
| <p className="text-sm text-muted-foreground animate-pulse"> | ||
| Running cryptographic verification... | ||
| </p> |
There was a problem hiding this comment.
Verify page still renders hardcoded copy outside i18n.
Line 132 (Running cryptographic verification...) and Line 357 (Share a New Presentation) are user-visible hardcoded strings. These should be dictionary-backed to keep localization complete.
Also applies to: 357-357
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/app/verify/`[token]/page.tsx around lines 131 - 133,
The verify page component contains hardcoded user-visible strings that bypass
the i18n/dictionary localization system, breaking localization consistency.
Replace the hardcoded string "Running cryptographic verification..." in the
verification status message and "Share a New Presentation" with
dictionary-backed translations using the existing i18n system to maintain
complete localization coverage throughout the page.
| Object.entries(replacements).forEach(([k, v]) => { | ||
| value = value.replace(new RegExp(`{${k}}`, 'g'), String(v)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/credit-history/src/lib/i18n.tsRepository: ACTA-Team/products-acta
Length of output: 625
🏁 Script executed:
rg -A 5 -B 5 "Object.entries\(replacements\)" apps/credit-history/src/lib/i18n.tsRepository: ACTA-Team/products-acta
Length of output: 350
🏁 Script executed:
# Search for calls to the i18n function to see what keys are passed
rg -A 2 -B 2 "t\(" apps/credit-history/src --type ts --type js | head -100Repository: ACTA-Team/products-acta
Length of output: 8803
🏁 Script executed:
# Check if there are any tests for the i18n module
find apps/credit-history -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "i18n\|replacements" 2>/dev/nullRepository: ACTA-Team/products-acta
Length of output: 51
🏁 Script executed:
# Search for calls to t() function with replacements parameter
rg "t\(['\"].*['\"].*\{" apps/credit-history/src --type ts --type tsx -A 2 -B 2Repository: ACTA-Team/products-acta
Length of output: 95
🏁 Script executed:
# Search for any usage of the replacements parameter in the t() function
rg -A 5 "const.*=.*t\(" apps/credit-history/src --type ts --type tsx | grep -A 5 "replacements\|{.*}"Repository: ACTA-Team/products-acta
Length of output: 95
🏁 Script executed:
# Check the en.json dictionary file to see what placeholder patterns are used
cat apps/credit-history/src/dictionaries/en.jsonRepository: ACTA-Team/products-acta
Length of output: 3049
🏁 Script executed:
# Search for calls to t() with replacements (using a broader pattern)
rg "t\(" apps/credit-history/src -A 2 | grep -B 2 -E "replacements|\{.*\}:"Repository: ACTA-Team/products-acta
Length of output: 543
🏁 Script executed:
# Look for test files and their content
find apps/credit-history -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.js" -o -name "*.spec.js" \)Repository: ACTA-Team/products-acta
Length of output: 49
🏁 Script executed:
# Search for specific pattern of t() calls with second parameter (replacements)
rg "t\(['\"][^'\"]+['\"],\s*\{" apps/credit-history/src -A 1Repository: ACTA-Team/products-acta
Length of output: 49
🏁 Script executed:
# Search for actual calls to t() with replacements object to see what keys are used
rg "t\(" apps/credit-history/src --type ts -B 2 -A 2 | grep -B 5 -A 5 "replacements\|{\s*[a-zA-Z]"Repository: ACTA-Team/products-acta
Length of output: 19113
🏁 Script executed:
# Look at the page.tsx file more carefully to see how t() is called with replacements
cat -n apps/credit-history/src/app/verify/[token]/page.tsx | grep -B 5 -A 5 "t("Repository: ACTA-Team/products-acta
Length of output: 12376
Avoid building RegExp from runtime placeholder keys.
Line 13 constructs new RegExp(\{${k}}`, 'g') from dynamic input, which can misbehave or throw for regex-significant characters. Use a literal-string replacement strategy (split/join`) for placeholders.
Proposed change
if (replacements) {
Object.entries(replacements).forEach(([k, v]) => {
- value = value.replace(new RegExp(`{${k}}`, 'g'), String(v));
+ value = value.split(`{${k}}`).join(String(v));
});
}🧰 Tools
🪛 ast-grep (0.43.0)
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp({${k}}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/credit-history/src/lib/i18n.ts` around lines 12 - 13, The code in the
replacements iteration loop constructs a RegExp from the dynamic placeholder key
k, which is unsafe because regex-special characters in k will be misinterpreted.
Replace the RegExp-based replacement approach with a literal-string replacement
strategy: instead of using new RegExp to match {k}, use the split/join pattern
where you split the value string on the literal placeholder string {k} and then
join with the replacement value String(v). This ensures placeholder keys are
treated as literal strings regardless of their content.
Source: Linters/SAST tools
| async listCredentials(): Promise<CreditCredential[]> { | ||
| // Simulate slight API delay for fidelity | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => resolve(mockCreditCredentials), 100); | ||
| }); |
There was a problem hiding this comment.
Return defensive copies from mock source methods.
These methods return shared in-memory references. Any downstream mutation of returned credentials/profile mutates global singleton state and can corrupt later reads.
Suggested change
class MockCreditCredentialSource implements CreditCredentialSource {
async listCredentials(): Promise<CreditCredential[]> {
// Simulate slight API delay for fidelity
return new Promise((resolve) => {
- setTimeout(() => resolve(mockCreditCredentials), 100);
+ setTimeout(() => resolve(mockCreditCredentials.map((c) => ({ ...c, claims: { ...c.claims } }))), 100);
});
}
async getCredential(id: string): Promise<CreditCredential | null> {
return new Promise((resolve) => {
setTimeout(() => {
const cred = mockCreditCredentials.find((c) => c.id === id);
- resolve(cred || null);
+ resolve(cred ? { ...cred, claims: { ...cred.claims } } : null);
}, 50);
});
}
async getProfileSummary(): Promise<CreditProfileSummary> {
return new Promise((resolve) => {
- setTimeout(() => resolve(mockProfileSummary), 100);
+ setTimeout(() => resolve({ ...mockProfileSummary }), 100);
});
}
}Also applies to: 108-113, 117-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/acta/src/mock.ts` around lines 101 - 105, The mock source methods
are returning direct references to shared in-memory objects like
mockCreditCredentials, which allows downstream code to mutate the global
singleton state. Modify the listCredentials method (and the other affected
methods at lines 108-113 and 117-120) to return defensive deep copies of the
mock data objects instead of returning the original references directly. This
prevents mutations of returned credentials or profile data from corrupting the
global mock state for subsequent calls.
| try { | ||
| await navigator.clipboard.writeText(value); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); |
There was a problem hiding this comment.
Clear copy-feedback timeout on unmount.
The pending timeout is never cleared; this can trigger a state update after unmount.
Suggested change
export function CopyField({ value, placeholder, className }: CopyFieldProps) {
const [copied, setCopied] = React.useState(false);
+ const resetTimerRef = React.useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ React.useEffect(() => {
+ return () => {
+ if (resetTimerRef.current) clearTimeout(resetTimerRef.current);
+ };
+ }, []);
const handleCopy = async () => {
try {
await navigator.clipboard.writeText(value);
setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ if (resetTimerRef.current) clearTimeout(resetTimerRef.current);
+ resetTimerRef.current = setTimeout(() => setCopied(false), 2000);
} catch (err) {
console.error('Failed to copy text: ', err);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setTimeout(() => setCopied(false), 2000); | |
| export function CopyField({ value, placeholder, className }: CopyFieldProps) { | |
| const [copied, setCopied] = React.useState(false); | |
| const resetTimerRef = React.useRef<ReturnType<typeof setTimeout> | null>(null); | |
| React.useEffect(() => { | |
| return () => { | |
| if (resetTimerRef.current) clearTimeout(resetTimerRef.current); | |
| }; | |
| }, []); | |
| const handleCopy = async () => { | |
| try { | |
| await navigator.clipboard.writeText(value); | |
| setCopied(true); | |
| if (resetTimerRef.current) clearTimeout(resetTimerRef.current); | |
| resetTimerRef.current = setTimeout(() => setCopied(false), 2000); | |
| } catch (err) { | |
| console.error('Failed to copy text: ', err); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/copy-field.tsx` at line 21, The setTimeout that
resets the copied state is never cleared, which can cause a state update warning
after the component unmounts. Wrap the setTimeout in a useEffect hook with a
cleanup function that clears the timeout using clearTimeout when the component
unmounts, ensuring the timeout is cancelled if the component is removed before
the 2000ms delay completes.
| <Button | ||
| type="button" | ||
| size="icon" | ||
| variant="outline" | ||
| className={`shrink-0 transition-all duration-200 cursor-pointer ${ | ||
| copied | ||
| ? 'border-emerald-500/50 bg-emerald-500/10 text-emerald-600 dark:text-emerald-400' | ||
| : '' | ||
| }`} | ||
| onClick={handleCopy} | ||
| > | ||
| {copied ? ( | ||
| <Check className="size-4 text-emerald-500 stroke-[3]" /> | ||
| ) : ( | ||
| <Copy className="size-4" /> | ||
| )} | ||
| </Button> |
There was a problem hiding this comment.
Add an accessible name to the icon-only copy button.
The button is icon-only and has no accessible label, so screen-reader users won’t know its action.
Suggested change
<Button
type="button"
size="icon"
variant="outline"
+ aria-label={copied ? 'Copied' : 'Copy link'}
+ title={copied ? 'Copied' : 'Copy link'}
className={`shrink-0 transition-all duration-200 cursor-pointer ${📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| type="button" | |
| size="icon" | |
| variant="outline" | |
| className={`shrink-0 transition-all duration-200 cursor-pointer ${ | |
| copied | |
| ? 'border-emerald-500/50 bg-emerald-500/10 text-emerald-600 dark:text-emerald-400' | |
| : '' | |
| }`} | |
| onClick={handleCopy} | |
| > | |
| {copied ? ( | |
| <Check className="size-4 text-emerald-500 stroke-[3]" /> | |
| ) : ( | |
| <Copy className="size-4" /> | |
| )} | |
| </Button> | |
| <Button | |
| type="button" | |
| size="icon" | |
| variant="outline" | |
| aria-label={copied ? 'Copied' : 'Copy link'} | |
| title={copied ? 'Copied' : 'Copy link'} | |
| className={`shrink-0 transition-all duration-200 cursor-pointer ${ | |
| copied | |
| ? 'border-emerald-500/50 bg-emerald-500/10 text-emerald-600 dark:text-emerald-400' | |
| : '' | |
| }`} | |
| onClick={handleCopy} | |
| > | |
| {copied ? ( | |
| <Check className="size-4 text-emerald-500 stroke-[3]" /> | |
| ) : ( | |
| <Copy className="size-4" /> | |
| )} | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/copy-field.tsx` around lines 38 - 54, The Button
component in the copy-field.tsx file that renders only an icon (Check or Copy)
lacks an accessible label for screen reader users. Add an aria-label attribute
to the Button element that describes the button's action. The label should
indicate the copy-to-clipboard functionality and ideally reflect the current
state using the copied boolean variable, for example "Copy to clipboard" when
not copied and "Copied to clipboard" when copied.
…entation-skeleton # Conflicts: # apps/credit-history/src/app/page.tsx # apps/credit-history/src/app/share/page.tsx # packages/ui/src/index.ts
2 participants
🚀 ACTA Pull Request
Mark with an
xall the checkboxes that apply (like[x])Closes #7
📌 Type of Change
📝 Changes Description
This PR implements the Share credential presentation skeleton for
apps/credit-history. It allows holders to select multiple credentials, set expiration bounds, and generate a secure, self-contained base64url presentation token that can be verified publicly by third-party lenders.Key implementations:
CreditCredential,CreditProfileSummary,CreditCredentialSource) and built a local mock data source representing 5 credit credentials with varying statuses (Valid vs. Revoked).@acta-products/ui): Implemented reusable and responsive widgets (Card,Checkbox,Input,Label,Badge,CopyField) with full Tailwind v4 support.en.jsonand a lightweight translator helpert()to prevent hardcoded strings./share): Built a client-side component to select credentials, configure expiration presets/custom date-times, and generate the URL-safe token. Includes the requested comment SEAM for cryptographic wiring./verify/[token]): Created the public validation layout that parses the presentation token, validates expiration dates (app-levelinvalidstate), performs verification simulations, and displays detailed claims.pnpm typecheck,pnpm lint,pnpm buildpass with zero errors).📸 Evidence
https://www.loom.com/share/fde1efcbacdb47238f6be7197e14f9e2
[Pega aquí tu enlace de Loom o grabación de pantalla demostrando el flujo]
*(💡 Nota para el video: Te sugiero grabar una demostración rápida de:
🌌 Comments
apps/credit-history/src/app/share/page.tsxfor easy replacement when integrating off-chain payloads or ZK proof persistence.Thank you for contributing to ACTA! We hope you can continue contributing to this project.
Summary by CodeRabbit