feat: adicione verificao de region no updater

[revogabe]

nesse PR adicionamos suporte a region no loadCache via query params

exemplo:

/download/exe?region=us

[kusanagi-claw]

QA Review — feat/check-region

Verdict: 🚫 CHANGES REQUESTED — There are blockers that must be fixed before merge.

---

🔴 Blockers (must fix before merge)

1. .DS_Store committed to src/

src/.DS_Store is a macOS metadata file and has no place in the repo. Additionally, .gitignore doesn't include .DS_Store at all — it will keep polluting commits from every dev working on a Mac.

Fix:

git rm --cached src/.DS_Store
echo '.DS_Store' >> .gitignore

2. No input validation on the region query param

In routes.ts, the region param is cast directly to CacheRegionType without runtime validation:

const { region } = params as { region: CacheRegionType };

The cast is a TypeScript lie — at runtime, region can be any string (e.g., ?region=invalid, ?region=BR, or not present at all).

Since loadCache is implemented as:

return Object.assign({}, region === 'br' ? latest : latestUS);

Any value that isn't 'br' — including 'BR', 'invalid', or an array from ?region=br&region=us — silently falls through to latestUS. This is a silent, invisible logic error with real UX consequences.

Fix: Add a guard before calling loadCache:

const rawRegion = params?.region;
const region: CacheRegionType = rawRegion === 'us' ? 'us' : 'br'; // explicit, documented fallback

Or return a 400 if the region is unrecognized.

3. this[latestByRegion] — dynamic property access breaks TypeScript safety

const latestByRegion = region === 'br' ? 'latest' : 'latestUS';
this[latestByRegion].version = tag_name;

Both latest and latestUS are private — TypeScript will either error or silently type this as any, bypassing all type checking. This entire method becomes a type-safety blind spot.

Fix: Use a conditional or a private getter instead:

const target = region === 'br' ? this.latest : this.latestUS;
target.version = tag_name;
// ...

---

🟡 Warnings (should fix)

4. releaseNotes link is now broken/regressed

Before this PR, releaseNotes pointed to the specific release tag:

https://github.com/account/repo/releases/tag/{version}

After this PR, both releaseNotes and allReleases point to the same URL:

https://github.com/account/repo/releases

The "Release Notes" link in the overview page no longer takes users to the specific release. This is a regression.

5. Duplicate HTML id attributes in index.hbs

When both BR and US sections are rendered, the page contains:

• Two elements with id="version"
• Two elements with id="date"

IDs must be unique per HTML spec. This breaks CSS selectors and any JS that uses document.getElementById. The existing CSS in the file targets #version and #date — only the first match will be styled correctly.

Fix: Change to classes: class="version", class="date".

6. No tests for the US region path

The two existing tests were updated to pass 'br' — fine — but zero tests were added for:

• loadCache('us') when a US release exists
• loadCache('us') when no US release exists (returns empty object)
• refreshCache correctly splitting BR vs US releases by tag suffix

The test mock only returns a release without -en-US in the tag, so the US path is never exercised. This is particularly risky given the dynamic property access and region splitting logic.

7. getLatestRelease is public but should be private

The method is an implementation detail of refreshCache and shouldn't be part of the public API of the Cache class. Add the private modifier.

8. Shared lastUpdate across regions masks partial refresh failures

If getLatestRelease(release, 'br') fails or returns early (no BR release found), but getLatestRelease(releaseUS, 'us') succeeds, this.lastUpdate is still updated at the end of refreshCache. On the next request, isOutdated() returns false and the empty BR cache is served stale until the interval expires. Consider per-region lastUpdate timestamps, or at minimum log a warning when a region returns no data.

---

🔵 Nitpicks (optional)

9. Unused variable in download handler

const request = req as RouterIncomingMessage; // declared but never used

req is used throughout the function, not request. Dead code, likely a copy-paste artifact.

10. getLatestRelease naming is misleading

The method doesn't return anything — it mutates instance state. A name like cacheReleaseByRegion or populateRegionCache would be more accurate.

11. release: any parameter type

getLatestRelease(release: any, region: 'br' | 'us') — the any defeats the purpose of TypeScript. The release shape is already implicit from usage. Introduce an interface or use the inferred GitHub API response type.

12. Tag suffix convention is undocumented

The entire BR/US split relies on the convention that US releases have -en-US in their tag_name. This convention exists nowhere in the README, a comment, or a constant. If someone creates a release with a different naming scheme, the whole feature silently breaks. At minimum, extract to a constant and add a comment:

const US_TAG_SUFFIX = '-en-US'; // Convention: US releases must include this in tag_name

13. packageManager field in package.json is unrelated

Unrelated change. Should go in a separate commit or PR to keep history clean.

14. In-code comments in PT-BR (index.ts)

// site bate pra pegar a versao latest — fine for this codebase, but inconsistent with the rest of the English comments.

---

Summary

Category	Count
🔴 Blockers	3
🟡 Warnings	5
🔵 Nitpicks	6

The core feature concept is sound and the approach of splitting releases by tag suffix is reasonable. However, the lack of runtime input validation on region is a real bug (silent wrong-region responses), the .DS_Store must never ship, and the dynamic property access should be replaced with a type-safe alternative. Fix those three and address the duplicate IDs and missing US tests before merge.

[kusanagi-claw]

QA Review — PR #2 (feat: region support)

Revisão completa após leitura de cache.ts, routes.ts, index.ts e cache.test.ts no branch feat/check-region.

---

🔴 Blockers

1. src/.DS_Store committed
O arquivo src/.DS_Store (gerado pelo macOS) está no PR. Nunca deve entrar no repositório — expõe metadados de estrutura de diretório local e polui o histórico. Ações necessárias:

• Remover o arquivo: git rm --cached src/.DS_Store
• Adicionar ao .gitignore: .DS_Store

2. Falha silenciosa quando nenhuma release BR é encontrada no GitHub
Em refreshCache():

const release = data.find(item => { ... isBR ... });
await this.getLatestRelease(release, 'br'); // release pode ser undefined

getLatestRelease tem um early return correto (if (!release?.assets ...) return;), mas quando isso acontece, this.latest permanece {} e this.lastUpdate é definido como Date.now() no final de refreshCache(). Resultado: pelo próximo intervalo inteiro (padrão 15 min), loadCache('br') retorna um objeto vazio silenciosamente, causando 404 em todos os downloads. Não há log de warning, nenhum erro, nenhuma retry. Deveria pelo menos logar um warning explícito.

3. URL de redirect proxy não preserva o region param — download silencioso da versão errada
No handler update em routes.ts, quando shouldProxyPrivateDownload é verdadeiro:

url: `${sanitizedBaseUrl}/download/${platformName}?update=true`
// ❌ Faltando: ?update=true&region=us

Um usuário US que chama /update/darwin/1.0.0?region=us recebe a resposta correta (versão US), mas o url de download aponta para /download/darwin?update=true — sem region=us. O cliente vai baixar a build BR no lugar da US. Bug funcional concreto em repositórios privados.

4. Type mismatch: version declarado como SemVer mas armazenado como string

// Declaração da classe:
private latest: { version?: SemVer; ... }

// Em getLatestRelease:
this[latestByRegion].version = tag_name; // tag_name é string (ex: "v4.0.0-canary.5")

O próprio teste confirma: expect(typeof storage.version).toBe('string'). Isso funciona em runtime porque tag_name é any e semver aceita strings, mas o tipo está mentindo. Se alguém depender do tipo SemVer (métodos como .major, .minor), vai explodir em runtime.

---

🟡 Warnings

5. Dynamic property access this[latestByRegion] com strict: true
O tsconfig.json tem "strict": true. O acesso:

const latestByRegion = region === 'br' ? 'latest' : 'latestUS';
this[latestByRegion].version = tag_name;

TypeScript em modo strict pode rejeitar indexação em propriedades privadas via string union sem index signature. Dependendo da versão do TS e das configurações exatas, isso pode ou não compilar. Recomendação: usar acesso explícito:

const cache = region === 'br' ? this.latest : this.latestUS;
cache.version = tag_name;

Mais seguro, mais legível, sem ambiguidade.

6. Convenção de tag -en-US completamente indocumentada
A lógica de distinção BR/US está hardcoded:

const isBR = !item.tag_name.includes('-en-US');
const isUS = item.tag_name.includes('-en-US');

Não há nenhuma menção no README, no corpo do PR, ou em comentários no código explicando que releases US devem ter o sufixo -en-US na tag. Quem criar uma release sem saber disso não vai entender por que o updater está ignorando ela.

7. Ausência total de testes para o path US
O mock em cache.test.ts só tem uma release sem -en-US:

tag_name: 'v4.0.0-canary.5', // tratado como BR

O loadCache('us') nunca é testado. Não há testes cobrindo:

• Cache US sendo populado corretamente
• loadCache('us') retornando dados US
• Comportamento quando não existe release US (cache vazio retornado silenciosamente)

8. region query param sem validação — valores inválidos passam silenciosamente
Em routes.ts:

const { region } = params as { region: CacheRegionType };

O cast as engana o TypeScript. Se alguém chamar ?region=invalid, o valor chega em loadCache e vai para o path latestUS (porque region === 'br' é false). Comportamento inesperado sem nenhum erro. Deveria haver validação explícita: if (region && region !== 'br' && region !== 'us') → 400.

---

🔵 Nitpicks

• loadCache(undefined) funciona — o default parameter = 'br' salva a situação quando region é undefined (query param ausente). Comportamento correto em runtime, mas o type cast em routes.ts está escondendo isso. Considerar tipar explicitamente como region?: CacheRegionType.

• index.ts — as únicas mudanças foram comentários em PT-BR nas rotas. A assinatura de loadCache está corretamente atualizada em cache.ts. Sem problemas aqui.

• Double loadCache call em overview — chama loadCache('br') e loadCache('us') separadamente. Se o cache expirar exatamente entre as duas chamadas, o segundo call pode triggerar outro refresh. Baixo risco mas vale notar.

• Typo no título do PR: "adicione verificao de region" → "verificação" (faltou ç).

• this.lastUpdate é setado mesmo quando nenhuma release é encontrada (linha final de refreshCache). Isso é o que causa o problema do blocker feat: adicione verificao de region no updater #2 — o cache pensa que está fresco mesmo estando vazio.

---

Verdict

REQUEST CHANGES

Os bloqueadores 1, 3 e 4 precisam ser corrigidos antes do merge. O bug do redirect sem region (#3) é um bug funcional silencioso que afeta usuários reais em repositórios privados. O DS_Store (#1) é limpeza obrigatória. O type mismatch de SemVer (#4) pode causar crashes em runtime se o tipo for usado como esperado. Recomendo também adicionar pelo menos um teste para o path US antes de mergear.