Store session automation artifacts with their sessions - #1824
Closed
Aaronontheweb wants to merge 11 commits into
Closed
Store session automation artifacts with their sessions#1824Aaronontheweb wants to merge 11 commits into
Aaronontheweb wants to merge 11 commits into
Conversation
Aaronontheweb
added
cleanup
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| public string GetOutputLogPath(BackgroundJobId id) => | ||
| Path.Combine(GetOutputDirectory(id), "output.log"); | ||
| public string GetOutputLogPath(BackgroundJobId id, SessionId sessionId) => | ||
| Path.Combine(GetOutputDirectory(id, sessionId), "output.log"); |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+234
to
+238
| foreach (var path in FindDefinitionPaths(id)) | ||
| { | ||
| if (TryReadValidDefinition(path) is { } stored && stored.Definition.Id == id) | ||
| definitions.Add(stored); | ||
| } |
Comment on lines
+237
to
+241
| foreach (var path in FindDefinitionPaths(id)) | ||
| { | ||
| if (TryReadValidDefinition(path) is { } stored && stored.Definition.Id == id) | ||
| definitions.Add(stored); | ||
| } |
Comment on lines
+249
to
+256
| foreach (var jobDirectory in EnumerateSessionJobDirectories()) | ||
| { | ||
| var path = GetContainedPath(jobDirectory, fileName, id); | ||
| if (IsSafeSessionPath(path, out var reason)) | ||
| AddIfPresent(paths, path); | ||
| else | ||
| _logger.LogError("Session job definition {Path} is unsafe: {Reason}", path, reason); | ||
| } |
Comment on lines
+252
to
+259
| foreach (var reminderDirectory in EnumerateSessionReminderDirectories()) | ||
| { | ||
| var path = GetContainedPath(reminderDirectory, fileName, id); | ||
| if (IsSafeSessionPath(path, out var reason)) | ||
| AddIfPresent(paths, path); | ||
| else | ||
| _logger.LogError("Session reminder definition {Path} is unsafe: {Reason}", path, reason); | ||
| } |
| { | ||
| var directory = GetOutputDirectoryPath(GetStorageDirectory(id, sessionId), id); | ||
| EnsureSafeOutputPath(directory); | ||
| return Path.Combine(directory, "output.log"); |
Comment on lines
+105
to
+115
| foreach (var autonomous in new[] { true, false }) | ||
| { | ||
| var allowed = policy.TryResolveWritePath( | ||
| artifactPath, | ||
| Ctx(TrustAudience.Personal, autonomous), | ||
| out _, | ||
| out var error); | ||
|
|
||
| Assert.False(allowed); | ||
| Assert.Contains("automation artifacts", error, StringComparison.OrdinalIgnoreCase); | ||
| } |
Aaronontheweb
commented
Aug 9, 2026
Aaronontheweb
left a comment
Aaronontheweb
left a comment
Collaborator
Author
There was a problem hiding this comment.
Left some comments
| _paths = paths; | ||
| _logger = logger ?? NullLogger<ReminderDefinitionStore>.Instance; | ||
| Directory.CreateDirectory(_directory); | ||
| Directory.CreateDirectory(_paths.RemindersDirectory); |
| } | ||
| } | ||
|
|
||
| private IEnumerable<string> EnumerateSessionReminderDirectories() |
Collaborator
Author
There was a problem hiding this comment.
Seems like we should only do this in instances where we have a stored reminder in the database somewhere. Because otherwise, we might be enumerating thousands of directories potentially.
|
|
||
| private bool TryGetSavePath(ReminderDefinition definition, out string path, out string error) | ||
| { | ||
| var definitions = FindValidDefinitions(definition.Id); |
Collaborator
Author
There was a problem hiding this comment.
How can there possibly be multiple sessions for the same reminder definition? And why pick the first one?
|
|
||
| foreach (var sessionDirectory in Directory.EnumerateDirectories(_paths.SessionsDirectory)) | ||
| { | ||
| var jobDirectory = Path.Combine(sessionDirectory, SessionDirectoryHelper.JobsSubdirectory); |
Collaborator
Author
|
This change is too broad for the next release. We will ship the bounded background-job cleanup in #1821 and revisit session-owned automation storage separately. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Compatibility
Existing scheduler payloads remain ID-only.
Existing reminder definitions, histories, job definitions, and logs do not move.
The managers keep their current ownership, limits, retry rules, passivation rules, and delivery routes.
The default agent reminder list shows active reminders.
The operator CLI list includes disabled and terminal reminders.
Validation
Netclaw.Actors.Tests: 2,940 passed.Netclaw.Security.Tests: 685 passed.Netclaw.Daemon.Tests: 951 passed.git diff --checkpassed.The eval suite did not run.
This change does not modify an eval-sensitive surface.
Closes #1820