Codex/config rag, update auth, and conect open ai - #1
Merged
Conversation
- Removed outdated legal items related to intellectual property from legal-content.ts. - Added a function to check for blocked legal basis in legal-content.ts. - Updated package.json to allow for more flexible test file matching. - Enhanced tests to ensure sensitive information is not exposed and added validation for admin session creation. - Introduced new tests for handling copyright content and ensuring proper responses for published knowledge questions. - Created a comprehensive schema foundation test to validate database migrations and integrity checks. - Implemented checks for source and provision publication, ensuring four-eyes review and immutability of creators.
- Add `openai-evidence.ts` module for composing evidence-based answers using OpenAI API. - Define types for evidence records, composition requests, and responses. - Implement validation functions for evidence requests and compositions. - Create a provider input builder and JSON schema generator for evidence compositions. - Add a smoke test script for basic functionality verification. - Introduce unit tests for evidence composition logic, including validation and API interaction. - Update `package.json` to include new test and smoke scripts.
- Introduced a new test suite for the legal evidence retriever in `tests/legal-evidence-retriever.test.mjs`. - Added various test cases to validate normalization, eligibility, ranking, and error handling for provision candidates. - Updated `package.json` to include a new test script for the legal evidence retriever.
…itation integrity - Added checksum versioning for provisions to ensure data integrity. - Introduced new provision effectivity statuses: "unknown", "in_force", "partially_in_force", "superseded", and "expired". - Implemented checksum computation for provisions, ensuring consistent hashing of relevant data. - Updated candidate eligibility checks to include citation revision and checksum validation. - Enhanced database schema to support new fields for citations and provisions. - Added tests to validate checksum functionality and citation integrity. - Updated migration scripts to include reviewed RAG bridge and maintain legacy data.
- Introduced migration for editorial trust primitives (0003_editorial_trust_primitives). - Updated journal to reflect the new migration entry. - Created comprehensive tests for the editorial workflow schema, ensuring proper migration behavior and integrity checks. - Enhanced existing tests to validate the migration process from 0000 to 0003, preserving legacy content and ensuring correct state transitions. - Implemented checks for role administration, revision creation, and audit event immutability.
…ry, and telemetry - Implemented a mock database for testing rate limiting functionality. - Updated admin login test to use a stronger password. - Added tests for source registry validation and mapping of official samples to drafts. - Introduced telemetry tests to ensure proper event capturing and serialization. - Enhanced worker observability by integrating it into the main worker entry point. - Updated migration journal to reflect new rate limit versioning.
…nality - Enhance image intent tests with additional cases for privacy and copyright concerns. - Implement tests for ambiguous image questions to ensure proper handling. - Introduce new tests for public showcase API, including validation of source URLs, response structure, and error handling. - Ensure deterministic ordering of showcases and proper handling of duplicate IDs in projections. - Validate rendering of showcase components and dialog interactions in the UI.
- Add synthetic technical shadow cases fixture in JSON format. - Create ai-shadow.ts to handle AI shadow configuration, validation, and batch processing. - Update openai-evidence.ts to support new AI models and improve error handling. - Introduce shadow-openai-evidence script for running AI shadow batches. - Implement tests for AI shadow functionality, including fixture validation and provider interaction. - Ensure proper handling of provider errors and response validation.
- Introduced new scripts for web search smoke tests. - Added commands for testing web search and web candidates in package.json. - Implemented tests for admin authentication, ensuring multi-account registry binds sessions to principals. - Updated editorial workflow schema tests to reflect new migration tags. - Created comprehensive tests for openai-web-search, validating configurations, sanitization, and citation handling. - Added tests for web search candidates, ensuring proper persistence and review workflows. - Enhanced error handling and validation in web search functionalities.
- Introduced `ChatAnswerSection` type to represent structured sections in chat answers. - Updated `projectPublicWebSearchAnswer` to parse and format answers into sections. - Modified `Home` component to render structured sections and warnings appropriately. - Added presentation contract for chat answers, ensuring no raw Markdown/HTML is exposed. - Implemented tests for section parsing and rendering to ensure compliance with new structure. - Updated documentation to reflect changes in presentation requirements and user stories.
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (80)
📝 WalkthroughWalkthroughThis change adds Cloudflare/D1 editorial foundations, authenticated candidate review, fail-closed legal retrieval, constrained AI and web search, public showcase rendering, rate limiting, telemetry, migrations, fixtures, runbooks, and broad integration tests. ChangesPlatform foundation
Grounded content and AI
Public experience
Verification and delivery
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 72
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.tsx (1)
83-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not discard law data when the showcase payload fails validation.
The effect throws on
!parsedShowcasesbeforesetManagedLawsruns. One invalid showcase row therefore removes all managed laws from the lookup table, and the page silently falls back to the bundledlawsarray. Parse the two collections independently so a showcase failure only degrades the showcase gallery.🔧 Proposed decoupling
const content = (await response.json()) as PublishedContent; const parsedShowcases = parsePublicShowcases(content.showcases); - if (!parsedShowcases) throw new Error("invalid showcase response"); if (!active) return; setManagedLaws((content.laws ?? []).map((item) => ({ @@ }))); - setManagedShowcases(parsedShowcases); - setShowcaseState(parsedShowcases.length > 0 ? "ready" : "empty"); + if (!parsedShowcases) { + setManagedShowcases([]); + setShowcaseState("degraded"); + return; + } + setManagedShowcases(parsedShowcases); + setShowcaseState(parsedShowcases.length > 0 ? "ready" : "empty");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/page.tsx` around lines 83 - 107, Decouple law and showcase parsing in the effect around setManagedLaws and setManagedShowcases: update managed laws from content.laws independently before handling showcase validation, and ensure an invalid parsedShowcases result only clears/degrades showcase state without discarding law data or triggering the bundled-laws fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/admin/AdminDashboard.tsx`:
- Around line 416-424: Update the candidate status rendering in the candidates
map to use a Vietnamese label map for all five candidate lifecycle states
instead of displaying candidate.status directly. Reuse the dashboard’s existing
status-label conventions, including “Đã xuất bản” and “Bản nháp,” and provide
labels for pending_review, rejected, approved, published, and archived while
preserving the status class value.
- Around line 299-318: Update startEdit so the default lastVerifiedAt date is
derived from the operator’s local calendar date rather than new
Date().toISOString(), preserving the existing YYYY-MM-DD format used by the
citation field.
- Around line 320-334: The mutate function currently propagates a post-success
load failure as a mutation error. Update mutate so the successful PATCH remains
successful and its success state cleanup is preserved, while isolating or
handling the awaited load() failure separately so callers do not set the “Không
thể cập nhật.” error; keep genuine PATCH failures propagating normally.
- Around line 399-412: Update the citation list rendered by draft.citations.map
in the AdminDashboard component to use the map index as the React key instead of
citation.url. Keep the existing citation editing behavior unchanged, using the
index already provided by the map callback.
- Line 409: Replace the combined “Khoản / điểm” input in the citation editor
with separate controlled inputs for citation.clause and citation.point, each
updating only its corresponding field in the draft citations map. Preserve
existing values independently and avoid splitting, recombining, or discarding
data during edits.
- Around line 251-297: Remove the duplicated fetch logic from the useEffect in
AdminDashboard and invoke the existing load callback instead. Move loading-state
completion and error handling into load, including setLoading(false), while
preserving the 401 redirect, response parsing, state assignments, and
unmount-safe abort handling required by the effect.
In `@app/admin/api/login/route.ts`:
- Around line 70-75: Update the forbidden-origin response in the
hasTrustedOrigin branch to include a Cache-Control: no-store header, matching
the other authentication responses while preserving the existing 403 status and
error payload.
In `@app/admin/api/web-search-candidates/route.ts`:
- Around line 55-126: Update the GET handler around listWebSearchCandidates to
accept and validate a status filter, page limit, and cursor from the request,
returning only the requested candidate page instead of loading the full dataset.
Extend listWebSearchCandidates and its query flow to apply status filtering and
cursor-based limiting, scope revisions and sources to the page’s candidate IDs,
and avoid loading history by default unless explicitly requested; include the
next cursor in the response when more results remain.
- Around line 142-152: Strengthen validation in the candidateId and
expectedVersion parsing block: require candidateId to match the canonical UUID
layout with hexadecimal groups separated by hyphens, and only convert
expectedVersion after confirming the raw field is a number, rejecting null,
strings, booleans, arrays, and omitted values before applying the existing
integer and non-negative checks. Preserve the existing 400 response for invalid
input.
- Around line 19-54: Update authorize to return a discriminated result: denied
responses must always include a non-null Response with actor null, while
successful authorization must include actor and a null denied value. In GET and
the other handlers such as PATCH around the referenced branch, branch on denied
and return only that response before accessing the guaranteed actor, eliminating
any path that returns null.
In `@app/api/chat/route.ts`:
- Around line 259-285: Update the web-search budget flow around
reserveWebBudget, webSearch, and settleWebBudget so every exception after a
successful reservation releases the reserved tokens before propagating to the
existing error handling. Ensure both provider-call failures and settleWebBudget
failures trigger cleanup, while preserving the current settlement behavior for
successful calls and the existing unavailable responses.
In `@db/index.ts`:
- Around line 121-188: The bootstrap schema around createLegalProvisionsTable is
missing the update-path guard enforced by
legal_provisions_rag_metadata_update_check in migration 0002. Add the equivalent
trigger to the bootstrap initialization, ensuring published legal_provisions
rows cannot update revision_id, checksum metadata, effectivity_status, or the
effective date window while remaining published, including transitions into
published within the same statement; preserve
createLegalProvisionsRevisionImmutableTrigger for its existing coverage.
- Around line 17-34: The bootstrap path around the legal_entries schema and
subsequent d1.batch trigger creation is unsafe for pre-existing databases
because CREATE TABLE IF NOT EXISTS does not add the review columns. Add
idempotent ALTER TABLE handling for review_status, created_by, reviewed_by, and
reviewed_at before creating triggers, or gate bootstrap-dependent routes until
the migration has run; preserve the existing schema constraints and trigger
setup for current databases.
In `@db/schema.ts`:
- Around line 343-347: Add a concise comment above the
editorial_role_grants_revocation_check in the schema explaining that
revoked_by_principal_id may temporarily be set while revoked_at is null because
the editorial_role_grants_set_revoked_at trigger fills the timestamp; preserve
the existing constraint unchanged.
- Around line 692-834: Add the missing Drizzle check(...) definitions to
webSearchCandidates, webSearchCandidateSources, webSearchCandidateRevisions,
webSearchCandidateEvents, and webSearchBudgetDays, mirroring every named
constraint from the existing workflow migration, including reviewer/editor
separation and host validation. Regenerate
drizzle/0006_petite_lady_deathstrike.sql for the affected table definitions and
drizzle/meta/0006_snapshot.json so the migration and snapshot preserve those
checks.
- Around line 725-728: Update the Drizzle index definitions for
web_search_candidates_status_updated_idx,
web_search_candidate_revisions_candidate_idx, and
web_search_candidate_events_candidate_idx to preserve their DESC column ordering
using raw SQL expressions or synchronized schema and migration changes. Ensure
generated migrations retain updatedAt, version, and occurredAt as descending
columns.
In `@docs/ADMIN_CREDENTIAL_ROTATION.md`:
- Around line 3-16: Update the admin credential rotation runbook to describe the
multi-account registry via ADMIN_ACCOUNTS_JSON, including its format,
principalId requirement, and precedence over the legacy variables
ADMIN_USERNAME, ADMIN_PASSWORD_HASH, and ADMIN_PRINCIPAL_ID. Replace the
single-admin scope and document that at least two distinct principals are
required for four-eyes approval/rejection; record the same registry, precedence,
and minimum-principal decision in docs/TECHNICAL_SPEC.md.
- Around line 38-52: Add RATE_LIMIT_KEY_SECRET and the D1 DB binding to the
required configuration section in the credential rotation runbook, specifying
that the secret must be at least 32 bytes. Include a rotation note stating that
changing RATE_LIMIT_KEY_SECRET rehashes all rate-limit keys and resets active
windows and penalties.
In `@docs/MIGRATION_RUNBOOK.md`:
- Line 38: Update the migration runbook sections covering scope, artifact
listing, activation gates, database checks, rollback procedures, and metadata so
migration 0006 is consistently included alongside 0001–0005, including its SQL
and verification evidence. If 0006 is intentionally excluded, explicitly
document the exclusion and deployment boundary instead, and remove conflicting
counts such as “all three SQL files.”
In `@docs/PROGRESS.md`:
- Around line 21-29: Update the aggregate progress row in the summary table to
reflect the story table’s verified counts: 9 Done, 19 Partial, 0 Todo, 0
Blocked, and 28 total. Reconcile the category assignments against the 28 listed
stories before treating the table as tracker evidence, and ensure product review
verifies each story’s status.
- Around line 77-78: Escape the pipe characters inside the inline code values in
the DEC-008 and DEC-009 decision-log rows so Markdown treats them as literal
content rather than table separators. Preserve the existing decision text and
table structure.
In `@docs/TECHNICAL_SPEC.md`:
- Around line 1472-1476: Correct the subsection heading “7.2.1 Web-search
candidate persistence và reviewed promotion” to “7.6.1” so it matches its parent
section and keeps references to candidate persistence reliable.
- Around line 599-601: Remove the “final review không còn blocker/high/medium”
status claim from the evidence entry in TECHNICAL_SPEC.md, while retaining the
test, TypeScript, ESLint, and build results. Keep review status tracking
separate and update it only after the review is complete.
- Around line 2209-2214: Resolve the persistence policy conflict between Section
7.6.1 and DEC-010/DEC-011 in docs/TECHNICAL_SPEC.md by selecting and documenting
one canonical rule. If draft persistence is intended, explicitly state that only
an unreviewed draft candidate is persisted, the raw question is excluded, and
human review is required before retrieval use; update the related
implementation, tests, and runbook to match, and record the final security and
rollout behavior unambiguously.
- Around line 593-597: Add explicit first-admin bootstrap authorization details
to the grant and audit specification: require a one-time out-of-band
authorization, record a bootstrap audit event, and define a permanent closed
state after bootstrap. Clarify that this is not runtime self-admin creation and
align the wording with the migration runbook’s lack of seeded credentials and
automatic application.
- Around line 54-64: Update the `/api/chat` description in the As-is
documentation to include the enabled `AI_WEB_SEARCH_ENABLED` flow, including
`web_search_candidates` persistence and the `mode: "web_search"` response. Align
the related flow and safety notes, or explicitly mark Sections 7.6 and `DEC-011`
as proposed instead of presenting them as current behavior.
- Around line 267-291: Clarify the `legal_sources` `official_host` constraint to
require lowercase hosts with no empty labels, consecutive dots, or
leading/trailing dots. State that `official_url` must contain an authority
exactly matching `official_host`, followed only by `/`, `?`, or `#`, while
preserving DEC-004’s canonical allowed-domain rules for non-draft sources.
In `@docs/THIRD_PARTY_DATA_ASSESSMENT.md`:
- Around line 18-20: Update the direct-search policy in the DEC-010 section and
the AI_WEB_SEARCH_ENABLED description to align activation and retention: public
fallback may run only when the server-validated eligibility conditions are met,
and eligible results must persist as immutable D1 drafts without the raw
question. Explicitly preserve that these results never enter the reviewed corpus
or auto-publish, and remove wording that says direct-search results are never
persisted or that the flag is only for backoffice discovery.
In `@docs/USER_STORIES.md`:
- Around line 1217-1218: Update the historical smoke-result entry in
USER_STORIES.md that references gpt-5.6-sol: remove it or clearly label it with
the prior policy and execution date. Prefer replacing it with the current smoke
evidence using the allowed pinned snapshot documented in the nearby acceptance
criteria.
In `@drizzle/0001_citation_foundation.sql`:
- Around line 98-115: The trigger definition for
legal_sources_invalidate_published_provisions should restrict execution to
updates of status, effective_from, last_verified_at, verified_by, or created_by
by adding the corresponding UPDATE OF column list, while preserving the existing
WHEN condition and provision invalidation logic.
In `@drizzle/0004_rate_limit_v1.sql`:
- Around line 30-31: Add a scheduled purge path for expired rows in both
rate-limit tables, deleting records where expires_at <= the current cutoff in
bounded batches. Configure the purge as a Cron Trigger or existing scheduled-job
mechanism, and document its schedule, batching, and operational guidance in
docs/OBSERVABILITY_RUNBOOK.md.
In `@drizzle/0005_web_search_candidate_workflow.sql`:
- Around line 147-182: Add a BEFORE INSERT trigger for
web_search_candidate_events that validates any non-null revision_id belongs to
the same candidate_id, rejecting mismatched pairs with the established error
behavior used by web_search_candidates_revision_binding_check. Locate the
referenced web_search_candidate_revisions and web_search_candidates identifiers
in the migration and preserve valid inserts and nullable revision_id behavior.
In `@drizzle/0006_petite_lady_deathstrike.sql`:
- Around line 443-475: Recreate the four review
triggers—legal_entries_review_insert_check, legal_entries_review_update_check,
legal_entries_created_by_immutable, and
legal_entries_material_change_invalidates_review—at the end of the migration
after legal_entries is renamed, preserving their existing validation,
immutability, and citation-invalidation behavior.
- Line 1: Remove all duplicate CREATE TABLE statements from migration 0006 for
legal_sources, legal_entry_citations, legal_provisions, editorial_*,
rate_limit_*, and web_search_* tables, preserving only the legal_entries rebuild
delta and its required statements.
In `@drizzle/meta/_journal.json`:
- Around line 47-52: Update the journal entry with idx 6 and tag
0006_petite_lady_deathstrike so its when timestamp is greater than
1785747600000, preserving the existing migration index, tag, version, and
breakpoints values.
- Around line 11-46: Add the missing Drizzle metadata snapshots
0001_snapshot.json through 0005_snapshot.json under the migration metadata,
matching the schema state represented by journal entries
0001_citation_foundation through 0005_web_search_candidate_workflow. Ensure each
snapshot corresponds to its journal index and preserves the migration sequence
used by drizzle-kit generate/check.
In `@drizzle/meta/0006_snapshot.json`:
- Around line 4-5: Restore the missing intermediate migration snapshots in
drizzle/meta, specifically 0001_snapshot.json through 0005_snapshot.json, so
they match the migrations listed by _journal.json. Ensure each snapshot
represents its predecessor schema in sequence, then regenerate the 0006 drift
against 0005_snapshot.json rather than leaving only the initial and final
snapshots.
In `@lib/admin-auth.ts`:
- Around line 27-37: Move the canonical toBase64Url/fromBase64Url
implementations from lib/password-hash.ts lines 9-26 into a shared module,
including the round-trip validation, then remove both local helpers from
lib/admin-auth.ts lines 27-37 and lib/password-hash.ts lines 9-26 and import the
shared functions in both files so both decoders behave identically.
- Around line 118-136: Update validateAdminCredentials to resolve the username
across accountRegistry() using safeEqual, then verify exactly one password hash:
the matched account’s hash or a module-level decoyPasswordHash in the same
encoded format. Preserve the existing input validation and return success only
when both the username match and single password verification succeed.
In `@lib/catalog-resolver.ts`:
- Around line 436-449: Update the suppression-row sort in
canonicalSuppressionSnapshotPayload to use deterministic plain codepoint
ordering instead of String.prototype.localeCompare. Compare the stringified
content keys with direct relational ordering, returning negative, positive, or
zero as appropriate, while preserving the existing row construction and sort
key.
In `@lib/image-intent.ts`:
- Around line 110-111: Update the phrase classification in the image-intent
logic so “bi lay dung” and “bi su dung” are not treated as standalone consent
signals that set non_consensual_sharing. Require an additional privacy signal
before assigning that intent, or route these phrases through a separate reuse
classification that contributes to copyrightReasons, preserving privacy
precedence only for genuine privacy cases.
- Around line 145-151: Update withoutPhrases to remove every adjacent and
repeated occurrence of each phrase, including inputs such as “ảnh hưởng ảnh
hưởng”, without allowing the replacement to consume the separator needed by the
next match. Preserve the existing trimming and whitespace normalization behavior
so hasPhrase receives the fully cleaned text.
In `@lib/ingestion-local.ts`:
- Around line 306-318: Update the idempotency-key derivation around
canonicalFixtureParts and its corresponding path at the additional location to
include request.createdBy alongside the existing plan-defining fields, ensuring
different authors produce different keys. Because this changes the encoding,
bump the encoding label and local-fixture-sha256-v2 prefix consistently so
existing keys remain distinguishable.
In `@lib/legal-chat.ts`:
- Around line 4-10: Replace the positional destructuring of laws in
lib/legal-chat.ts with stable id-based lookups for helmetLaw and
falseInformationLaw. Validate that both curated entries exist and fail loudly
when either is missing, ensuring legal citations continue to use the reviewed
laws data regardless of array order or membership changes.
In `@lib/legal-content.ts`:
- Around line 88-94: Expand hasBlockedLegalBasis to reject every supported
free-text spelling of decree 131/2013/NĐ-CP, including variants without the
current prefixes and with common Vietnamese formatting or diacritic differences;
add coverage for each rejected spelling. Keep matching based on the normalized
compact value, or introduce structured document-number matching if already
supported, while preserving non-matching legal bases.
In `@lib/legal-evidence-retriever.ts`:
- Around line 1115-1127: Update the checksum verification around
computeProvisionChecksum to run independently for all metadataEligible
candidates concurrently rather than awaiting each digest sequentially, while
preserving the current result order, eligibility filtering, and DEPENDENCY_ERROR
handling. Do not change ranking or candidate selection behavior in this minimal
fix.
In `@lib/openai-evidence.ts`:
- Around line 391-395: Update the numeric guard in the composition validation
block using a Unicode-aware decimal-number check with the Nd property, so
fullwidth, Arabic-Indic, Devanagari, and other Unicode decimal digits return
NUMERIC_MISMATCH. Preserve the existing getAllCompositionText(composition) flow
and consider including No characters only if the presentation layer treats them
as numeric references.
In `@lib/openai-web-search.ts`:
- Around line 395-406: Update the citation loop in the final answer handling to
skip citations for which canonicalOfficialSourceUrl returns no URL instead of
returning UNTRUSTED_CITATION immediately. Continue collecting valid official
sources up to MAX_SOURCES, and retain the existing MISSING_OFFICIAL_CITATION
failure when no trusted citations remain.
- Around line 214-248: Update the text parsing flow around textItems and
citation validation to retain each content.text value unchanged while validating
url_citation start_index and end_index against the raw generated text length.
After citation offsets are validated and citations are built, trim the text only
for the returned answer, preserving valid citations affected by leading or
trailing whitespace.
In `@lib/public-showcase.ts`:
- Around line 131-155: Restrict public law data to an explicit allowlist: add
and use projectPublicLaw in createPublicContentHandler alongside
projectPublishedShowcases, excluding editorial fields such as createdBy,
reviewedBy, reviewedAt, and reviewStatus. In app/api/content/route.ts lines
8-19, replace db.select() with explicit legalEntries and showcases column
selections, retaining status because the projections require it.
In `@lib/rate-limit.ts`:
- Around line 383-425: Resolve the request ID once in each login flow and reuse
it throughout: update beforeLogin, recordLoginFailure, and resetLoginPair
together with loginIdentity so generated IDs remain consistent across events and
response headers. Also make the clientCount, accountCount, and pairAttemptCount
request_count reads use the same ?? 0 fallback convention.
- Around line 344-350: Remove the unconditional cleanupStatements calls from the
request paths used by beforeLogin, recordLoginFailure, resetLoginPair, and
consumeChat, while preserving their rate-limit checks and other batch
operations. Run cleanupStatements only through a periodic Cron Trigger or an
explicit low-frequency gate such as the existing time value once per minute,
ensuring normal rate-limit decisions do not issue the cleanup DELETE statements
on every request.
In `@lib/source-registry.ts`:
- Around line 123-130: Create one exported DEC-004 host validator containing the
exact official hosts and `.chinhphu.vn` suffix behavior, then replace the local
checks in `isAllowedOfficialHost` and `lib/legal-evidence-retriever.ts` with
that shared validator. Reuse the same validator in showcase code and the related
SQL migration/schema constraints so all allowlists and suffix handling remain
consistent.
- Around line 331-446: Deep-freeze sourceRegistry at export time so the registry
array, every record, and nested mutable arrays such as allowedHosts, fields,
formats, and risks cannot be modified at runtime. Add or reuse a recursive
freeze helper near the registry definition, apply it to the exported value, and
preserve the existing readonly SourceRegistryRecord[] and as const typing.
In `@lib/web-search-candidates.ts`:
- Around line 680-707: Update findReviewedWebCandidate to use a term-indexed
lookup for the normalized query terms instead of scanning all published
candidates and sources. Select only matching candidate IDs, constrain both
candidate and source retrieval to those IDs, and preserve the existing
validation and scoring behavior for the selected candidates.
In `@lib/worker-observability.ts`:
- Around line 44-59: Update the catch block around dependencies.handler.fetch to
bind the caught exception, log it with the requestId and relevant error details,
then preserve the existing generic 500 Response.json fallback unchanged.
In `@package.json`:
- Around line 12-24: Update the aggregate test script in package.json to run
Node’s test runner with --experimental-strip-types, matching the individual
TypeScript-importing test scripts and supporting the declared Node >=22.13.0
range.
In `@scripts/shadow-openai-evidence.mjs`:
- Around line 2-4: Add tsx to the root package.json devDependencies and update
package-lock.json accordingly, preserving the locked version and ensuring the
dependency used by the register import in scripts/shadow-openai-evidence.mjs is
available after a clean install.
In `@scripts/smoke-openai-web-search.mjs`:
- Around line 1-14: Update the loader setup in the script to avoid using
registerHooks, which requires Node.js 22.15.0+, and use a loader API compatible
with the project’s declared Node.js target; alternatively, raise and enforce the
minimum Node.js version to 22.15.0 or later.
In `@tests/admin-auth.test.mjs`:
- Around line 63-179: Add a test.beforeEach hook in the admin-auth test suite
that resets the shared workerEnv via setWorkerEnv(validConfig) before every
test. Keep the existing per-test environment mutations and assertions unchanged,
ensuring credential and session tests start from an isolated valid
configuration.
In `@tests/ai-shadow.test.mjs`:
- Around line 190-211: Strengthen the test around runAiShadowBatch so it
verifies AI_SHADOW_MAX_CASES actually limits processing, rather than only
matching the fixture size. Use a fixture containing more than 20 cases or
configure a cap below fixture.cases.length, then assert the processed
calls/attempted count and skippedByLimit reflect the configured cap.
In `@tests/catalog-resolver.test.mjs`:
- Around line 548-577: Update the test “resolver snapshots input, does not
mutate caller data and freezes output” to verify the actual staticRecords array
passed to resolveCatalog remains unchanged: snapshot it before invocation,
compare staticRecords against that snapshot after resolution, and use a separate
array for the caller-side reverse mutation. Remove the ineffective comparison
against fixture.records while preserving the existing result and freezing
assertions.
In `@tests/editorial-workflow-schema.test.mjs`:
- Around line 739-747: Update the assert.throws expectations in the editorial
review request tests, including the additional block around the four-eyes case,
to match only the specific guard message each scenario should produce. Use the
exact “review request must bind the current subject revision” message for the
replay test and the exact “active independent reviewer” message for the
four-eyes test, removing both alternations.
- Around line 380-395: Update the test cases around the editorial_subjects
inserts to assert the distinct rejection reason for each scenario: keep the
authorization message for disabled or missing-role creators, and expect the
separate state-transition or optimistic-version initialization messages for the
published and nonzero-version cases. Split the shared loop or its assertions so
each insert validates the appropriate guard message.
- Around line 577-622: The rejected submission assertions in the test “request
submission requires draft/current revision and an active editor or admin” should
match the actor-specific authorization error rather than the generic
current-revision binding message. Update each relevant assert.throws
expectation, including no-role, other-editor, and disabled-editor cases, to
assert the message produced when the submitter is not an active draft
creator/editor/admin, while preserving the existing state and zero-insert
checks.
In `@tests/image-intent.test.mjs`:
- Around line 6-35: Extract the duplicated registerHooks module-resolution logic
into a shared helper, such as tests/helpers/register-loader.mjs, exporting a
function that accepts the worker environment global name and returns the
environment object while preserving all existing resolution mappings. Replace
the hooks in tests/image-intent.test.mjs:6-35 with the helper using
__imageIntentWorkerEnv, tests/openai-web-search.test.mjs:7-36 using
__webSearchWorkerEnv, tests/rate-limit.test.mjs:8-37 using __rateLimitWorkerEnv,
and tests/telemetry.test.mjs:6-35 using __telemetryWorkerEnv.
In `@tests/legal-evidence-retriever.test.mjs`:
- Around line 320-329: Update the test name to match the assertions in the test
named “date and TTL boundaries are inclusive and future review is rejected,”
removing the “future review is rejected” clause since future-timestamp behavior
is already covered by separate tests. Preserve the existing inclusive boundary
setup and eligibility assertion.
In `@tests/openai-web-search.test.mjs`:
- Around line 502-621: Move cleanup of globalThis.__webSearchWorkerEnv into a
test.afterEach hook so it runs even when assertions fail. Add the hook near the
test setup, then remove the trailing key-deletion loops from both tests while
preserving their existing environment configuration and assertions.
In `@tests/public-showcase.test.mjs`:
- Around line 172-177: Update the assertions in the loop over items to verify
title, summary, and topic using string-containment checks rather than
constructing RegExp instances from fixture values; keep the existing showcase ID
assertion unchanged.
In `@tests/rate-limit.test.mjs`:
- Around line 518-528: Correct the over-escaped regex alternatives in the leak
assertions: in tests/rate-limit.test.mjs lines 518-528, replace the doubled
escaping for 203.0.113 with single regex escaping; make the same 203.0.113
correction in tests/telemetry.test.mjs lines 166-166, and correct evil.example
in tests/telemetry.test.mjs lines 529-532. Match the already-correct pattern at
line 298 of tests/telemetry.test.mjs.
In `@tests/rendered-html.test.mjs`:
- Around line 43-58: Update the rate_limit_buckets mock in the INSERT INTO
rate_limit_buckets branch to clamp request_count with the configured cap using
Math.min((current?.request_count ?? 0) + 1, cap), allowing the count to reach
but not exceed cap and preserving the production rate-limit behavior.
In `@tests/schema-foundation.test.mjs`:
- Around line 139-145: Update the chinhphu.vn assertion in the migration test to
match the exact allowlist expression used by the migration, removing the broad
alternation that independently matches any chinhphu.vn occurrence. Preserve
coverage for the intended wildcard/subdomain form.
---
Outside diff comments:
In `@app/page.tsx`:
- Around line 83-107: Decouple law and showcase parsing in the effect around
setManagedLaws and setManagedShowcases: update managed laws from content.laws
independently before handling showcase validation, and ensure an invalid
parsedShowcases result only clears/degrades showcase state without discarding
law data or triggering the bundled-laws fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 68dcc1b2-c5d5-4d44-af91-8357baaeb3a9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (80)
.env.exampleAGENTS.mdREADME.mdapp/admin/AdminDashboard.tsxapp/admin/admin.cssapp/admin/api/content/route.tsapp/admin/api/login/route.tsapp/admin/api/web-search-candidates/route.tsapp/api/chat/route.tsapp/api/content/route.tsapp/globals.cssapp/page.tsxcloudflare-env.d.tscomponents/ShowcaseGallery.tsxdb/index.tsdb/schema.tsdocs/ADMIN_CREDENTIAL_ROTATION.mddocs/CODE_REVIEW.mddocs/INGESTION_LOCAL_RUNBOOK.mddocs/MIGRATION_RUNBOOK.mddocs/OBSERVABILITY_RUNBOOK.mddocs/PRODUCT_REQUIREMENTS.mddocs/PROGRESS.mddocs/SOURCE_REGISTRY.mddocs/TECHNICAL_SPEC.mddocs/THIRD_PARTY_DATA_ASSESSMENT.mddocs/USER_STORIES.mddocs/WEB_SEARCH_REVIEW_RUNBOOK.mddrizzle/0001_citation_foundation.sqldrizzle/0002_reviewed_rag_bridge.sqldrizzle/0003_editorial_trust_primitives.sqldrizzle/0004_rate_limit_v1.sqldrizzle/0005_web_search_candidate_workflow.sqldrizzle/0006_petite_lady_deathstrike.sqldrizzle/meta/0006_snapshot.jsondrizzle/meta/_journal.jsonfixtures/ai-shadow/cases.v1.jsonfixtures/catalog/static-catalog.v1.jsonfixtures/source-registry/vbpl-nd168.sample.jsonlib/admin-auth.tslib/ai-shadow.tslib/catalog-resolver.tslib/chat-answer-presentation.tslib/image-intent.tslib/ingestion-local.tslib/legal-chat.tslib/legal-content.tslib/legal-evidence-retriever.tslib/official-source-url.tslib/openai-evidence.tslib/openai-web-search.tslib/password-hash.tslib/public-showcase.tslib/rate-limit.tslib/source-registry.tslib/telemetry.tslib/web-search-candidates.tslib/worker-observability.tspackage.jsonscripts/generate-admin-password-hash.mjsscripts/shadow-openai-evidence.mjsscripts/smoke-openai-evidence.mjsscripts/smoke-openai-web-search.mjstests/admin-auth.test.mjstests/ai-shadow.test.mjstests/catalog-resolver.test.mjstests/editorial-workflow-schema.test.mjstests/image-intent.test.mjstests/ingestion-local.test.mjstests/legal-evidence-retriever.test.mjstests/openai-evidence.test.mjstests/openai-web-search.test.mjstests/public-showcase.test.mjstests/rate-limit.test.mjstests/rendered-html.test.mjstests/schema-foundation.test.mjstests/source-registry.test.mjstests/telemetry.test.mjstests/web-search-candidates.test.mjsworker/index.ts
| That path is prohibited until the repository has an explicit, reviewed Wrangler | ||
| configuration and a verified environment-to-database mapping. | ||
|
|
||
| ## 2. Scope of migrations 0001–0005 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the runbook for every repository migration.
The runbook stops at migration 0005. The repository progress records migration 0006. The metadata section also lists only 0001–0004 and “all three SQL files,” which conflicts with the earlier artifact list.
An operator can activate code without applying or verifying migration 0006. Update the scope, artifact list, activation gate, database checks, rollback instructions, and metadata section. If 0006 is intentionally excluded, document that exclusion and its deployment boundary.
Based on learnings: full-stack implementation must keep documentation and migration/test evidence consistent.
Also applies to: 130-146, 289-299, 316-322
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/MIGRATION_RUNBOOK.md` at line 38, Update the migration runbook sections
covering scope, artifact listing, activation gates, database checks, rollback
procedures, and metadata so migration 0006 is consistently included alongside
0001–0005, including its SQL and verification evidence. If 0006 is intentionally
excluded, explicitly document the exclusion and deployment boundary instead, and
remove conflicting counts such as “all three SQL files.”
Source: Learnings
Comment on lines
+21
to
+29
| | Nhóm | Done | Partial | Todo | Blocked | | ||
| |---|---:|---:|---:|---:| | ||
| | Tra cứu và hiểu pháp luật | 3 | 2 | 0 | 0 | | ||
| | Hỏi đáp có kiểm soát | 2 | 3 | 0 | 0 | | ||
| | Quản trị nội dung | 3 | 2 | 0 | 0 | | ||
| | Dữ liệu và nguồn | 0 | 3 | 0 | 0 | | ||
| | Bảo mật, vận hành, chất lượng | 1 | 4 | 0 | 0 | | ||
| | RAG và nhập dữ liệu ngoài | 0 | 4 | 0 | 0 | | ||
| | **Tổng** | **9** | **18** | **0** | **0** | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the progress summary with the story table.
The story table lists 28 stories. It contains 9 Done stories and 19 Partial stories. The summary reports 9 Done, 18 Partial, and 27 total.
Update the aggregate counts and reconcile category assignments before using this table as tracker evidence.
As per coding guidelines: progress totals must match verified acceptance evidence. Based on learnings: product review must check story status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PROGRESS.md` around lines 21 - 29, Update the aggregate progress row in
the summary table to reflect the story table’s verified counts: 9 Done, 19
Partial, 0 Todo, 0 Blocked, and 28 total. Reconcile the category assignments
against the 28 listed stories before treating the table as tracker evidence, and
ensure product review verifies each story’s status.
Sources: Coding guidelines, Learnings
Comment on lines
+77
to
+78
| | 2026-07-31 | DEC-008 | Catalog snapshot có `available_records|available_empty|unavailable`; success-empty là static overlay ready, unavailable mới degraded 200/no-store; reviewed suppression chặn static resurrection. | US-017 | | ||
| | 2026-07-31 | DEC-009 | AI integration đầu tiên chỉ là offline/local shadow, không import chat/API; `AI_SHADOW_ENABLED=false`, exact allowlist `gpt-5.4-mini|gpt-5.4-mini-2026-03-17`, `store:false`, không web/tool/persist. Route shadow chờ production bundle + `waitUntil` seam. | US-026, US-004 | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape pipe characters in the decision log.
The inline code values in DEC-008 and DEC-009 contain |. Markdown table parsers treat these characters as cell separators. The rendered table can lose part of each decision.
Escape the pipes or use a delimiter that does not contain |.
Based on static analysis: markdownlint detects extra table columns at these lines.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 77-77: Table column count
Expected: 4; Actual: 6; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 78-78: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 78-78: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 78-78: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PROGRESS.md` around lines 77 - 78, Escape the pipe characters inside the
inline code values in the DEC-008 and DEC-009 decision-log rows so Markdown
treats them as literal content rather than table separators. Preserve the
existing decision text and table structure.
Source: Linters/SAST tools
Comment on lines
+54
to
+64
| | Trang công khai | Next.js client UI, tìm kiếm/lọc và chat | `app/page.tsx` | | ||
| | Nội dung nền | Điều luật, nguồn và ví dụ hard-code | `lib/legal-content.ts` | | ||
| | Public content API | Đọc `law`/`showcase` đã `published`; trả mảng rỗng khi D1 lỗi | `app/api/content/route.ts` | | ||
| | Chat API | Managed knowledge → hard-code knowledge → fail-closed | `app/api/chat/route.ts` | | ||
| | Retrieval | Chấm điểm keyword đơn giản trên tối đa 100 bài published | `lib/legal-chat.ts` | | ||
| | CMS API | CRUD trực tiếp qua Drizzle; trạng thái `draft`/`published` | `app/admin/api/content/route.ts` | | ||
| | Admin auth | Một credential từ env, cookie phiên ký HMAC, TTL 8 giờ | `lib/admin-auth.ts` | | ||
| | Data layer | Drizzle ORM trên Cloudflare D1; citation/reviewed-RAG bridge đã có schema nhưng chưa nối read/write API | `db/`, `drizzle/0001_citation_foundation.sql`, `drizzle/0002_reviewed_rag_bridge.sql` | | ||
| | Runtime | Next.js qua Vinext/Cloudflare Worker | `worker/index.ts` | | ||
| | AI provider | Chưa có runtime consumer; `.env.example` mới chỉ ghi biến dự kiến | Chưa triển khai | | ||
| | Ingestion/index | Chưa có connector, raw staging/quarantine, scheduler hoặc FTS5 table | Chưa triển khai | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'AI_WEB_SEARCH_ENABLED|mode.?web_search|openai-web-search|web_search_candidates|findManagedAnswer|findCuratedAnswer|mode.?unavailable' \
--glob '*.ts' --glob '*.tsx' --glob '*.mjs' --glob '*.md'Repository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo files (relevant names):\n'
git ls-files | rg '^(docs/TECHNICAL_SPEC\.md|app/api/chat/route\.ts|lib/legal-chat\.ts|\.env\.example|worker/index\.ts|package\.json)$' || true
printf '\ndocs/TECHNICAL_SPEC.md line count:\n'
wc -l docs/TECHNICAL_SPEC.md
printf '\nAround docs/TECHNICAL_SPEC.md lines 45-85:\n'
sed -n '45,85p' docs/TECHNICAL_SPEC.md
printf '\nChat route outline if exists:\n'
ast-grep outline app/api/chat/route.ts --view expanded 2>/dev/null || true
printf '\nChat route lines:\n'
cat -n app/api/chat/route.ts
printf '\nSearch docs for chat/web/search/decisions:\n'
rg -n -i \
'chat|web search|web_search|AI_WEB_SEARCH|search|openai|provider|DEC-011|7\.6|Section 7\.6' \
docs/TECHNICAL_SPEC.mdRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 29617
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
doc = Path("docs/TECHNICAL_SPEC.md").read_text()
routes = Path("app/api/chat/route.ts").read_text()
checks = {
"section_3_1_describes_chat_no_ai": "không gọi AI provider" in doc[doc.index("3.1 Thành phần"):doc.index("3.2 Luồng chat hiện tại")],
"section_3_2_no_provider_call": "không gọi AI provider" in doc[doc.index("### 3.2 Luồng chat hiện tại"):doc.index("Điểm an toàn đang có:")],
"section_7_6_mentions_web_search": "web_search" in doc[doc.index("7.6 OpenAI provider contract"):doc.index("### 7.2.1 Web-search candidate persistence")],
"section_7_2_1_describes_persistence": "lưu draft candidate",
}
print("doc checks:")
for k, needle in checks.items():
print(f"{k}: {needle if isinstance(needle, bool) else ('True' if isinstance(needle, str) and needle in doc else 'False')}")
print("\nroute runtime facts:")
print("imports web-search helpers:", all(s in routes for s in [
"readOpenAiWebSearchConfig",
"searchAllowedLegalSources",
"findReviewedWebCandidate",
"reserveWebSearchBudget",
"settleWebSearchBudget",
"persistWebSearchCandidate",
]))
print("checks AI_WEB_SEARCH_ENABLED:", "readOpenAiWebSearchConfig(env)" in routes and "!webSearchConfig.enabled" in routes)
print("calls provider adapter:", "searchAllowedLegalSources(" in routes)
print("returns web_search mode:", "mode: \"web_search\"" in routes)
print("persists before return:", "await persistWebCandidate(" in routes)
PYRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 526
Align the /api/chat As-is description with the web-search flow.
docs/TECHNICAL_SPEC.md says /api/chat currently returns only knowledge or unavailable, while app/api/chat/route.ts enables AI_WEB_SEARCH_ENABLED, persists web_search_candidates, and returns mode: "web_search". Update the As-is table/flow and safety notes to include the web-search path, or mark Sections 7.6 and DEC-011 as proposed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/TECHNICAL_SPEC.md` around lines 54 - 64, Update the `/api/chat`
description in the As-is documentation to include the enabled
`AI_WEB_SEARCH_ENABLED` flow, including `web_search_candidates` persistence and
the `mode: "web_search"` response. Align the related flow and safety notes, or
explicitly mark Sections 7.6 and `DEC-011` as proposed instead of presenting
them as current behavior.
Sources: Coding guidelines, Learnings
Comment on lines
+267
to
+291
| ### 5.1 `legal_sources` | ||
|
|
||
| | Field | Kiểu đề xuất | Ràng buộc | | ||
| |---|---|---| | ||
| | `id` | integer | PK | | ||
| | `document_number` | text | required, ví dụ `168/2024/NĐ-CP` | | ||
| | `title` | text | required | | ||
| | `official_url` | text | required, HTTPS, domain được cho phép | | ||
| | `official_host` | text | required, lowercase host không chứa ký tự ngoài `a-z0-9.-` hoặc `..` | | ||
| | `issued_at` | text/date | nullable | | ||
| | `effective_from` | text/date | required trước publish | | ||
| | `effective_to` | text/date | nullable | | ||
| | `status` | text | `draft`, `in_force`, `expired`, `superseded` | | ||
| | `created_by` | text/integer | required | | ||
| | `last_verified_at` | text/datetime | required trước publish | | ||
| | `verified_by` | text/integer | required khi `in_force`; khác `created_by` | | ||
| | `created_at`, `updated_at` | text/datetime | required | | ||
|
|
||
| `official_url` không được dùng để tự parse host trong SQL. Biên tập/API phải lưu | ||
| host đã canonicalize riêng vào `official_host`; database buộc authority ngay sau | ||
| `https://` phải khớp chính xác field này và chỉ cho delimiter kết thúc, `/`, `?` | ||
| hoặc `#`. Source khác `draft` chỉ chấp nhận `official_host` bằng `vbpl.vn`, | ||
| `vbpl.moj.gov.vn`, `chinhphu.vn` hoặc có suffix `.chinhphu.vn` (DEC-004). | ||
| Thiết kế này chặn suffix/path/query/fragment giả như | ||
| `https://evil.example?.chinhphu.vn/...`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 -i \
'official_host|officialHost|parseOfficialSource|authority|DEC-004|allowed_hosts' \
--glob '*.ts' --glob '*.sql' --glob '*.mjs' --glob '*.md'Repository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked docs files:\n'
git ls-files 'docs/*.md' | sort
printf '\nTECHNICAL_SPEC relevant section:\n'
if [ -f docs/TECHNICAL_SPEC.md ]; then
nl -ba docs/TECHNICAL_SPEC.md | sed -n '240,310p'
fi
printf '\nGit status short:\n'
git status --short
printf '\nAll text matches in repository (file-limited): \n'
rg -n -C 4 -i 'official_host|officialHost|parseOfficialSource|authority|DEC-004|allowed_hosts|legal_sources|vbpl\.vn|chinhphu\.vn' --glob '*.ts' --glob '*.sql' --glob '*.mjs' --glob '*.md' --glob '*.js' --glob '*.json' --glob '*.yaml' --glob '*.yml' . || true
printf '\nFind likely source files names:\n'
git ls-files | rg '\.(ts|js|sql|mjs)$' | sed -n '1,200p'Repository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 598
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
patterns = {
"spec_consecutive_dot_allowed_like_read": r"\.a-z0-9.-",
"simple_authority_regex_from_text_like": None,
}
hosts = [
"vbpl.vn",
"vbpl.moj.gov.vn",
"chinhphu.vn",
"src.example.chinhphu.vn",
"evil.example?.chinhphu.vn",
"example..com",
".example.com",
"example.com.",
"example.com",
"",
"example..con",
]
print("spec_consecutive_dot_allowed_like_read will be matched against by Python code.")
print("No regex implementation exists yet from repository source, but this probe checks candidate host shapes.")
PYRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 361
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked docs files:\n'
git ls-files 'docs/*.md' | sort
printf '\nTECHNICAL_SPEC relevant section:\n'
if [ -f docs/TECHNICAL_SPEC.md ]; then
nl -ba docs/TECHNICAL_SPEC.md | sed -n '240,310p'
fi
printf '\nGit status short:\n'
git status --short
printf '\nAll text matches in repository (file-limited): \n'
rg -n -C 4 -i 'official_host|officialHost|parseOfficialSource|authority|DEC-004|allowed_hosts|legal_sources|vbpl\.vn|chinhphu\.vn' --glob '*.ts' --glob '*.sql' --glob '*.mjs' --glob '*.md' --glob '*.js' --glob '*.json' --glob '*.yaml' --glob '*.yml' . || true
printf '\nFind likely source file names:\n'
git ls-files | rg '\.(ts|js|sql|mjs)$' | sed -n '1,200p'
printf '\nInspect source text for candidate validators:\n'
for f in $(git ls-files | rg '\.(ts|js|sql|mjs)$' || true); do
if rg -q -i 'official_host|officialHost|legal_sources|allowed_hosts|authority|vbpl\.vn|chinhphu\.vn' "$f"; then
echo "---- $f ----"
rg -n -C 5 -i 'official_host|officialHost|legal_sources|allowed_hosts|authority|vbpl\.vn|chinhphu\.vn' "$f" || true
fi
doneRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 598
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '240,310p' docs/TECHNICAL_SPEC.md
printf '\nImplementation/spec/search results:\n'
rg -n -C 4 -i 'official_host|officialHost|parseOfficialSource|authority|DEC-004|allowed_hosts|legal_sources|vbpl\.vn|chinhphu\.vn' --glob '*.ts' --glob '*.sql' --glob '*.mjs' --glob '*.md' --glob '*.js' --glob '*.json' --glob '*.yaml' --glob '*.yml' . || true
printf '\nCandidate source files:\n'
git ls-files | rg '\.(ts|js|sql|mjs)$' | sed -n '1,200p'
printf '\nDetailed matches in candidate files:\n'
for f in $(git ls-files | rg '\.(ts|js|sql|mjs)$' || true); do
if rg -q -i 'official_host|officialHost|legal_sources|allowed_hosts|authority|vbpl\.vn|chinhphu\.vn' "$f"; then
echo "---- $f ----"
rg -n -C 5 -i 'official_host|officialHost|legal_sources|allowed_hosts|authority|vbpl\.vn|chinhphu\.vn' "$f" || true
fi
doneRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 50399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'lib/official-source-url.ts:\n'
cat -n lib/official-source-url.ts | sed -n '1,150p'
printf '\nMIGRATION_RUNBOOK host authority section:\n'
sed -n '218,232p' docs/MIGRATION_RUNBOOK.md
printf '\nRelevant migration constraints:\n'
sed -n '286,302p' drizzle/0006_petite_lady_deathstrike.sql
printf '\nSource usages of official-source-url validators:\n'
rg -n -C 4 'isOfficialGovernmentHost|canonicalOfficialSourceUrl|parseOfficialSourceLinks' --glob '*.ts' --glob '*.js' . || true
printf '\nTests mentioning official host / authority / canonical:\n'
rg -n -C 4 'officialHost|official_host|exact authority|url_authority|canonical|vbpl\.vn|chinhphu\.vn|consecutive|leading|trailing' --glob '*.test.ts' --glob '*.spec.ts' --glob '*.test.tsx' --glob '*.spec.tsx' --glob '*.test.js' --glob '*.spec.js' . || trueRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 13000
Clarify official_host formatting for DEC-004.
The spec currently says “không chứa ký tự ngoài a-z0-9.- hoặc ..”, which can read as allowing consecutive dots. DEC-004 says non-draft sources may only use the canonical allowed domains. Make the constraint explicit: no empty labels, no .., no leading/trailing dots, and official_url must parse the authority exactly as official_host plus /, ?, or #.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/TECHNICAL_SPEC.md` around lines 267 - 291, Clarify the `legal_sources`
`official_host` constraint to require lowercase hosts with no empty labels,
consecutive dots, or leading/trailing dots. State that `official_url` must
contain an authority exactly matching `official_host`, followed only by `/`,
`?`, or `#`, while preserving DEC-004’s canonical allowed-domain rules for
non-draft sources.
Comment on lines
+502
to
+621
| persistWebCandidate: async (_requestId, result) => { | ||
| persisted += 1; | ||
| persistedResult = result; | ||
| return "33333333-3333-4333-8333-333333333333"; | ||
| }, | ||
| webSearch: async (_config, question) => { | ||
| webCalls += 1; | ||
| assert.equal(question, "Một quy định mới là gì?"); | ||
| return { | ||
| ok: true, | ||
| answer: "Kết quả tra cứu có căn cứ Chính phủ.", | ||
| warning: "Chưa kiểm duyệt.", | ||
| sources: [ | ||
| { | ||
| title: "Nguồn Chính phủ", | ||
| url: "https://vbpl.vn/document", | ||
| }, | ||
| ], | ||
| model: "gpt-5.4-mini", | ||
| usage: { | ||
| inputTokens: 1, | ||
| outputTokens: 1, | ||
| totalTokens: 2, | ||
| }, | ||
| }; | ||
| }, | ||
| }); | ||
| const response = await chat(chatRequest("Một quy định mới là gì?")); | ||
| assert.equal(response.status, 200); | ||
| assert.equal(response.headers.get("cache-control"), "no-store"); | ||
| assert.deepEqual(await response.json(), { | ||
| answer: "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | ||
| sections: [ | ||
| { | ||
| kind: "summary", | ||
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | ||
| bullets: [], | ||
| }, | ||
| ], | ||
| mode: "web_search", | ||
| warning: "Chưa kiểm duyệt.", | ||
| sources: [ | ||
| { | ||
| title: "Nguồn Chính phủ", | ||
| url: "https://vbpl.vn/document", | ||
| }, | ||
| ], | ||
| }); | ||
| assert.equal(webCalls, 1); | ||
| assert.equal(persisted, 1); | ||
| assert.equal( | ||
| persistedResult.answer, | ||
| "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | ||
| ); | ||
| assert.deepEqual(persistedResult.sections, [ | ||
| { | ||
| kind: "summary", | ||
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | ||
| bullets: [], | ||
| }, | ||
| ]); | ||
| for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) { | ||
| delete globalThis.__webSearchWorkerEnv[key]; | ||
| } | ||
| }); | ||
|
|
||
| test("chat fails closed when a successful web result cannot be persisted", async () => { | ||
| Object.assign(globalThis.__webSearchWorkerEnv, { | ||
| AI_WEB_SEARCH_ENABLED: "true", | ||
| OPENAI_API_KEY: "test-key", | ||
| OPENAI_MODEL: "gpt-5.4-mini", | ||
| }); | ||
| const chat = createChatHandler({ | ||
| limiter: () => ({ consumeChat: async () => allowed }), | ||
| telemetry: { emit() {} }, | ||
| managedAnswer: async () => null, | ||
| curatedAnswer: () => null, | ||
| reviewedWebAnswer: async () => null, | ||
| reserveWebBudget: async () => ({ | ||
| dayStart: 1, | ||
| reservedTokens: 12_000, | ||
| }), | ||
| settleWebBudget: async () => true, | ||
| persistWebCandidate: async () => null, | ||
| webSearch: async () => ({ | ||
| ok: true, | ||
| answer: "Có nguồn nhưng D1 đang lỗi.", | ||
| warning: "Chưa kiểm duyệt.", | ||
| sources: [{ title: "Nguồn", url: "https://vbpl.vn/document" }], | ||
| model: "gpt-5.4-mini", | ||
| usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 }, | ||
| }), | ||
| }); | ||
| const response = await chat(chatRequest("Quy định mới?")); | ||
| assert.equal(response.status, 200); | ||
| assert.equal((await response.json()).mode, "unavailable"); | ||
| for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) { | ||
| delete globalThis.__webSearchWorkerEnv[key]; | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Clean up the worker environment in a hook, not at the end of the test body.
Both tests set keys on globalThis.__webSearchWorkerEnv and delete them on the last lines of the test body. If any assertion above fails, the deletion never runs. The enabled web-search configuration then leaks into later tests in this file and changes their outcome. Move the cleanup into test.afterEach.
♻️ Proposed refactor
+function setWebSearchEnv(values) {
+ Object.assign(globalThis.__webSearchWorkerEnv, values);
+}
+
+test.afterEach(() => {
+ for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) {
+ delete globalThis.__webSearchWorkerEnv[key];
+ }
+});Then remove both trailing for (const key of Object.keys(...)) delete ... loops.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test("chat uses guarded web search only after retrieval no-match", async () => { | |
| let webCalls = 0; | |
| let persisted = 0; | |
| let persistedResult; | |
| Object.assign(globalThis.__webSearchWorkerEnv, { | |
| AI_WEB_SEARCH_ENABLED: "true", | |
| OPENAI_API_KEY: "test-key", | |
| OPENAI_MODEL: "gpt-5.4-mini", | |
| }); | |
| const chat = createChatHandler({ | |
| limiter: () => ({ consumeChat: async () => allowed }), | |
| telemetry: { emit() {} }, | |
| managedAnswer: async () => null, | |
| curatedAnswer: () => null, | |
| reviewedWebAnswer: async () => null, | |
| reserveWebBudget: async () => ({ | |
| dayStart: 1, | |
| reservedTokens: 12_000, | |
| }), | |
| settleWebBudget: async () => true, | |
| persistWebCandidate: async (_requestId, result) => { | |
| persisted += 1; | |
| persistedResult = result; | |
| return "33333333-3333-4333-8333-333333333333"; | |
| }, | |
| webSearch: async (_config, question) => { | |
| webCalls += 1; | |
| assert.equal(question, "Một quy định mới là gì?"); | |
| return { | |
| ok: true, | |
| answer: "Kết quả tra cứu có căn cứ Chính phủ.", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [ | |
| { | |
| title: "Nguồn Chính phủ", | |
| url: "https://vbpl.vn/document", | |
| }, | |
| ], | |
| model: "gpt-5.4-mini", | |
| usage: { | |
| inputTokens: 1, | |
| outputTokens: 1, | |
| totalTokens: 2, | |
| }, | |
| }; | |
| }, | |
| }); | |
| const response = await chat(chatRequest("Một quy định mới là gì?")); | |
| assert.equal(response.status, 200); | |
| assert.equal(response.headers.get("cache-control"), "no-store"); | |
| assert.deepEqual(await response.json(), { | |
| answer: "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | |
| sections: [ | |
| { | |
| kind: "summary", | |
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | |
| bullets: [], | |
| }, | |
| ], | |
| mode: "web_search", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [ | |
| { | |
| title: "Nguồn Chính phủ", | |
| url: "https://vbpl.vn/document", | |
| }, | |
| ], | |
| }); | |
| assert.equal(webCalls, 1); | |
| assert.equal(persisted, 1); | |
| assert.equal( | |
| persistedResult.answer, | |
| "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | |
| ); | |
| assert.deepEqual(persistedResult.sections, [ | |
| { | |
| kind: "summary", | |
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | |
| bullets: [], | |
| }, | |
| ]); | |
| for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) { | |
| delete globalThis.__webSearchWorkerEnv[key]; | |
| } | |
| }); | |
| test("chat fails closed when a successful web result cannot be persisted", async () => { | |
| Object.assign(globalThis.__webSearchWorkerEnv, { | |
| AI_WEB_SEARCH_ENABLED: "true", | |
| OPENAI_API_KEY: "test-key", | |
| OPENAI_MODEL: "gpt-5.4-mini", | |
| }); | |
| const chat = createChatHandler({ | |
| limiter: () => ({ consumeChat: async () => allowed }), | |
| telemetry: { emit() {} }, | |
| managedAnswer: async () => null, | |
| curatedAnswer: () => null, | |
| reviewedWebAnswer: async () => null, | |
| reserveWebBudget: async () => ({ | |
| dayStart: 1, | |
| reservedTokens: 12_000, | |
| }), | |
| settleWebBudget: async () => true, | |
| persistWebCandidate: async () => null, | |
| webSearch: async () => ({ | |
| ok: true, | |
| answer: "Có nguồn nhưng D1 đang lỗi.", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [{ title: "Nguồn", url: "https://vbpl.vn/document" }], | |
| model: "gpt-5.4-mini", | |
| usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 }, | |
| }), | |
| }); | |
| const response = await chat(chatRequest("Quy định mới?")); | |
| assert.equal(response.status, 200); | |
| assert.equal((await response.json()).mode, "unavailable"); | |
| for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) { | |
| delete globalThis.__webSearchWorkerEnv[key]; | |
| } | |
| }); | |
| function setWebSearchEnv(values) { | |
| Object.assign(globalThis.__webSearchWorkerEnv, values); | |
| } | |
| test.afterEach(() => { | |
| for (const key of Object.keys(globalThis.__webSearchWorkerEnv)) { | |
| delete globalThis.__webSearchWorkerEnv[key]; | |
| } | |
| }); | |
| test("chat uses guarded web search only after retrieval no-match", async () => { | |
| let webCalls = 0; | |
| let persisted = 0; | |
| let persistedResult; | |
| Object.assign(globalThis.__webSearchWorkerEnv, { | |
| AI_WEB_SEARCH_ENABLED: "true", | |
| OPENAI_API_KEY: "test-key", | |
| OPENAI_MODEL: "gpt-5.4-mini", | |
| }); | |
| const chat = createChatHandler({ | |
| limiter: () => ({ consumeChat: async () => allowed }), | |
| telemetry: { emit() {} }, | |
| managedAnswer: async () => null, | |
| curatedAnswer: () => null, | |
| reviewedWebAnswer: async () => null, | |
| reserveWebBudget: async () => ({ | |
| dayStart: 1, | |
| reservedTokens: 12_000, | |
| }), | |
| settleWebBudget: async () => true, | |
| persistWebCandidate: async (_requestId, result) => { | |
| persisted += 1; | |
| persistedResult = result; | |
| return "33333333-3333-4333-8333-333333333333"; | |
| }, | |
| webSearch: async (_config, question) => { | |
| webCalls += 1; | |
| assert.equal(question, "Một quy định mới là gì?"); | |
| return { | |
| ok: true, | |
| answer: "Kết quả tra cứu có căn cứ Chính phủ.", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [ | |
| { | |
| title: "Nguồn Chính phủ", | |
| url: "https://vbpl.vn/document", | |
| }, | |
| ], | |
| model: "gpt-5.4-mini", | |
| usage: { | |
| inputTokens: 1, | |
| outputTokens: 1, | |
| totalTokens: 2, | |
| }, | |
| }; | |
| }, | |
| }); | |
| const response = await chat(chatRequest("Một quy định mới là gì?")); | |
| assert.equal(response.status, 200); | |
| assert.equal(response.headers.get("cache-control"), "no-store"); | |
| assert.deepEqual(await response.json(), { | |
| answer: "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | |
| sections: [ | |
| { | |
| kind: "summary", | |
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | |
| bullets: [], | |
| }, | |
| ], | |
| mode: "web_search", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [ | |
| { | |
| title: "Nguồn Chính phủ", | |
| url: "https://vbpl.vn/document", | |
| }, | |
| ], | |
| }); | |
| assert.equal(webCalls, 1); | |
| assert.equal(persisted, 1); | |
| assert.equal( | |
| persistedResult.answer, | |
| "Trả lời ngắn\nKết quả tra cứu có căn cứ Chính phủ.", | |
| ); | |
| assert.deepEqual(persistedResult.sections, [ | |
| { | |
| kind: "summary", | |
| paragraphs: ["Kết quả tra cứu có căn cứ Chính phủ."], | |
| bullets: [], | |
| }, | |
| ]); | |
| }); | |
| test("chat fails closed when a successful web result cannot be persisted", async () => { | |
| Object.assign(globalThis.__webSearchWorkerEnv, { | |
| AI_WEB_SEARCH_ENABLED: "true", | |
| OPENAI_API_KEY: "test-key", | |
| OPENAI_MODEL: "gpt-5.4-mini", | |
| }); | |
| const chat = createChatHandler({ | |
| limiter: () => ({ consumeChat: async () => allowed }), | |
| telemetry: { emit() {} }, | |
| managedAnswer: async () => null, | |
| curatedAnswer: () => null, | |
| reviewedWebAnswer: async () => null, | |
| reserveWebBudget: async () => ({ | |
| dayStart: 1, | |
| reservedTokens: 12_000, | |
| }), | |
| settleWebBudget: async () => true, | |
| persistWebCandidate: async () => null, | |
| webSearch: async () => ({ | |
| ok: true, | |
| answer: "Có nguồn nhưng D1 đang lỗi.", | |
| warning: "Chưa kiểm duyệt.", | |
| sources: [{ title: "Nguồn", url: "https://vbpl.vn/document" }], | |
| model: "gpt-5.4-mini", | |
| usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 }, | |
| }), | |
| }); | |
| const response = await chat(chatRequest("Quy định mới?")); | |
| assert.equal(response.status, 200); | |
| assert.equal((await response.json()).mode, "unavailable"); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/openai-web-search.test.mjs` around lines 502 - 621, Move cleanup of
globalThis.__webSearchWorkerEnv into a test.afterEach hook so it runs even when
assertions fail. Add the hook near the test setup, then remove the trailing
key-deletion loops from both tests while preserving their existing environment
configuration and assertions.
Comment on lines
+172
to
+177
| for (const item of items) { | ||
| assert.match(html, new RegExp(`data-showcase-id="${item.id}"`)); | ||
| assert.match(html, new RegExp(item.title)); | ||
| assert.match(html, new RegExp(item.summary)); | ||
| assert.match(html, new RegExp(item.topic)); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Build the assertions from string containment instead of unescaped regular expressions.
new RegExp(item.title) and new RegExp(item.summary) treat fixture text as a pattern. The current fixture has no regular-expression metacharacters, so the test passes today. A future fixture value with (, ?, or . would change the meaning or throw.
♻️ Proposed change
for (const item of items) {
assert.match(html, new RegExp(`data-showcase-id="${item.id}"`));
- assert.match(html, new RegExp(item.title));
- assert.match(html, new RegExp(item.summary));
- assert.match(html, new RegExp(item.topic));
+ assert.ok(html.includes(item.title), item.title);
+ assert.ok(html.includes(item.summary), item.summary);
+ assert.ok(html.includes(item.topic), item.topic);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const item of items) { | |
| assert.match(html, new RegExp(`data-showcase-id="${item.id}"`)); | |
| assert.match(html, new RegExp(item.title)); | |
| assert.match(html, new RegExp(item.summary)); | |
| assert.match(html, new RegExp(item.topic)); | |
| } | |
| for (const item of items) { | |
| assert.match(html, new RegExp(`data-showcase-id="${item.id}"`)); | |
| assert.ok(html.includes(item.title), item.title); | |
| assert.ok(html.includes(item.summary), item.summary); | |
| assert.ok(html.includes(item.topic), item.topic); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/public-showcase.test.mjs` around lines 172 - 177, Update the assertions
in the loop over items to verify title, summary, and topic using
string-containment checks rather than constructing RegExp instances from fixture
values; keep the existing showcase ID assertion unchanged.
Comment on lines
+518
to
+528
| for (const event of state.events) { | ||
| assert.deepEqual(Object.keys(event).sort(), [ | ||
| "outcome", | ||
| "policyVersion", | ||
| "requestId", | ||
| "retryAfter", | ||
| "scope", | ||
| ]); | ||
| assert.equal(event.policyVersion, rateLimitPolicyVersion); | ||
| assert.doesNotMatch(JSON.stringify(event), /203\\.0\\.113|[0-9a-f]{64}/i); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Double-escaped dots disable three leak assertions. These assert.doesNotMatch patterns are regex literals, not strings. \\. matches a literal backslash followed by any character, so the affected alternatives can never match the sensitive values. The tests still pass because other alternatives in the same pattern remain valid, which hides the gap.
tests/rate-limit.test.mjs#L518-L528: change203\\.0\\.113to203\.0\.113on line 527.tests/telemetry.test.mjs#L166-L166: change203\\.0\\.113to203\.0\.113.tests/telemetry.test.mjs#L529-L532: changeevil\\.exampletoevil\.exampleon line 531.
Line 298 of tests/telemetry.test.mjs already uses the correct single-escaped form. Use it as the reference.
📍 Affects 2 files
tests/rate-limit.test.mjs#L518-L528(this comment)tests/telemetry.test.mjs#L166-L166tests/telemetry.test.mjs#L529-L532
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/rate-limit.test.mjs` around lines 518 - 528, Correct the over-escaped
regex alternatives in the leak assertions: in tests/rate-limit.test.mjs lines
518-528, replace the doubled escaping for 203.0.113 with single regex escaping;
make the same 203.0.113 correction in tests/telemetry.test.mjs lines 166-166,
and correct evil.example in tests/telemetry.test.mjs lines 529-532. Match the
already-correct pattern at line 298 of tests/telemetry.test.mjs.
Comment on lines
+43
to
+58
| if (/INSERT INTO rate_limit_buckets/i.test(query)) { | ||
| const [scope, keyHash, windowStart, expiresAt, cap] = values; | ||
| const key = `${scope}:${keyHash}:${windowStart}`; | ||
| const current = buckets.get(key); | ||
| const value = { | ||
| request_count: Math.min((current?.request_count ?? 0) + 1, cap), | ||
| expires_at: expiresAt, | ||
| }; | ||
| buckets.set(key, value); | ||
| return [{ request_count: value.request_count }]; | ||
| } | ||
| if (/SELECT request_count[\s\S]+FROM rate_limit_buckets/i.test(query)) { | ||
| const [scope, keyHash, windowStart] = values; | ||
| const value = buckets.get(`${scope}:${keyHash}:${windowStart}`); | ||
| return value ? [{ request_count: value.request_count }] : []; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 10 'INSERT INTO rate_limit_buckets' lib/rate-limit.ts drizzle/0004_rate_limit_v1.sqlRepository: tomnyson/tuyen_truyen_an_toan_giao_thong
Length of output: 1236
Mirror the real bucket upsert bound in the mock.
lib/rate-limit.ts clamps request_count at ?5 in min(request_count + 1, ?5), so the returned count can equal the configured cap. This mock uses min(count + 1, cap), which also returns the cap, so repeated mocked validAdminCookie requests cannot exceed the login cap and the rate-limit behavior under test diverges from production. Update the mock to return Math.min((current?.request_count ?? 0) + 1, cap) so request_count can reach the cap and surpass the strict count > cap check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/rendered-html.test.mjs` around lines 43 - 58, Update the
rate_limit_buckets mock in the INSERT INTO rate_limit_buckets branch to clamp
request_count with the configured cap using Math.min((current?.request_count ??
0) + 1, cap), allowing the count to reach but not exceed cap and preserving the
production rate-limit behavior.
Comment on lines
+139
to
+145
| assert.match(migration, /vbpl\.vn/); | ||
| assert.match(migration, /vbpl\.moj\.gov\.vn/); | ||
| assert.match(migration, /\?\\?\*?\.chinhphu\.vn|chinhphu\.vn/); | ||
| assert.match(migration, /`verified_by` != `created_by`/i); | ||
| assert.match(migration, /`reviewed_by` != `created_by`/i); | ||
| assert.match(migration, /display_order` >= 0/i); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The alternation makes the chinhphu.vn subdomain check meaningless.
In /\?\\?\*?\.chinhphu\.vn|chinhphu\.vn/, the second branch matches any occurrence of chinhphu.vn, so the first branch never adds coverage. The assertion no longer proves that the migration allows the intended subdomain form. Assert the exact allowlist expression used in the migration instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/schema-foundation.test.mjs` around lines 139 - 145, Update the
chinhphu.vn assertion in the migration test to match the exact allowlist
expression used by the migration, removing the broad alternation that
independently matches any chinhphu.vn occurrence. Preserve coverage for the
intended wildcard/subdomain form.
- Added DEC-012 to allow `thuvienphapluat.vn` as a fallback source after official no-result, marked as unverified and requiring validation. - Updated documentation to reflect changes in source registry and technical specifications regarding reference searches. - Enhanced the search logic to include a second search for references when the official search yields no results. - Introduced new types and functions for handling reference sources in the official-source-url module. - Modified the OpenAI web search logic to differentiate between official and reference sources, including appropriate warnings and handling for legal claims. - Added tests to ensure the correct behavior of the new reference search functionality and its integration with the chat interface.
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
2 participants
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 by CodeRabbit
New Features
Documentation
Tests