fix(core): strip the deprecated Cf bidi-adjacent controls

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.

[Sma1lboy]

Summary

U+206A–206F pass both sanitizer classes in packages/core/src/text-sanitize.ts: BIDI_FORMAT_REGEX stops at U+2069 and ZERO_WIDTH_REGEX stops at U+2064, leaving the six code points between them uncovered. All six are category Cf — invisible, deprecated, bidi-related formatting characters — and none of them are whitespace, so the \s+ collapse did not remove them either.

The bidi class is extended to U+206F, replacing with a space to stay consistent with the other bidi controls.

FOREIGN_UNSAFE_CHARS in foreign-session.ts is extended in the same commit rather than a follow-up. Its own doc comment describes it as "one source of truth" with the sanitizer, so fixing only the sanitizer would recreate exactly the drift that comment exists to prevent — and the id guard has the same gap.

Fixes#3823

Verification

npm run lint, npm run format:check, npm --workspace @maka/core run typecheck, and the full @maka/core suite (657/657) pass.

Both new assertions were confirmed RED before the fix by reverting the two source files and rebuilding:

✖ strips the deprecated Cf bidi-adjacent controls U+206A-206F (#3823)
✖ accepts safe tokens and rejects control/bidi/whitespace/overlong ids
ℹ pass 26 fail 2

Behaviour on current main, per code point:

cp name survives_sanitize id_guard_accepts
U+206A INHIBIT SYMMETRIC SWAPPING true true
U+206B ACTIVATE SYMMETRIC SWAPPING true true
U+206C INHIBIT ARABIC FORM SHAPING true true
U+206D ACTIVATE ARABIC FORM SHAPING true true
U+206E NATIONAL DIGIT SHAPES true true
U+206F NOMINAL DIGIT SHAPES true true
surviving_sanitize=6/6 accepted_by_id_guard=6/6

After the fix, both columns are 0/6. The regression test also pins controls that must not move: U+2069, U+2060 and U+202E stay stripped, and /\s/ is confirmed false for the new range — which is why the whitespace collapse never caught them.

Review focus

What this does and does not demonstrate. The reproduction shows the characters pass both pipelines, and that two session names differing only by one of them render identically while comparing unequal. It does not construct an attack — that would need an assumption about who controls session names, which I did not want to invent. Flagging so the change is judged on what is actually shown.

Deliberately out of scope. The issue raises variation selectors (U+FE00–FE0F) and tag characters (U+E0000–E007F) as a possible follow-up. Both are real invisible-content vectors, but they are a separate class decision with different trade-offs (variation selectors carry meaning in legitimate emoji and CJK sequences), so they are not folded in here.

Overlap with #3692. That PR adds packages/core/src/__tests__/text-sanitize.test.ts covering the existing bidi set, not this range. I put the regression in foreign-session.test.ts next to the existing sanitizeForeignText tests because it needs to assert the id-guard side too. If #3692 lands first and a maintainer prefers the coverage consolidated in the new file, I am happy to rebase and move the sanitizer half there.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 (Claude Code) — investigation, the fix, and the regression tests. Reviewed and verified locally by me; the commit carries a Generated-by trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

I reviewed this head and found no blocking code issues.

Correctly handles U+206A–206F via sanitizer; verification passed. Hosted test currently has no checks on this head — needs CI green before merge.

[P3] Test coverage gap: normalizeUserSessionName regression not covered (only sanitizeForeignText loop), and isSafeForeignId only samples endpoints — suggest table-driven full range.

简体中文

该头无阻断，仅测试覆盖建议。

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Sma1lboy]

Both coverage gaps addressed in 5f3ab2625, and the branch is rebased onto current main.

• isSafeForeignId now covers all six code points (U+206A–206F) rather than sampling the endpoints.
• Added packages/core/src/__tests__/session-name.test.ts — no session-name test file existed. It exercises the full range through normalizeUserSessionName, which is the user-visible surface the issue names, and pins that an ordinary name is left untouched.

I put the session-name coverage in its own file rather than extending foreign-session.test.ts, since that file imports only from foreign-session.js and the assertion belongs to a different module.

Verification on the rebased head: @maka/core 670 tests, 670 pass. Reverting only text-sanitize.ts and foreign-session.ts while keeping the tests fails 3 of them, including the new session-name one, so the coverage is load-bearing rather than decorative. Lint, format and typecheck pass.

Agreed that the hosted test check still needs to run on this head before merge.

[Astro-Han]

Both P3s from the last head are closed. session-name.test.ts is new and loops the full 0x206a..0x206f range rather than sampling, and isSafeForeignId's rejection list went from seven sampled endpoints to all six of the new code points.

I checked the range boundary rather than trusting it: ⁦- matches U+2069 through U+206F and stops before U+2070 SUPERSCRIPT ZERO, which is a visible No character — so widening the bidi class strips only the deprecated Cf controls and cannot eat printable text. Extending FOREIGN_UNSAFE_CHARS in the same commit is right for the reason you gave: its own comment claims to be one source of truth with the sanitizer, so fixing one and not the other would create exactly the drift that comment exists to prevent.

(Unrelated to this PR: foreign-session.test.ts renders as a binary diff because it contains a literal NUL byte, and that's already true on main. I'll track it separately.)

AI-assisted review: I verified the regex boundaries at U+2069/206A/206F/2070 by running them, and read both test files at head 5f3ab2625. AI review is not independent human review.