packages/firebird/src/query.ts (2)

14-14: Unused declaration: connectionManager property.

This property is declared but never used. Line 21 uses this.connection instead. Either this declaration is unnecessary or there's a property name mismatch.

♻️ Proposed fix - remove unused declaration

 export class FirebirdQuery extends AbstractQuery {
-  declare connectionManager: FirebirdConnection;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query.ts` at line 14, The class has an unused property
declaration connectionManager while the code uses this.connection; remove the
unused connectionManager declaration or rename it to connection to match usage
(update any references to the property accordingly), ensuring the class property
name (connection) aligns with places like methods referencing this.connection in
the Query class.

---

157-182: Consider hoisting error class imports to module level.

Using require() inside the method body incurs repeated module resolution overhead on each error. These imports could be moved to the top of the file for better performance and clearer dependencies.

♻️ Proposed refactor

 import { AbstractQuery } from '@sequelize/core';
+import {
+  DatabaseError,
+  ForeignKeyConstraintError,
+  UniqueConstraintError,
+  ValidationError,
+  ValidationErrorItem,
+} from '@sequelize/core';
 import { logger } from '@sequelize/core/_non-semver-use-at-your-own-risk_/utils/logger.js';

Then replace the require() calls with direct usage of the imported classes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query.ts` around lines 157 - 182, Hoist the runtime
imports for Sequelize error classes out of the method and into the module scope
to avoid repeated require() calls: add a single top-level destructuring
assignment like const { UniqueConstraintError, ForeignKeyConstraintError,
ValidationError, ValidationErrorItem, DatabaseError } =
require('@sequelize/core'); (or equivalent ES import) at the top of the file,
then replace the inline require(...) usages inside the function that reference
UniqueConstraintError, ForeignKeyConstraintError, ValidationError,
ValidationErrorItem, and DatabaseError with the now-top-level identifiers so the
function returns new UniqueConstraintError(...), new
ForeignKeyConstraintError(...), new ValidationError(...), and new
DatabaseError(...) without performing require() on each execution.

packages/firebird/src/query-interface.ts (1)

3-9: Placeholder class noted.

The class extends AbstractQueryInterface but doesn't implement any Firebird-specific methods. The doc comment mentions "auto-increment via SEQUENCE + BEFORE INSERT TRIGGER" support, but the actual DDL operations aren't implemented yet.

As per coding guidelines, schema operations for new features should be added in query-interface.ts. Consider tracking this as a follow-up task to implement the promised auto-increment DDL functionality.

Would you like me to open an issue to track implementing the Firebird-specific DDL operations (SEQUENCE/TRIGGER creation for auto-increment)?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query-interface.ts` around lines 3 - 9,
FirebirdQueryInterface currently only extends AbstractQueryInterface but lacks
the Firebird-specific DDL operations promised in the docstring; add
implementations to query-interface.ts that provide auto-increment support via
SEQUENCE + BEFORE INSERT TRIGGER: implement methods on the
FirebirdQueryInterface class such as createAutoIncrement (or createSequence and
createTrigger), dropAutoIncrement (or dropSequence/dropTrigger), and any helpers
to generate the proper SQL and execute it via the existing query runner; ensure
method names are exported on FirebirdQueryInterface and integrate with existing
AbstractQueryInterface patterns so callers can create/drop sequences and
triggers for auto-increment columns.

packages/firebird/readme.md (1)

63-79: Add language specifier to fenced code block.

The package structure code block should have a language specifier for proper rendering and linting compliance.

📝 Proposed fix

-```
+```text
 packages/firebird/
 ├── package.json

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/readme.md` around lines 63 - 79, The fenced
package-structure block in README.md is missing a language specifier; update the
opening fence for the snippet that lists the packages/firebird tree from ``` to
```text (or another appropriate language like ```bash) so the block becomes a
labeled code fence, leaving the closing ``` intact; this change targets the
README.md file's package structure code block.

packages/core/src/sequelize-typescript.ts (1)

203-206: Pre-existing duplicate entries in SUPPORTED_DIALECTS.

Note: 'mariadb' and 'mssql' appear twice in the array. This is a pre-existing issue unrelated to this PR, but may be worth addressing in a follow-up.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/sequelize-typescript.ts` around lines 203 - 206, The
SUPPORTED_DIALECTS array contains duplicate entries ('mariadb' and
'mssql')—remove the duplicates so each dialect appears only once; locate the
SUPPORTED_DIALECTS constant in sequelize-typescript.ts and update the array to
hold unique strings (e.g., use a deduplicated list or construct it via
Array.from(new Set(...))) to ensure no repeated dialect names.

packages/firebird/src/connection-manager.ts (1)

199-233: Consider static imports for error classes.

The error classes (ConnectionRefusedError, AccessDeniedError, etc.) are dynamically required within #translateError. Using static imports at the top of the file would improve bundler compatibility and code clarity.

Use static imports for error classes

-import type { AbstractConnection, ConnectionOptions } from '@sequelize/core';
-import { AbstractConnectionManager, ConnectionError } from '@sequelize/core';
+import type { AbstractConnection, ConnectionOptions } from '@sequelize/core';
+import {
+  AbstractConnectionManager,
+  AccessDeniedError,
+  ConnectionError,
+  ConnectionRefusedError,
+  HostNotFoundError,
+  HostNotReachableError,
+  InvalidConnectionError,
+} from '@sequelize/core';

 // ... later in `#translateError` ...

   `#translateError`(err: Error & { gdscode?: number }): ConnectionError {
     const msg = err.message ?? '';

     if (/connection refused/i.test(msg)) {
-      const { ConnectionRefusedError } = require('@sequelize/core');
       return new ConnectionRefusedError(err);
     }

     if (/access denied/i.test(msg) || err.gdscode === 335_544_472) {
-      const { AccessDeniedError } = require('@sequelize/core');
       return new AccessDeniedError(err);
     }

     // ... similar for other error types
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/connection-manager.ts` around lines 199 - 233, Move the
dynamic require calls inside `#translateError` to static imports at the top of the
module: import { ConnectionRefusedError, AccessDeniedError, HostNotFoundError,
HostNotReachableError, InvalidConnectionError, ConnectionError } from
'@sequelize/core'; then update `#translateError` to reference these imported
classes directly (remove require(...) calls) so the function uses the statically
imported ConnectionRefusedError, AccessDeniedError, HostNotFoundError,
HostNotReachableError, InvalidConnectionError and ConnectionError symbols.

packages/firebird/src/query-generator.internal.ts (1)

6-8: Consider documenting why technical schema names is empty.

Firebird doesn't support schemas in the same way other databases do. A brief comment explaining this would help maintainers understand the empty array isn't an oversight.

Add explanatory comment

   getTechnicalSchemaNames(): readonly string[] {
+    // Firebird does not support schemas; there are no technical schema names to exclude
     return [];
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query-generator.internal.ts` around lines 6 - 8, The
getTechnicalSchemaNames() method currently returns an empty array but lacks
explanation; add a short comment above the getTechnicalSchemaNames() function
stating that Firebird does not implement named schemas like other DBs and
therefore there are no technical schema names to return, so returning [] is
intentional (reference the getTechnicalSchemaNames function to locate the spot).

packages/firebird/src/_internal/data-types-overrides.ts (2)

170-174: Shadowing the global JSON identifier.

The class name JSON shadows the global JSON object, which can cause confusion and potential issues if the global JSON is needed within this module. This is flagged by the static analysis tool.

Consider renaming to FirebirdJSON or JSON_TYPE to avoid shadowing, though since this is a data-type override module following the Sequelize pattern, the shadowing may be intentional for consistency with other dialects.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/_internal/data-types-overrides.ts` around lines 170 -
174, The class named JSON shadows the global JSON identifier; rename the export
class JSON (and any references) to a non-conflicting identifier like
FirebirdJSON or JSON_TYPE, update its extends clause (currently BaseTypes.JSON)
and any places that import or instantiate this type so they reference the new
name, and keep the toSql() implementation unchanged to return 'BLOB SUB_TYPE
TEXT'.

---

113-116: Method signature diverges from base class.

The escape method adds an optional _options parameter that doesn't exist in the base BOOLEAN.escape(value: boolean | unknown): string signature (see packages/core/src/abstract-dialect/data-types.ts:1349-1351). While this works due to TypeScript's structural typing, it's inconsistent with the base class contract.

Align with base class signature

-  escape(value: boolean | unknown, _options?: any): string {
+  escape(value: boolean | unknown): string {
     return value ? 'TRUE' : 'FALSE';
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/_internal/data-types-overrides.ts` around lines 113 -
116, The BOOLEAN.escape method signature diverges from the base class by
including an extra optional parameter; update the method signature in the
BOOLEAN class from escape(value: boolean | unknown, _options?: any): string to
match the base class signature escape(value: boolean | unknown): string, and
remove the unused _options parameter from the implementation so the override
exactly matches the base contract.

packages/firebird/src/connection-manager.ts (1)

74-79: ⚠️ Potential issue | 🟡 Minor

Align the documented password default with the actual one.

The JSDoc says the default password is 'masterkey', but connect() falls back to ''. Pick one so configuration docs and runtime behavior match.

Also applies to: 149-150

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/connection-manager.ts` around lines 74 - 79, The JSDoc
for the password property currently documents a default of 'masterkey' but
connect() actually falls back to an empty string; update the documentation to
match runtime behavior by changing the JSDoc default to '' (empty string) for
the password property and any other JSDoc occurrences (see the other docblock
around the password/join lines referenced), or alternatively change the
connect() fallback to 'masterkey' if you prefer that as the real default—ensure
consistency between the password?: string JSDoc and the connect()
implementation.

packages/firebird/src/_internal/data-types-overrides.ts (1)

128-136: ⚠️ Potential issue | 🟠 Major

Override toBindableValue(), not stringify().

AbstractDataType.escape() serializes DATE values through toBindableValue(), so this Firebird-specific formatter is bypassed and the base timestamp format still leaks through. The as Date cast is also unsafe because sanitize() can return DayJS/Moment objects.

Possible fix

 export class DATE extends BaseTypes.DATE {
   toSql(): string {
     // Firebird TIMESTAMP = date + time
     return 'TIMESTAMP';
   }

-  stringify(value: AcceptedDate): string {
-    const d = this.sanitize(value) as Date;
-    // Firebird accepts: 'YYYY-MM-DD HH:MM:SS.mmm'
-
-    return (
-      `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ` +
-      `${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}.${pad(d.getMilliseconds(), 3)}`
-    );
+  override toBindableValue(value: AcceptedDate): string {
+    return this._applyTimezone(value).format('YYYY-MM-DD HH:mm:ss.SSS');
   }
 }

#!/bin/bash
rg -n "escape\\(|toBindableValue\\(|stringify\\(" \
  packages/core/src/abstract-dialect/data-types.ts \
  packages/firebird/src/_internal/data-types-overrides.ts -C2

packages/firebird/src/dialect.ts (3)

53-55: ⚠️ Potential issue | 🟠 Major

supports.returnValues overstates Firebird support.

The adjacent comment says Firebird only supports RETURNING on INSERT, but 'returning' advertises generic support and lets core generate invalid UPDATE ... RETURNING / DELETE ... RETURNING SQL unless those paths are overridden.

As per coding guidelines, "When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/dialect.ts` around lines 53 - 55, The
supports.returnValues flag currently set to the generic 'returning' overstates
Firebird's capability and allows core to emit UPDATE/DELETE ... RETURNING;
change supports.returnValues so it only advertises RETURNING for INSERT (e.g., a
structure indicating insert: 'returning' or otherwise disable update/delete
returning) and ensure any related places (the inserts configuration and any
update/delete code paths) are adjusted so core will not generate UPDATE ...
RETURNING or DELETE ... RETURNING for the Firebird dialect.

---

112-115: ⚠️ Potential issue | 🟠 Major

Don't disable identifier quoting for Firebird.

Firebird supports double-quoted identifiers. Using an empty delimiter turns quoting off completely, so reserved words and case-sensitive identifiers remain unsafe even though the core helper now avoids the regex crash.

As per coding guidelines, "When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/dialect.ts` around lines 112 - 115, The constructor in
this Firebird dialect currently disables identifier quoting by setting
identifierDelimiter: '', which is incorrect—change identifierDelimiter to '"'
(double-quote) so Firebird double-quoted identifiers remain supported, and
update the dialect feature flags in the same file (where Firebird dialect
features are declared) to explicitly mark identifier quoting/case-sensitive
identifier support per the coding guidelines; locate the constructor and the
feature flags block (references: constructor(sequelize: Sequelize, options:
FirebirdDialectOptions), identifierDelimiter) and adjust them accordingly.

---

95-98: ⚠️ Potential issue | 🟠 Major

Declare truncate.cascade as unsupported.

The dialect metadata should reject { cascade: true } up front instead of silently accepting it while the query generator falls back to DELETE FROM.

As per coding guidelines, "When adding a dialect feature, update packages/{dialect}/src/dialect.ts to declare feature support flags".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/dialect.ts` around lines 95 - 98, The truncate metadata
for the Firebird dialect currently only sets restartIdentity and must explicitly
mark cascade as unsupported; update the truncate object in
packages/firebird/src/dialect.ts (the truncate metadata) to include cascade:
false (or the dialect's unsupported flag) so the dialect rejects { cascade: true
} up front instead of silently accepting it and letting the query generator
fallback to DELETE FROM.

packages/firebird/src/query.ts (3)

16-18: ⚠️ Potential issue | 🟠 Major

Don't hardcode 'id' as the generated key name.

This only works for models whose auto-increment column is literally id. Inserts returning user_id, invoice_no, or any other generated column will not hydrate the instance primary key correctly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query.ts` around lines 16 - 18, Replace the hardcoded
return in getInsertIdField() so it reads the model/table metadata for the
auto-generated primary key name instead of always returning 'id' — for example,
return the configured primary key from this.model (e.g. this.model.primaryKey or
this.model.getPrimaryKeyName() or this.model.meta.primaryKey) and only fall back
to 'id' if no metadata exists; update getInsertIdField() to use that metadata
path so inserts returning user_id, invoice_no, etc. hydrate correctly.

---

136-145: ⚠️ Potential issue | 🟠 Major

Keep normalized rows plain and lossless.

This block currently forces every key to lowercase, ignores the lowercase_keys connection option, converts every Buffer to UTF-8, and injects dataValues / _previousDataValues into raw results. That breaks case-sensitive aliases, corrupts binary BLOB/OCTETS data, and leaks model internals into rows that should stay plain.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query.ts` around lines 136 - 145, The `#normaliseRow`
function currently mutates raw rows by forcing all keys to lowercase,
unconditionally converting Buffer values to UTF-8 strings, and injecting
dataValues / _previousDataValues; update it to keep rows plain and lossless:
respect the connection option lowercase_keys (only lower-case keys if that
option is enabled), preserve Buffer/Uint8Array/BLOB types instead of converting
them to strings, and remove the insertion of dataValues and _previousDataValues
so the method returns an untouched record with only the normalized key
transformation applied when enabled; locate the method named `#normaliseRow` to
implement these changes.

---

53-69: ⚠️ Potential issue | 🟠 Major

Affected-row counts are incorrect here.

Line 58/61 always reports insert count 1, and Lines 65/69 derive update/delete counts from returned rows even though those statements do not yield row sets in this dialect. That will break Sequelize's update, destroy, and bulk insert result contracts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/query.ts` around lines 53 - 69, The `#handleResponse`
currently returns a hardcoded insert count of 1 and derives update/delete counts
from returned row arrays, which is incorrect for this dialect; change
`#handleResponse` (and related call to handleInsertQuery) so that for inserts you
return the inserted row(s) and the actual insert count (use rows.length, not 1),
and for update/delete do NOT use rows.length but instead extract the
affected-row count from the protocol/metadata (e.g. this.affectedRows or a new
helper like getAffectedCountFromResponse()) and return [rowsOrUndefined,
affectedCount]; update isInsertQuery/isUpdateQuery/isDeleteQuery branches to use
those real counts and add a small helper on the class to normalize metadata ->
affected count if needed (refer to `#handleResponse`, handleInsertQuery,
isInsertQuery, isUpdateQuery, isDeleteQuery).

packages/firebird/src/connection-manager.ts (2)

199-233: Consider importing error classes at the top instead of dynamic requires.

The repeated require('@sequelize/core') calls inside #translateError are inefficient and deviate from the ES module import pattern used elsewhere in the file. Since ConnectionError is already imported, the other error classes can be imported alongside it.

Suggested refactor

-import { AbstractConnectionManager, ConnectionError } from '@sequelize/core';
+import {
+  AbstractConnectionManager,
+  ConnectionError,
+  ConnectionRefusedError,
+  AccessDeniedError,
+  HostNotFoundError,
+  HostNotReachableError,
+  InvalidConnectionError,
+} from '@sequelize/core';
...
   `#translateError`(err: Error & { gdscode?: number }): ConnectionError {
     const msg = err.message ?? '';

     if (/connection refused/i.test(msg)) {
-      const { ConnectionRefusedError } = require('@sequelize/core');
       return new ConnectionRefusedError(err);
     }

     if (/access denied/i.test(msg) || err.gdscode === 335_544_472) {
-      const { AccessDeniedError } = require('@sequelize/core');
       return new AccessDeniedError(err);
     }
     // ... similar for other error types
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/connection-manager.ts` around lines 199 - 233, Replace
the dynamic require calls inside the private method `#translateError` with static
imports at the top of the module: import ConnectionRefusedError,
AccessDeniedError, HostNotFoundError, HostNotReachableError, and
InvalidConnectionError from '@sequelize/core' alongside the existing
ConnectionError import, then update the `#translateError` function to instantiate
those imported classes instead of calling require(...) so the method uses the
statically imported symbols (ConnectionRefusedError, AccessDeniedError,
HostNotFoundError, HostNotReachableError, InvalidConnectionError,
ConnectionError).

---

175-177: Validation only checks the closed flag, not actual connection health.

The validate() method returns true if the connection hasn't been explicitly closed, but doesn't verify the connection is still alive (e.g., server-side timeout or network drop). Consider adding a lightweight ping or query if node-firebird supports it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/connection-manager.ts` around lines 175 - 177, The
validate() method currently only checks the CLOSED_SYMBOL flag; update
validate(connection: FirebirdConnection) to perform a lightweight health check
(e.g., call a ping method or run a trivial query like SELECT 1) in addition to
checking CLOSED_SYMBOL, catch any errors/timeouts and return false on failure,
and return true only when both the closed flag is false and the health check
succeeds; ensure the implementation uses the existing CLOSED_SYMBOL and handles
errors quickly (short timeout) to avoid blocking the pool.

packages/firebird/src/_internal/data-types-overrides.ts (1)

114-116: escape() may produce incorrect output for non-boolean falsy values.

The method coerces any falsy value (including 0, '', null, undefined) to 'FALSE'. Consider explicit boolean type checking for stricter correctness.

Suggested improvement

   escape(value: boolean | unknown, _options?: any): string {
-    return value ? 'TRUE' : 'FALSE';
+    return value === true ? 'TRUE' : 'FALSE';
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/firebird/src/_internal/data-types-overrides.ts` around lines 114 -
116, The escape(value: boolean | unknown, _options?: any) method currently
coerces any falsy value to 'FALSE'; change it to explicitly check boolean
identity so only true yields 'TRUE' and only false yields 'FALSE' (e.g., if
(value === true) return 'TRUE'; if (value === false) return 'FALSE';), and for
non-boolean inputs either throw a TypeError or handle them explicitly (choose
consistent behavior for non-boolean values) to avoid silently producing
incorrect SQL.