Repository navigation
chore: drop banned typecheck paths and fix broken npm scripts - #6452
Open
daggerstuff wants to merge 2 commits into
Open
daggerstuff wants to merge 2 commits into
daggerstuff wants to merge 2 commits into
Conversation
Remove root astro check/tsc scripts and stale docs references; repoint broken npm script paths; align agent/devcontainer docs with oxlint-only lint.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
documentation
Contributor
Reviewer's GuideThis cleanup removes banned and broken root typecheck workflows, redirects surviving validation and utility commands to valid lint or script targets, deletes obsolete tooling, and updates documentation and permissions to match the new workflow. Review the package-script dependency changes closely, especially the intentionally unchanged out-of-scope references to pnpm typecheck and the reported pre-existing test failures. Flow diagram for updated validation scriptsflowchart LR
A["Developer or Jules"] --> B["pnpm lint"]
B --> C["oxlint and repository lint gates"]
D["Makefile typecheck/check-all"] --> B
E["jules:check"] --> B
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
px Advisor ReviewRisk: unknown No summary available No findings. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
89
to
99
| "lint:markdown:ci:all": "pnpm exec markdownlint --config .markdownlint.json --ignore node_modules --ignore '**/node_modules/**' \"**/*.md\" \"**/*.mdx\"", | ||
| "lint:fix": "oxlint -c .oxlintrc.json --fix-dangerously . && pnpm lint:no-suppressions", | ||
| "typecheck": "NODE_OPTIONS='--max-old-space-size=8192' astro check && NODE_OPTIONS='--max-old-space-size=8192' tsc --noEmit", | ||
| "typecheck:strict": "NODE_OPTIONS='--max-old-space-size=8192' tsx scripts/validate-typescript.ts", | ||
| "type-check": "pnpm run typecheck", | ||
| "check": "NODE_OPTIONS='--max-old-space-size=8192' astro check", | ||
| "check:all": "pnpm run check && pnpm run typecheck", | ||
| "ts:debug": "./debug-typescript.sh normal", | ||
| "ts:debug:verbose": "./debug-typescript.sh verbose", | ||
| "ts:debug:minimal": "./debug-typescript.sh minimal", | ||
| "ts:debug:clean": "./debug-typescript.sh clean", | ||
| "ts:debug:repro": "./debug-typescript.sh repro", | ||
| "ts:debug:monitor": "./debug-typescript.sh monitor", | ||
| "ts:check:debug": "tsc --noEmit --project config/tsconfig.debug.json", | ||
| "ts:check:main": "tsc --noEmit --project config/tsconfig.json", | ||
| "ts:check:aws": "tsc --noEmit --project config/tsconfig.aws.json", | ||
| "ts:compare": "pnpm run ts:check:debug && echo '✅ Debug config works' && pnpm run ts:check:main && echo '✅ Main config works'", | ||
| "ts:switch:debug": "cp config/tsconfig.json config/tsconfig.json.backup && cp config/tsconfig.debug.json config/tsconfig.json", | ||
| "ts:switch:main": "cp config/tsconfig.json.backup config/tsconfig.json", | ||
| "ts:logs": "echo 'TypeScript logs location varies by system. Run: ./debug-typescript.sh normal'", |
Contributor
There was a problem hiding this comment.
Bug: Removal of root typecheck scripts breaks scripts/devops/aggregate-readiness.py, causing the /readiness API route to return 503 and breaking run-final-tests.sh.
Severity: HIGH
Suggested Fix
Retain or alias the typecheck scripts in package.json, or update scripts/devops/aggregate-readiness.py and scripts/testing/run-final-tests.sh to use the updated typecheck command.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: package.json#L88-L99
Potential issue: Removing root scripts such as `typecheck`, `check`, `check:all`, and
`safe:typecheck` breaks dependent scripts and endpoints.
`scripts/devops/aggregate-readiness.py` runs `pnpm typecheck`, which now fails with a
missing script error. Consequently, the live `/readiness` API endpoint
(`apps/web/src/api/routes/readiness.ts`) always fails and returns HTTP 503 'not-ready'.
Additionally, `scripts/testing/run-final-tests.sh` executes `npm run typecheck` with
`set -e`, aborting test execution prior to stopping the background server process.
Did we get this right? 👍 / 👎 to inform future reviews.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/content/blog-web.js" line_range="14" />
<code_context>
/* jshint esversion: 6, node: true */
// IMPORTANT: Import Sentry instrumentation first
-import '../instrument.mjs'
+import '../../config/instrument.mjs'
// Get current directory
</code_context>
<issue_to_address>
**Unauthenticated blog UI exposes local content**
When a developer runs `pnpm blog` on a machine reachable by other network users, `server.listen(PORT)` accepts connections on all interfaces, and the request handler has no authentication: a network caller can retrieve blog status/report output or send `action=generate` to write draft files into the workspace. The fixed import makes this interface startable via `pnpm blog`, exposing local content and allowing unauthorized changes.
Bind the UI to `127.0.0.1` and require authentication for any operation that reads or changes workspace content.
</issue_to_address>
### Comment 2
<location path="package.json" line_range="91" />
<code_context>
"lint:markdown:ci": "node scripts/ci/lint-changed.mjs --markdown",
"lint:markdown:ci:all": "pnpm exec markdownlint --config .markdownlint.json --ignore node_modules --ignore '**/node_modules/**' \"**/*.md\" \"**/*.mdx\"",
"lint:fix": "oxlint -c .oxlintrc.json --fix-dangerously . && pnpm lint:no-suppressions",
- "typecheck": "NODE_OPTIONS='--max-old-space-size=8192' astro check && NODE_OPTIONS='--max-old-space-size=8192' tsc --noEmit",
- "typecheck:strict": "NODE_OPTIONS='--max-old-space-size=8192' tsx scripts/validate-typescript.ts",
- "type-check": "pnpm run typecheck",
</code_context>
<issue_to_address>
**Removed scripts break validation**
When readiness, final tests, GitLab validation, or docs deployment runs, `run_typecheck` and the readiness aggregator invoke the removed `typecheck` script, GitLab invokes the removed `check` script, docs invokes the removed `safe:typecheck` script, and `run-final-tests.sh` also calls `typecheck`; readiness reports not-ready or returns 503, and validation fails or is skipped.
Update all affected callers to supported validation commands and retain a valid docs typecheck step.
Also at `package.json:24`, `package.json:94`, `package.json:219`.
</issue_to_address>
### Comment 3
<location path="package.json" line_range="210" />
<code_context>
- "tags:maintenance": "node scripts/tag-maintenance.js full-maintenance",
- "version:release": "node scripts/version-manager.js release",
- "version:info": "node scripts/version-manager.js info",
+ "tags:create": "node scripts/utils/tag-manager.cjs create",
+ "tags:list": "node scripts/utils/tag-manager.cjs list",
+ "tags:validate": "node scripts/utils/tag-manager.cjs validate",
</code_context>
<issue_to_address>
**Concurrent tag creates lose rollback requests**
When two callers create a tag for the same environment within one second, `tag-manager.cjs` generates the same second-resolution tag name for both creates, so one `git tag -a` fails because the tag already exists and its rollback request is not recorded.
Generate a unique tag name for each create.
</issue_to_address>
### Comment 4
<location path="package.json" line_range="211" />
<code_context>
- "version:release": "node scripts/version-manager.js release",
- "version:info": "node scripts/version-manager.js info",
+ "tags:create": "node scripts/utils/tag-manager.cjs create",
+ "tags:list": "node scripts/utils/tag-manager.cjs list",
+ "tags:validate": "node scripts/utils/tag-manager.cjs validate",
+ "tags:cleanup": "node scripts/utils/tag-manager.cjs cleanup",
</code_context>
<issue_to_address>
**Advertised tag commands fail**
When a caller runs one of the `pnpm tags:*` scripts, `tag-manager.cjs` handles only `create` and `validate`, so `tags:list` and `tags:cleanup` fall through to `usage()` and exit with status 1; `tags:create` and `tags:validate` also exit with usage unless an environment argument is supplied. Callers cannot perform the advertised tag operations.
Implement `list` and `cleanup` in `tag-manager.cjs` and ensure the `create` and `validate` scripts receive the required environment argument.
Also at `package.json:212-213`.
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: scripts/content/blog-web.js:14, package.json:91, package.json:210, package.json:211
| /* jshint esversion: 6, node: true */ | ||
| // IMPORTANT: Import Sentry instrumentation first | ||
| import '../instrument.mjs' | ||
| import '../../config/instrument.mjs' |
Contributor
There was a problem hiding this comment.
🟡 Medium · Unauthenticated blog UI exposes local content
When a developer runs pnpm blog on a machine reachable by other network users, server.listen(PORT) accepts connections on all interfaces, and the request handler has no authentication: a network caller can retrieve blog status/report output or send action=generate to write draft files into the workspace. The fixed import makes this interface startable via pnpm blog, exposing local content and allowing unauthorized changes.
Bind the UI to 127.0.0.1 and require authentication for any operation that reads or changes workspace content.
Prompt for AI agents
In `scripts/content/blog-web.js` at line 14:
**Unauthenticated blog UI exposes local content**
When a developer runs `pnpm blog` on a machine reachable by other network users, `server.listen(PORT)` accepts connections on all interfaces, and the request handler has no authentication: a network caller can retrieve blog status/report output or send `action=generate` to write draft files into the workspace. The fixed import makes this interface startable via `pnpm blog`, exposing local content and allowing unauthorized changes.
Bind the UI to `127.0.0.1` and require authentication for any operation that reads or changes workspace content.| "lint:markdown:ci": "node scripts/ci/lint-changed.mjs --markdown", | ||
| "lint:markdown:ci:all": "pnpm exec markdownlint --config .markdownlint.json --ignore node_modules --ignore '**/node_modules/**' \"**/*.md\" \"**/*.mdx\"", | ||
| "lint:fix": "oxlint -c .oxlintrc.json --fix-dangerously . && pnpm lint:no-suppressions", | ||
| "typecheck": "NODE_OPTIONS='--max-old-space-size=8192' astro check && NODE_OPTIONS='--max-old-space-size=8192' tsc --noEmit", |
Contributor
There was a problem hiding this comment.
🔴 Critical · Removed scripts break validation
When readiness, final tests, GitLab validation, or docs deployment runs, run_typecheck and the readiness aggregator invoke the removed typecheck script, GitLab invokes the removed check script, docs invokes the removed safe:typecheck script, and run-final-tests.sh also calls typecheck; readiness reports not-ready or returns 503, and validation fails or is skipped.
Update all affected callers to supported validation commands and retain a valid docs typecheck step.
Also at package.json:24, package.json:94, package.json:219.
Prompt for AI agents
In `package.json` at line 91:
**Removed scripts break validation**
When readiness, final tests, GitLab validation, or docs deployment runs, `run_typecheck` and the readiness aggregator invoke the removed `typecheck` script, GitLab invokes the removed `check` script, docs invokes the removed `safe:typecheck` script, and `run-final-tests.sh` also calls `typecheck`; readiness reports not-ready or returns 503, and validation fails or is skipped.
Update all affected callers to supported validation commands and retain a valid docs typecheck step.
Also at `package.json:24`, `package.json:94`, `package.json:219`.From .github/copilot-instructions.md L14-15:
- **Surgical Edits**: Keep changes minimal, safe, tightly scoped. Write clear,
self-documenting code. Remove only what your edits make obsolete.| "tags:maintenance": "node scripts/tag-maintenance.js full-maintenance", | ||
| "version:release": "node scripts/version-manager.js release", | ||
| "version:info": "node scripts/version-manager.js info", | ||
| "tags:create": "node scripts/utils/tag-manager.cjs create", |
Contributor
There was a problem hiding this comment.
🟡 Medium · Concurrent tag creates lose rollback requests
When two callers create a tag for the same environment within one second, tag-manager.cjs generates the same second-resolution tag name for both creates, so one git tag -a fails because the tag already exists and its rollback request is not recorded.
Generate a unique tag name for each create.
Prompt for AI agents
In `package.json` at line 210:
**Concurrent tag creates lose rollback requests**
When two callers create a tag for the same environment within one second, `tag-manager.cjs` generates the same second-resolution tag name for both creates, so one `git tag -a` fails because the tag already exists and its rollback request is not recorded.
Generate a unique tag name for each create.| "version:release": "node scripts/version-manager.js release", | ||
| "version:info": "node scripts/version-manager.js info", | ||
| "tags:create": "node scripts/utils/tag-manager.cjs create", | ||
| "tags:list": "node scripts/utils/tag-manager.cjs list", |
Contributor
There was a problem hiding this comment.
🟡 Medium · Advertised tag commands fail
When a caller runs one of the pnpm tags:* scripts, tag-manager.cjs handles only create and validate, so tags:list and tags:cleanup fall through to usage() and exit with status 1; tags:create and tags:validate also exit with usage unless an environment argument is supplied. Callers cannot perform the advertised tag operations.
Implement list and cleanup in tag-manager.cjs and ensure the create and validate scripts receive the required environment argument.
Also at package.json:212-213.
Prompt for AI agents
In `package.json` at line 211:
**Advertised tag commands fail**
When a caller runs one of the `pnpm tags:*` scripts, `tag-manager.cjs` handles only `create` and `validate`, so `tags:list` and `tags:cleanup` fall through to `usage()` and exit with status 1; `tags:create` and `tags:validate` also exit with usage unless an environment argument is supplied. Callers cannot perform the advertised tag operations.
Implement `list` and `cleanup` in `tag-manager.cjs` and ensure the `create` and `validate` scripts receive the required environment argument.
Also at `package.json:212-213`.From .github/copilot-instructions.md L14-15:
- **Surgical Edits**: Keep changes minimal, safe, tightly scoped. Write clear,
self-documenting code. Remove only what your edits make obsolete.
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
This branch was successfully deployed
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.
Summary
Subtraction-first cleanup: remove banned root
astro check/tscnpm scripts, fix stale doc references, and repair or delete broken script targets. No new tooling.Script / doc changes
typecheck,typecheck:strict,type-check,check,check:allvalidate-typescript*target fortypecheck:strictts:check:debug,ts:check:main,ts:check:aws,ts:comparets:check:*family usestsc;ts:comparedepended on removed scriptssafe:typechecktypecheckjules:checkstagingbranch; runspnpm lintinstead ofcheck:alltags:*scripts/utils/tag-manager.cjs(single CJS entry; removed duplicate.jsunder"type": "module")tags:maintenancescripts/tag-maintenance.jsanywhereversion:release,version:infoscripts/version-manager.jsanywheremcp:list,mcp:run,mcp:managescripts/mcp-manager.js;mcp:installerkeptblogscripts/content/blog-web.js; fixedinstrument.mjsimport toconfig/instrument.mjsso the path resolvesdev:all-servicesscripts/dev/health-check.shprefix; concurrent service start unchangedtypecheck:python,lint*,ts:debug*GEMINI.mdpnpm typecheckreasonix.toml[permissions] allowastro check/typecheckbash patternssetup.md,development-setup.mdpnpm lint; removed typecheck bulletfix-ts-errors.sh,ts-error-docs.mdtscworkflow.devcontainer/devcontainer.jsonpackageManager([email protected])Makefiletypecheck/check-allpnpm lintOut of scope (not changed; still mention banned commands)
scripts/devops/aggregate-readiness.pystill invokespnpm typecheckfor readiness lanespnpm typecheckin comments or checkliststypecheckscripts in workspace packages (e.g. agents) unchangedVerification
pnpm lint— pass (Node 24.20.0)node scripts/utils/tag-manager.cjs— usage prints (path resolves)node scripts/content/blog-web.js— starts UI (path + import resolve)pnpm test:unit— 2 pre-existing failures only:graphql.test.ts(suite),versioning.test.ts(6-month sunset assertion); no new failures vs stagingSummary by Sourcery
Remove unsupported type-checking commands and clean up broken or obsolete development scripts while standardizing validation on the repository lint workflow.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: