.pr_agent_accepted_suggestions

PR 2 (2026-06-29)

[observability] Code scanning output removed

---

Code scanning output removed

The workflow still claims Code Scanning integration and grants `security-events: write`, but it now runs plain `pyre check` without generating/uploading SARIF, so no Pyre findings will appear in GitHub Code Scanning.

Issue description

The workflow text/permissions indicate Code Scanning integration, but the job no longer produces or uploads SARIF after switching to direct pyre check, so Code Scanning will not receive results.

Issue Context

This is a behavior regression from “Code Scanning workflow” semantics to a plain CI command.

Fix Focus Areas

• .github/workflows/pyre.yml[6-8]
• .github/workflows/pyre.yml[30-34]
• .github/workflows/pyre.yml[53-54]

---

---

PR 1 (2026-06-29)

[reliability] Execute exceptions escape registry

---

Execute exceptions escape registry

ComponentRegistry.run() calls Component.execute() without try/except, so exceptions from Component subclasses escape and abort execution. In AutomationEngine, these get recorded as a generic "__engine__" failure, obscuring which component actually crashed.

Issue description

ComponentRegistry.run() does not guard component.execute(); if a custom Component.execute() raises, the exception propagates.

• When invoked through AutomationEngine, the exception is caught at the engine layer and recorded as a ComponentOutput(component="__engine__", ...), which loses the identity of the failing component.

Issue Context

• FunctionComponent.execute() already catches exceptions and returns an error output, but custom Component subclasses are not protected.
• run_pipeline() relies on run() and expects to stop on failure; an exception is a harder failure mode than a structured ComponentOutput(success=False).

Fix Focus Areas

• src/automation/components.py[228-254]
• src/automation/engine.py[293-325]

Suggested fix

Wrap component.execute(input_) in ComponentRegistry.run() with try/except and return a ComponentOutput for that component name on exception (ideally including traceback text or at least exception type/message). This preserves per-step attribution and avoids escalating normal component failures into engine-level generic errors.

---

[reliability] Overwrite skips teardown

---

Overwrite skips teardown

ComponentRegistry.register() overwrites an existing component for the same name without calling teardown() on the old instance. Re-registering components can leak resources or leave stale side effects because teardown() is only called by deregister().

Issue description

ComponentRegistry.register() replaces self._components[name] unconditionally and does not teardown any previously-registered component under the same name.

Issue Context

• Components may allocate resources in setup() and release them in teardown().
• teardown() is called only in deregister(), not during replacement.

Fix Focus Areas

• src/automation/components.py[171-180]
• src/automation/components.py[211-222]

Suggested fix

On register(name, component):

• If name already exists, either:
  • call old.teardown() before overwriting, or
  • raise a ValueError (forcing explicit deregister() first).
• Consider ordering: teardown old first, then setup new, then store new, so partial failures don’t leave registry in an inconsistent lifecycle state.

---

[correctness] Missing variables ignored

---

Missing variables ignored

AutomationEngine._snapshot_variables() silently skips variable names that are referenced by an automation but not registered. This hides configuration errors and can lead to components running with incomplete metadata.

Issue description

AutomationEngine._snapshot_variables() uses VariableRegistry.get() and omits missing variables with no error/warning, even if the automation explicitly references them.

Issue Context

• VariableRegistry already provides require() which raises on missing variables.
• Engine passes the snapshot into run metadata for each component step.

Fix Focus Areas

• src/automation/engine.py[339-351]
• src/automation/variables.py[212-220]

Suggested fix

Decide on desired behavior and enforce it consistently:

• Fail fast: use self.variables.require(name) and convert missing-variable errors into a failed RunResult with a clear message, OR
• Warn loudly: log a warning and include the key with a sentinel (e.g. None plus an error field) so downstream components can detect the misconfiguration.

---

[correctness] Peek drops collected value

---

Peek drops collected value

GeneratorVariable.peek() advances the underlying generator and caches the value, but collect() drains only the generator and ignores any cached peeked value. If peek() is called before collect(), the peeked element is silently omitted from the collected list.

Issue description

GeneratorVariable.peek() advances the generator and stores the next value in _peeked_value, but collect() currently returns list(self._ensure_gen()) and does not include _peeked_value when _peeked is set. This causes a silent off-by-one/data-loss bug when callers mix peek() and collect().

Issue Context

• peek() implements lookahead by calling next() on the generator and caching the result.
• next()/__next__() return the cached value when _peeked is set, but collect() bypasses that logic.

Fix Focus Areas

• src/automation/variables.py[68-88]

Suggested fix

Update collect() to account for the peek cache, e.g.:

• If _peeked is True, prepend _peeked_value to the collected list and clear the peek state before draining the generator.
• Consider clearing _peeked_value after consumption (in next()/__next__()) to avoid retaining large objects unnecessarily.

---

---