CREST Adapter

[calvinp0]

Addition of CREST Adapter that complements the heuristic adapter.

In general, an unused global variable should either be removed, renamed to indicate it is intentionally unused, or explicitly marked as public API if it is meant to be used by other modules. For a settings/config module, globals are typically defined precisely so that other modules can import them, so the appropriate fix is usually to make that intention explicit.

The single best fix here, without changing existing functionality, is to add CREST_PATH (and, for consistency, CREST_ENV_PATH) to the module’s __all__ list. If __all__ already exists in arc/settings/settings.py, we would add these names to it; if it does not, we will introduce a new __all__ definition near the bottom of the file (after the assignment to CREST_PATH, CREST_ENV_PATH). This explicitly declares these globals as part of the public API and will generally cause CodeQL’s “unused global variable” check to accept them as intentional. No new imports or helper methods are needed; we only add a small __all__ declaration.

Concretely, in arc/settings/settings.py, just below line 553 where CREST_PATH, CREST_ENV_PATH = find_crest_executable() is defined, we will insert a __all__ list that includes these two names. We will show this as a replacement block that replaces that final assignment line with the same assignment followed by the __all__ declaration.

In general, for an unused global that is intentionally kept (for documentation or for potential future use), rename it to follow an "unused" naming convention so static analysis tools and human readers understand it is not a mistake. If the value has side effects on the right-hand side, keep the right-hand side intact and only change or remove the left-hand side variable name.

Here, find_crest_executable() must still be called so that CREST_PATH continues to be initialized exactly as before; we only need to make it clear that the second return value is not used. The simplest non‑breaking change is to bind the environment component to a clearly unused name such as _unused_crest_env_path. This preserves all side effects and behavior of find_crest_executable(), keeps CREST_PATH available exactly as before, and satisfies the convention for unused variables described in the background.

Concretely, in arc/settings/settings.py at the bottom where CREST_PATH, CREST_ENV_PATH = find_crest_executable() is defined, change CREST_ENV_PATH to _unused_crest_env_path. No imports or additional methods are required.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When a duplicate TS guess is found, the code appends to the method string (e.g., 'Heuristics and CREST'). However, it doesn't update the method_sources list, which is now the canonical way to track multiple contributing methods. Consider updating method_sources here using TSGuess._normalize_method_sources() to maintain consistency with the new approach used in cluster_tsgs() and the CREST adapter.

Suggested change

	if method_label.lower() not in other_tsg.method.lower():
	other_tsg.method += f' and {method_label}'
	if method_label.lower() not in other_tsg.method.lower():
	base_method = other_tsg.method
	other_tsg.method += f' and {method_label}'
	existing_sources = getattr(other_tsg, "method_sources", None)
	if existing_sources:
	combined_sources = list(existing_sources) + [method_label]
	else:
	combined_sources = [base_method, method_label]
	other_tsg.method_sources = TSGuess._normalize_method_sources(combined_sources)

[Copilot]

The path parameter is defined but never used within the function body. This parameter should either be removed from the function signature or its intended usage should be implemented.

[Copilot]

The CREST executable search prioritizes a hard-coded path '/Local/ce_dana' which appears to be environment-specific. Consider making this configurable via an environment variable or moving it to lower priority to avoid issues in environments where this path doesn't exist or isn't relevant.

Suggested change

	# Priority 1: /Local/ce_dana standalone builds
	crest_path = find_highest_version_in_directory("/Local/ce_dana", "crest")
	# Priority 1: standalone builds in a configurable directory (default: /Local/ce_dana)
	standalone_dir = os.getenv("ARC_CREST_STANDALONE_DIR", "/Local/ce_dana")
	crest_path = find_highest_version_in_directory(standalone_dir, "crest")

[Copilot]

The extract_digits function will raise a ValueError if the input string contains no digits, as int("") will fail. Consider adding error handling or validation to ensure the string contains at least one digit before attempting conversion.

Suggested change

	return int(re.sub(r"[^\d]", "", s))
	digit_str = re.sub(r"[^\d]", "", s)
	if not digit_str:
	raise ValueError(f"No digits found in string: {s!r}")
	return int(digit_str)

[Copilot]

If all occurrences fail to compute distances (all KeyError exceptions at line 577), the occurrence_distances list will be empty, causing min() at line 581 to raise a ValueError. Consider adding a check to handle the case where occurrence_distances is empty, or raising a more descriptive error.

[Copilot]

The activation hook writes export AUTOTST_ROOT="$repo_path" into a shell script using the raw repo_path (derived from pwd), which will then be sourced on environment activation. If repo_path contains characters like $(…) or backticks (possible in directory names), those will be interpreted as command substitutions when the hook is executed, leading to command injection and arbitrary code execution in the user’s shell. To avoid this, ensure that the path is safely escaped or encoded before embedding it in a shell script (for example, avoid writing untrusted paths directly inside double quotes in sourced files).

[Copilot]

In the path-mode setup, the script replaces __AUTOTST_PATH__ in ~/.bashrc with the raw output of pwd, which is then used in a line export AUTOTST_ROOT="__AUTOTST_PATH__" added to the shell init file. If the working directory name includes characters such as $(…) or backticks, they will become part of ~/.bashrc and be executed as command substitutions when a new shell is started, enabling arbitrary code execution. You should sanitize or safely escape the directory path before writing it into ~/.bashrc, rather than interpolating pwd directly into a sourced shell script.