Repository navigation
test: close short-tag and unknown-template discovery gaps - #26
Conversation
bnjmnrsh
left a comment
bnjmnrsh
left a comment
There was a problem hiding this comment.
Independent review of published head 3df9a0c against base d749596; verified tree a268de09c207eeb4e3c076b9991cdd4d80968ceb in a separate clone. No blocking findings.
Read AGENTS.md, quality acceptance boundaries and the complete two-file patch. Unknown suffixes and linked template bodies receive complete inspection; bare opening tags are detected independently of process INI; only genuine leading XML declarations are removed before scanning the remainder. Existing Markdown, valid JSON and declared Bash boundaries remain deliberate; no runtime, resource, dependency or release change.
Independent composer check passed on PHP 8.3.6 using the locked tooling: PHPStan level 5 clean, both standards profiles clean, PHPUnit 52 tests / 709 assertions. Additional isolated probes proved actual long-preamble bare-tag execution with short_open_tag=1 and inert text with =0, with guard rejection in both settings; unknown-suffix long PHP; genuine XML acceptance; XML-lookalike and XML-followed-by-PHP rejection; valid JSON and declared Bash acceptance with malformed JSON/undeclared Bash rejection. Worktree restored clean.
Native CI and merge authorization remain the coordinator's separate gates; this review grants no publication or consumer adoption.
|
Exact candidate qualification: base d749596, head 3df9a0c, tree a268de09c207eeb4e3c076b9991cdd4d80968ceb. Author full canonical checks pass; independent review 5445881527 has no blockers with additional actual INI0/1 and XML/inert boundaries. Applicable native floor/current-PHP jobs and terminal quality pass. No runtime/dependency/exported-standard or release changes. Ready for Ben's specific merge decision; #136 remains open until all candidates delivered and wider release/certification dispositions addressed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3df9a0caab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Memory finding repaired at cf2d62a, tree af373a3a2637a34a65c024c993c9e91f924e23c9 (identical to local 78c3173). Discovery now streams 8 KiB chunks with marker overlap for regular files and external regular-file link targets. No automatic binary exemption. Sparse 160 MiB candidates pass under 128 MiB memory, while split and late PHP markers still require review. Bounded JSON/XML/shebang handling and whitespace-boundary controls included. Full locked composer check passes: 16 directly analysed maintained paths, PHPCS, 54 tests/756 assertions; focused controls 4/149. Fresh native CI and independent re-review are running; previous-head qualification does not substitute for those results. |
bnjmnrsh
left a comment
bnjmnrsh
left a comment
There was a problem hiding this comment.
Independent refreshed review on published head cf2d62a, base d749596, tree af373a3a2637a34a65c024c993c9e91f924e23c9. No blocking findings.
Reviewed the complete streaming revision and new controls in a separate checkout. Independent composer check passes: direct PHPStan level5 for all16 entrypoints, both standards profiles, PHPUnit54 tests/756 assertions. Independently reproduced 160MiB inert regular files and external regular-file symlinks under a128MiB PHP limit; both pass without the previous allocation failure. Bare markers crossing offsets8191 and16383, and at160MiB-1, are rejected for both forms. Previous actual short_open_tag0/1 execution/discovery, long unknown suffix, genuine/fake/following-PHP XML, valid/invalid JSON, and declared/undeclared Bash controls all still pass.
The stream keeps complete marker discovery with bounded overlap. Small valid JSON retains bounded data treatment; oversized JSON and declarations receive fail-closed review rather than implicit exclusion. Binary-format blanket exclusion was deliberately declined under the accepted narrow-exemption policy, with written disposition; the independently reproduced memory issue is fixed. Both late review threads are now explicitly dispositioned and resolved on this materially revised candidate.
Review checkout remains clean. Runtime, resources, dependencies and consumer/publication behavior remain unchanged; merge/publication authorization remains separate.
|
Final revised candidate qualification: cf2d62a on base d749596, tree af373a3a2637a34a65c024c993c9e91f924e23c9. Author and independent full composer check pass54 tests/756 assertions, direct PHPStan5 coverage16 and locked PHPCS. Independent review5446250414 found no blockers and reproduced bounded-memory, split/late-marker and prior INI/XML/JSON/Bash controls. Native37662249908 passes PHP8.0, PHP8.5, quality and GitGuardian. Both late threads are resolved with concrete fix/policy dispositions. Ready for Ben's merge decision; no merge or release performed. |
1 participant
Follow-up to RocketsAreNostalgic/.github#136.
Existing discovery missed bare PHP tags and HTML-prefixed PHP in unknown suffixes. Inspect unknown formats completely and detect opening tags regardless of short_open_tag, preserving genuine leading XML and narrowly evidenced inert examples. Actual INI0/1 runtime and guard controls cover bare/long tags, XML adjacency and undeclared scripts.
Base d749596; tree a268de09c207eeb4e3c076b9991cdd4d80968ceb, equal to local e2c203f495c1d02167d0644ef2c87ea70303a10f. Final canonical, independent review and native CI qualification in progress. No runtime, exported resources or dependency changes. Candidate awaits Ben's merge decision.