chore: tighten EmDash core and adapter type safety - #310
Conversation
|
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
Scope checkThis PR touches 28 files. PRs with a broad scope are harder to review. Please confirm the scope hasn't drifted beyond the intended change. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Made-with: Cursor
75d278c to
5427451
Compare
|
I have read the CLA Document and I hereby sign the CLA |
Overlapping PRsThis PR modifies files that are also changed by other open PRs: This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
recheck |
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
There was a problem hiding this comment.
This is a well-intentioned and mostly correct type-safety and SQL-hardening sweep. The approach fits EmDash's architecture: replacing unsafe as any/as unknown casts with narrower types, adding runtime guards where narrowing alone isn't enough, and using sql.ref() and parameterized templates instead of string-interpolated SQL. The SQL changes in content.ts and rewrite-urls.ts follow Kysely's safe patterns, and mapOrderField already whitelists order columns so sql.ref(dbField) is injection-safe.
Three gaps remain:
- Two dangling
eslint-disable-next-line @typescript-eslint/no-explicit-anycomments inplayground-middleware.tswere left behind afterKysely<any>was replaced withKysely<Database>. These are likely to trigger lint diagnostics about unused disable directives. - No changeset was added, yet the PR changes exported types (
StandardHookHandler,StandardRouteHandler,StandardPluginDefinition) and introduces new runtime validation in the WordPress plugin import route. Per AGENTS.md, published package changes require a changeset. - The new
parseWpPluginImportConfigvalidation function inwordpress-plugin/execute.tschanges route behavior (invalid configs now return 400 instead of being passed through), but there are no tests covering the validation paths. AGENTS.md expects tests for new runtime behavior.
|
Hello. This PR has been open for more than a month, yet the CLA has not been signed. If you intend to follow through, kindly open a new PR and sign the CLA. Thank you. |
|
I realize you have sent a message to sign the CLA but it seems like there are unresolved conflicts. Feel free to open a new PR either way and we are happy to take a new look. |
Summary
This pull request is a type-safety and SQL-hardening cleanup across EmDash runtime layers with no behavior changes.
Scope and intent
Notes