Кэш Вычислить и Выполнить не зависит от числа глобальных контекстов

[sfaqer]

Вычислить/Выполнить кэшируют скомпилированное выражение, а оно связывало области по номеру в списке кадра: сначала глобальные контексты, за ними область модуля и вложенных вычислений. ПодключитьВнешнююКомпоненту с [GlobalContext] добавляет в этот список контекст, номера сдвигаются, и выражение из кэша вместо переменной или метода модуля берет свойство/метод компоненты или падает с ArgumentOutOfRangeException. Воспроизводится и в одном потоке:

Для Сч = 1 По 2 Цикл
	Сообщить(Вычислить("МояПеременная")); // на втором круге - свойство компоненты
	Если Сч = 1 Тогда
		ПодключитьВнешнююКомпоненту(ПутьКDll); // с глобальным контекстом
	КонецЕсли;
КонецЦикла;

Глобальные контексты теперь связываются с выражением напрямую, а область модуля и вложенных вычислений — по номеру с конца списка, который от числа глобальных контекстов не зависит.

Тест EvalScopesTests в Core.Tests, а tests/global-context-addin.os прогоняет сценарии из #344 и #911 и подключение во время работы с тестовой компонентой, которая теперь добавляет глобальный контекст.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Corrected how scripts resolve module variables and functions after a global context is attached, including when expressions are evaluated repeatedly.
  • Improved global-context access across different component load orders, dynamically loaded scripts, and objects created before or after attachment.
  • Corrected the order used to identify nested execution-frame scopes, so scope references resolve to the intended scope.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fb10b4ae-9deb-4f61-af9e-39fe304c460b

---

📥 Commits

Reviewing files that changed from the base of the PR and between c71b377 and 9224118.

---

📒 Files selected for processing (1)

• src/ScriptEngine/Machine/JoinedScopes.cs

---

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

---

---

📝 Walkthrough

Walkthrough

Scope binding now separates root scopes from nested inner scopes. Compiler context extraction and frame-scope resolution use reverse indexing for inner scopes. Tests cover evaluated expressions and global-context loading and attachment scenarios.

Changes

Scope Binding

Layer / File(s)	Summary
Scope mapping and resolution src/OneScript.Core/Compilation/Binding/ScopeBindingDescriptor.cs, src/ScriptEngine/Machine/JoinedScopes.cs, src/ScriptEngine/Machine/MachineInstance.cs, src/ScriptEngine/Machine/ModuleSymbolBinding.cs, src/ScriptEngine/Compiler/ModuleDumpWriter.cs	JoinedScopes tracks the root scope list and inner-scope count, and provides reverse lookup. Compiler context extraction adds root scopes with static bindings and inner scopes with reverse-indexed frame bindings. Frame-scope resolution uses reverse lookup, and module dumps show a caret-prefixed, one-based scope depth.
Evaluation scope validation src/Tests/OneScript.Core.Tests/EvalScopesTests.cs	Adds a test that attaches a global context during a loop and checks that evaluated expressions return the expected module-scope values on both iterations.
Global-context loading and attachment scenarios src/Component/SimpleGlobalContext.cs, tests/global-context-addin.os, tests/global-context/*	Adds a test component and scenarios for library load order, script loading, runtime attachment, and objects created before component attachment. The add-in runs the scenarios in separate processes and checks their output.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

---

Merge Risk | ⚪ Minimal · up to 92241

Merge Risk: ⚪ Minimal · up to 92241

No concrete regression is established in the reviewed change, so no change-specific user impact currently warrants delaying the merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 92241

The change stabilizes cached expression bindings without demonstrating broader access to objects or methods. Risk is bounded, but compatibility with external hosts and behavior during concurrent attachment or interrupted execution remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is context selection within an execution machine: script expressions resolve properties and methods on existing global or frame contexts. The inspected change does not demonstrate an expansion to additional services, tenants, credentials, or data stores; downstream host exposure remains unverified.

Trust Boundaries and Controls

• inferred — For inspected callers, script-provided expressions continue through symbol extraction and binding resolution to existing context interfaces. Direct root bindings preserve the selected context identity, and bounded reverse lookup prevents root growth from redirecting an inner binding to a component context. No new authorization bypass was demonstrated by this flow.

Resilience and Maintainability Implications

• observed — Global contexts remain backed by an unsynchronized mutable list. Evaluate has finally-based frame cleanup, whereas Execute and debugger evaluation retain pre-existing cleanup paths reached after successful execution. These conditions were not shown to worsen in this PR, but concurrent attachment and interruption recovery remain unestablished for the changed binding lifecycle.

---

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: cached Вычислить and Выполнить expressions no longer depend on the number of global contexts.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[EvilBeaver]

Внешние компоненты, которые добавляют глобальный скоуп ривносят столько адовых ошибок, что мы просто отказались их поддерживать. На эту тему тут уже заведено три или больше issues. Если по простому, то добавление контекстов в рантайме дает очень и очень неочевидные баги.

Иными словами, я боюсь это трогать и вливать. Надо найти все старые issues с этими же проблемами и смотреть в т.ч. и те тестовые ситуации

[sfaqer]

Нашел старые: #344 и #911 (к ним привязан #804). Там компонента с глобальным контекстом подключалась в ПриЗагрузкеБиблиотеки, и если библиотека была не последней в #Использовать, движок падал с ArgumentOutOfRangeException в SetFrame или с NPE на вызове метода модуля. Это исправлено в #956.

Прогнал их сценарии на develop и на этой ветке с тестовой компонентой с [GlobalContext]: библиотека с компонентой первой и последней рядом с обычной библиотекой, вызов метода модуля, загрузка такого сценария через ЗагрузитьСценарий, как в testrunner. Обе сборки работают одинаково. Разница только при подключении во время работы: на develop Вычислить после ПодключитьВнешнююКомпоненту падает с ArgumentOutOfRangeException, на ветке переменные и методы модуля на месте, а Вычислить видит компоненту.

Если нужно, добавлю эти сценарии в тесты: класс с [GlobalContext] в тестовую компоненту и сценарии в tests/.

[sfaqer]

Добавил эти сценарии в тесты: в тестовой компоненте теперь есть класс с [GlobalContext], а tests/global-context-addin.os прогоняет сценарии из #344 и #911 и подключение во время работы, каждый отдельным процессом. На develop без исправления падает только подключение во время работы, остальные проходят и там.

[EvilBeaver]

Есть еще issue #1000 тоже связанный с порядком добавления глобальных скоупов.

[sfaqer]

Есть еще issue #1000 тоже связанный с порядком добавления глобальных скоупов.

С этим PR он не связан. #1000 про порядок выполнения тел модулей библиотеки: InitExternalLibrary выполнял их по порядку файлов, и тело модуля, обратившееся к модулю, до которого очередь еще не дошла, видело его переменные пустыми. Глобальных контекстов модули не добавляют, на develop, 2.2.0 и этой ветке поведение было одинаковым. Починил в #1788.

[sonar-openbsl-ru-qa-bot]

Quality Gate passed

Issues

• 0 New Issues
• 2 Fixed Issues
• 0 Accepted Issues

Measures

• 0 Security Hotspots
• No data about coverage (35.10% Estimated after merge)
• 0.00% Duplicated Code (2.50% Estimated after merge)

Project ID: OneScript

View in SonarQube

[EvilBeaver]

Мне кажется норм. Я бы попросил еще кого-то взглянуть, страшновато трогать это место. @Mr-Rm ?