cleanup and submit

[egenn]

No description provided.

[gemini-code-assist]

Code Review

This pull request refactors the handling of the logfile parameter across the Agent class and security utilities. It simplifies the resolution logic in the generate method by removing the default fallback and updates the Agent class definition to require a character value for the logfile field. Documentation has been updated to reflect these changes and the CRAN policy regarding temporary files. Feedback was provided to include validation for the resolved logfile in the generate method to ensure it is a scalar character, preventing late-stage failures.

[Copilot]

Pull request overview

Updates the agent security log defaults/documentation to align with CRAN policy by avoiding a working-directory logfile default and centralizing the default path on an R option + tempfile() fallback.

Changes:

• Document logfile defaulting behavior as getOption("rtemis_security_logfile", tempfile(...)).
• Make Agent@logfile a required character field and simplify generate() logfile resolution to per-call override → agent field.
• Require logfile to be provided to report_agent_unauthorized_tool() and validate it as a non-empty scalar.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
r/man/create_agent.Rd	Updates create_agent() Rd docs for the new CRAN-compliant logfile default behavior.
r/R/utils_security.R	Makes incident logging require an explicit logfile and validates it.
r/R/07_Agent.R	Makes logfile a required Agent property, updates roxygen docs, and changes generate() logfile fallback logic.

---

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

[Copilot]

This change alters logfile resolution behavior (defaulting via create_agent/constructor and removing the fallback in generate). There are existing testthat tests for create_agent()/Agent behavior, but none assert the new default/option precedence for logfile. Please add tests to cover: (1) create_agent(logfile = NULL) uses getOption("rtemis_security_logfile") when set, else a tempfile; and (2) generate(logfile = NULL) uses x@logfile.

[Copilot]

After resolving logfile <- logfile %||% x@logfile, the value is never validated. Since Agent@logfile can still be NA_character_ or "" (the property type only enforces character, not non-empty), this can lead to late/indirect failures (e.g., incident logging) or silently skipping audit logging. Consider validating the resolved path immediately (e.g., with .check_scalar_character) and/or ensuring the parent directory exists before proceeding.

Suggested change

	logfile <- logfile %||% x@logfile
	logfile <- logfile %||% x@logfile
	if (!is.character(logfile) || length(logfile) != 1L || is.na(logfile) || !nzchar(logfile)) {
	stop("`logfile` must be a non-empty character scalar.", call. = FALSE)
	}
	logfile_dir <- dirname(logfile)
	if (!dir.exists(logfile_dir)) {
	dir.create(logfile_dir, recursive = TRUE, showWarnings = FALSE)
	if (!dir.exists(logfile_dir)) {
	stop(
	sprintf("Parent directory for `logfile` does not exist and could not be created: %s", logfile_dir),
	call. = FALSE
	)
	}
	}

[Copilot]

logfile is now a required character property, but neither the constructor nor the validator enforces that it is a non-empty scalar path. As a result, Agent(logfile = "")/NA_character_ will construct successfully and only fail later when logging is attempted. Consider validating logfile in the constructor after applying the NULL default (and/or in the class validator) so invalid paths are rejected at creation time.