Skip to content

fix(app-facts): open the catalog through an escaped read-only URI - #377

Closed
MendixMau wants to merge 2 commits into
masterfrom
fix/app-facts-db-uri
Closed

MendixMau wants to merge 2 commits into
masterfrom
fix/app-facts-db-uri

Conversation

@MendixMau

Copy link
Copy Markdown
Owner

Direct lane

What changed and why (one paragraph):
Step 2 of project-bin/app-facts.sh opened the catalog with "file:%s?mode=ro" % DB, inserting the path unescaped. In a URI, # starts a fragment and ? starts the query, so a model folder named a #2 or b?c cut the path short and dropped mode=ro with it. SQLite then created an empty database at the truncated path and opened that: a zero-byte a or b beside the model folder, outside the project. The shell's catalog check had already passed, because it reads the plain path with the sqlite3 CLI.

What master actually does differs from the backlog's description. It does not write empty facts. It stops with a Python traceback (no such table: modules_data, exit 2) and leaves the stray file behind.

The fix:

  • New mxtk_sqlite_ro_uri <path> in _common.sh builds the URI. It is the pathname2url escaping that mxtk_model_version already used, and mxtk_model_version now calls the shared helper too, so the escaping exists once instead of being copied.
  • Step 2 refuses with a FAULT line if the database it opened has 0 tables, so it never writes facts from an empty database.
  • app-facts.sh refuses if _common.sh is older than the helper, rather than falling back to an unescaped path.

Field evidence: none yet. The fixture catalog is synthetic, as in test-catalog-edge-guard.sh: its tables and columns are exactly the ones app-facts queries, and a stub mxcli puts it in place.

Needs a Windows run: the path reaches Python through an environment variable, and whether Git Bash (MSYS2) converts a /c/... value there for a native Python is unconfirmed. I deliberately left this alone: no native_path was added for DB. Someone should run the new fixture under Git Bash on Windows.

  • No client data anywhere in the diff
  • Size cap: 4 files, about 40 non-fixture lines
  • Test tier: fixtures only.
    • New tests/wave2/test-app-facts-db-uri.sh project-bin/app-facts.sh: 8/8. It covers folders a #2, b?c and a plain control (each must produce real facts and leave no stray file), the URI shape (file:/x/a%20%232/b%3Fc/catalog.db?mode=ro), and the 0-table refusal.
    • Against master's app-facts.sh and _common.sh the new fixture fails 6 of 8; only the two plain-folder control cases pass.
    • test-app-report.sh bin/app-report.sh: 109/109
    • test-catalog-edge-guard.sh project-bin/graph-sweep.sh: 20/20
    • test-mxbuild-version-match.sh project-bin/_common.sh: 18/18
    • That fixture always has the sqlite3 CLI, so I checked mxtk_model_version's Python fallback by hand with sqlite3 off PATH and a model under a #2/: it returns 11.12.2, the same as master.
    • render-routing --check: baseline 75,028 words before and after
  • Both layouts: not affected. DB is derived from the .mpr's own folder.
  • Both platforms: Windows still needs a run (see above).
  • CHANGELOG.md line appended, credited

Source: r-eng-backlog item 3.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw


Generated by Claude Code

"file:%s?mode=ro" % DB cut the path at '#' or '?' in a folder name, dropped
mode=ro, and SQLite created and opened an empty file at the truncated path.
New mxtk_sqlite_ro_uri in _common.sh (the pathname2url escaping
mxtk_model_version already used, which now calls it); step 2 refuses a
0-table catalog. Fixture test-app-facts-db-uri.sh: 8/8, 6 fail on master.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw
…lacks the helper

Review finding on #377: the hard refusal when _common.sh had no mxtk_sqlite_ro_uri broke
app-facts' own design (it already falls back for require_py and find_mpr), and its message
named project-bin/_common.sh (bin/_common.sh in a consuming project) and a sync command with
no project root. Step 2 now builds the same pathname2url file:...?mode=ro URI inline when the
helper is missing, and passes the catalog path through native_path when that exists, as
mxtk_model_version does.

Fixture test-app-facts-db-uri.sh gains a case: app-facts beside a _common.sh with the helper
stripped, model folder "a #2" -> facts written, nothing created beside the folder (fails on
1649f31, which refused). The 0-table case now takes its interpreter from require_py and a
$PYTHON wrapper instead of hard-coded python3 names (4 check-portability violations).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw
MendixMau pushed a commit that referenced this pull request Oct 14, 2026
…only URI

Review: independent review MERGE-AFTER-FIX — URI escaping verified (#, ?, %41, space, &, ;=, [], unicode; read-only, no stray files; Windows nturl2path form valid); finding: hard refusal on old _common.sh broke app-facts' own fallback design and named wrong paths → inline pathname2url fallback + native_path, 2 fixture cases, in b97c9f6; confirmation MERGE: db-uri 10/0 (8 fail on master), app-report 109/0, catalog-edge-guard 20/0, mxbuild-version-match 18/0, check-portability clean, check-scripts 110/110. No field run yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw
MendixMau added a commit that referenced this pull request Oct 15, 2026
…journey controls, fixes) (#385)

Lands #340 #363 #362 #355 #370 #368 #364 #365 #366 #372 #367 #371 #376 #369 #373 #377 #378 #380.

Review: each constituent PR independently reviewed by a fresh agent that did not write it; merged into integration/2026-10-15 only on a MERGE verdict with green CI (per-PR Review lines in the integration merge commits). After-fix rounds: #373 (env-var cap, rename/skip-loops freshness), #377 (inline URI fallback), #378 (full-pack double count, slice content). Stub diet (#369/#378/#380) behaviour A/B, 5 reps x 2 drafting tasks + 5 retrieval probes x 3 reps: trimmed context no worse than master on every scored metric (planted STOP refused 20/20, parse failures page 2/5 vs 3/5, probes 15/15); mdl Stage 5 56,236 -> 30,647 words, baseline 75,028 -> 73,233. Full suites on c9bc073: run-tests 33/0, wave2 111 fixtures 0 failing. Field evidence: none yet for the instruments (#355 warn-only, #373/#377 read-only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw
@MendixMau

Copy link
Copy Markdown
Owner Author

Landed on master in #385 (squash 868b3cb), after its own independent review on the night integration branch. Closing this PR.


Generated by Claude Code

@MendixMau MendixMau closed this Oct 15, 2026
@MendixMau
MendixMau deleted the fix/app-facts-db-uri branch October 15, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants