fix: make build chmod cross-platform - #1187
Conversation
📝 WalkthroughWalkthroughThe build scripts for ChangesBuild permission handling
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
DCO sign-off missing One or more commits in this PR are missing a How to fix: # Single commit
git commit --amend -s
git push --force-with-lease
# Multiple commits -- replace N with the number of commits in your PR
git rebase --signoff HEAD~N
git push --force-with-lease
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
O
pm run build dos dois servidores MCP terminava em chmod +x build/index.js,
que nao existe no Windows. O tsc ja tinha compilado quando a chain quebrava,
entao o build saia integro mas com exit 1. Esse e o problema real: o build
"falhando" com artefato bom treina a gente a ignorar o exit code, e ai a
proxima falha de verdade passa batido.
Trocado por
ode -e "require('node:fs').chmodSync(...)". Sem dependencia nova:
fs.chmodSync e stdlib e no Windows nao lanca, so nao tem efeito util, que e
exatamente o comportamento desejado. O shebang #!/usr/bin/env node continua nos
dois fontes, entao o bit de execucao segue sendo aplicado no Unix.
Corrigido nos DOIS servidores, nao so no guardian onde apareceu. O egc-memory
tinha a mesma linha e teria continuado quebrado.
Verificacao: egc-guardian roda ponta a ponta com exit 0. No egc-memory o tsc
para antes, por um bug independente e anterior a este commit (TS5108,
moduleResolution=node10 removido, porque node_modules tem typescript 7.0.2
enquanto o package.json declara ^5.3.3); ali o chmod foi validado isolado,
tambem exit 0. Esse bug de versao fica pra branch propria.
Signed-off-by: Matheus Fuentes <matheus.fu.p@hotmail.com>
768431a to
6d90c59
Compare
|
|
@fuentes71 heads up: this branch started conflicting with main after the v1.1.18 release. A quick rebase and it is ready to move. |
|
Closing this one with credit where it is due, @fuentes71: your diagnosis was exactly right, and the v1.1.18 release hardening shipped the same fix as a dedicated postbuild script (mcp/servers/*/scripts/postbuild.js). It skips the chmod on Windows and preserves the compiler file mode on Unix, so both servers build with exit 0 on every platform straight from main. Your branch predates that landing, so there is nothing left for it to change. If you can, pull main and confirm the Windows build on your machine; that confirmation is still valuable. And #1188 stays open waiting for your answer. |
2 participants
O que a branch entrega
Torna o
npm run builddos dois servidores MCP cross-platform. Hoje a chain termina emchmod +x build/index.js, comando que nao existe no Windows.O problema
O
tscja compilou quando a chain quebra, entao o build sai integro mas com exit 1. O artefato esta bom e o comando reporta falha.Esse e o risco real, e nao e o build: um build que falha vermelho com artefato bom ensina todo mundo a ignorar o exit code, e a proxima falha de verdade passa batido. Em CI, significa que o job so nao quebra por sorte de plataforma.
A correcao
chmod +x build/index.jsviranode -e "require('node:fs').chmodSync('build/index.js', 0o755)".fs.chmodSynce stdlib. Descarteishxjustamente por isso.#!/usr/bin/env nodeno topo, entao o bit de execucao e intencional. Simplesmente remover ochmodteria trocado um bug barulhento por um silencioso.Escopo
Corrigido nos dois servidores,
egc-guardianeegc-memory, nao so onde o problema apareceu. O memory tinha a mesma linha e teria continuado quebrado.Verificacao
npm run buildcom exit 0 nos dois servidores, no Windows, em cima desta base. Um diff de 2 linhas.Fora de escopo, de proposito
Durante a investigacao o
egc-memoryfalhava tambem comTS5108: moduleResolution=node10 has been removed. Confirmei que era staleness da base antiga: apos rebase namainatual, com a migracao do TypeScript 7 (#689) ja integrada, o erro nao ocorre mais. Nada a fazer aqui.🤖 Generated with Claude Code
Summary by cubic
Make the MCP server build scripts cross-platform by replacing
chmod +xwith a Nodefs.chmodSynccall. Fixes Windows builds that compiled but exited with code 1.chmod +x build/index.jswithnode -e "require('node:fs').chmodSync('build/index.js', 0o755)"inegc-guardianandegc-memory.tscsucceeds, avoiding false CI failures.Written for commit 6d90c59. Summary will update on new commits.
Summary by CodeRabbit