add v1 of freighter extension best practices skill#2687
Open
leofelix077 wants to merge 4 commits intomasterfrom
Open
add v1 of freighter extension best practices skill#2687leofelix077 wants to merge 4 commits intomasterfrom
leofelix077 wants to merge 4 commits intomasterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a first version of a “Freighter best practices” skill/doc bundle to codify engineering conventions for the Freighter extension repo (architecture, messaging, i18n, security, testing, etc.), plus a lightweight repo entry-point index.
Changes:
- Add
llms.txtas a quick link index into key Freighter docs. - Add
docs/skills/freighter-best-practices/skill definition + reference guides (code style, architecture, messaging, security, testing, etc.). - Update
CLAUDE.md“AI Agent Context” documentation.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| llms.txt | Adds a concise index of key docs/entry points for agents and contributors. |
| docs/skills/freighter-best-practices/SKILL.md | Defines the skill and links to reference guides by concern. |
| docs/skills/freighter-best-practices/references/testing.md | Documents Jest/Playwright testing conventions and commands. |
| docs/skills/freighter-best-practices/references/security.md | Documents key storage, message validation, CSP, and common security pitfalls. |
| docs/skills/freighter-best-practices/references/performance.md | Captures performance guidance and common optimization patterns/anti-patterns. |
| docs/skills/freighter-best-practices/references/messaging.md | Documents popup/background/content-script message flows and handler patterns. |
| docs/skills/freighter-best-practices/references/i18n.md | Documents react-i18next usage, locale files, and translation build workflow. |
| docs/skills/freighter-best-practices/references/git-workflow.md | Documents branch/commit/PR expectations and release workflow overview. |
| docs/skills/freighter-best-practices/references/error-handling.md | Documents thunk + background handler error-handling patterns and Sentry usage. |
| docs/skills/freighter-best-practices/references/dependencies.md | Documents workspace layout and dependency placement/upgrade workflow. |
| docs/skills/freighter-best-practices/references/code-style.md | Documents Prettier/ESLint expectations and TS/React style conventions. |
| docs/skills/freighter-best-practices/references/architecture.md | Summarizes runtime contexts, state management patterns, and key extension conventions. |
| docs/skills/freighter-best-practices/references/anti-patterns.md | Lists common pitfalls (deps suppression, non-null assertions, module state in bg, etc.). |
| CLAUDE.md | Updates AI-agent context and repo conventions summary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CLAUDE.md
Outdated
Comment on lines
+124
to
+126
| 1. `pretty-quick --staged` — Prettier on staged files | ||
| 2. `lint-staged` — ESLint fix on staged `.ts`/`.tsx` | ||
| 3. Translation build — auto-generates `extension/src/popup/locales/` files |
There was a problem hiding this comment.
The pre-commit hook description here doesn’t match the repo’s actual Husky hook. .husky/pre-commit runs ./.husky/addTranslations.sh and pretty-quick --staged only; it does not run lint-staged, and the translation build happens before formatting. Please update this section to reflect the current hook (or update the hook to match this doc).
Suggested change
| 1. `pretty-quick --staged` — Prettier on staged files | |
| 2. `lint-staged` — ESLint fix on staged `.ts`/`.tsx` | |
| 3. Translation build — auto-generates `extension/src/popup/locales/` files | |
| 1. `./.husky/addTranslations.sh` — translation build that auto-generates | |
| `extension/src/popup/locales/` files | |
| 2. `pretty-quick --staged` — Prettier on staged files |
CLAUDE.md
Outdated
| - **Translations:** All user-facing strings wrapped in `t()` from | ||
| `react-i18next`. Run `yarn build:extension:translations` to generate keys. See | ||
| `extension/LOCALIZATION.MD`. | ||
| - **Imports:** External packages first, then internal. ESLint enforces ordering. |
There was a problem hiding this comment.
This line says ESLint enforces import ordering, but the current eslint.config.js does not configure an import/order (or equivalent) rule—import ordering is a convention only. Please reword to avoid implying it’s enforced.
Suggested change
| - **Imports:** External packages first, then internal. ESLint enforces ordering. | |
| - **Imports:** External packages first, then internal; follow this convention | |
| consistently. |
|
|
||
| ```typescript | ||
| // CORRECT | ||
| t("Send {{amount}} XLM", { amount: "100" })// WRONG |
There was a problem hiding this comment.
The example code block merges the CORRECT and WRONG examples onto the same line (...)// WRONG), which makes the snippet confusing and syntactically invalid. Split this into separate lines and clearly label the WRONG example as its own line/block.
Suggested change
| t("Send {{amount}} XLM", { amount: "100" })// WRONG | |
| t("Send {{amount}} XLM", { amount: "100" }); | |
| // WRONG |
Merge CLAUDE.md into AGENTS.md: add documentation link index at the top (llms.txt style), full quick reference table, repository structure, architecture details, conventions, and known complexity from CLAUDE.md. Best practices entry points table kept at the bottom. Remove CLAUDE.md and update llms.txt to point to AGENTS.md.
Rewrite AGENTS.md as the single AI agent entry point: - Add glossary section with domain-specific terminology - Add documentation link index (replaces llms.txt) - Remove sections that duplicate best-practices reference files (code style details, branch conventions, PR instructions) - Keep unique context: env setup, repo map, architecture orientation, security alert list, known complexity/gotchas, pre-submission checklist - Delete llms.txt (content absorbed into AGENTS.md) - Delete CLAUDE.md (already removed in previous commit)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skill Eval:
freighter-best-practices— Benchmark ReportDate: 2026-04-09
Iterations: 1
Repo: stellar/freighter
Model: Claude Opus 4.6
Summary
on further passes both get close to 100%, as Claude picks up automatically on the rules and the entry points
Assertion Results (aggregated across 1 iterations)
anti-patterns-refactor
architecture-redux-duck
architecture-state-decision
code-style-scss
code-style-todo-magic
err-handling-bg-handler
err-handling-thunk
i18n-settings
messaging-service-type
performance-list
performance-selectors
security-ext-msg
security-key-export
testing-thunk