diff --git a/.gitignore b/.gitignore index 34265c73..05497662 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ knowledge_bank/index/registry.jsonl cache/ data/ temp/ +workflows/ uploads/ config.json *.local diff --git a/.plans/0005-knowledge-bank-upgrade-parity.md b/.plans/0005-knowledge-bank-upgrade-parity.md new file mode 100644 index 00000000..235d0210 --- /dev/null +++ b/.plans/0005-knowledge-bank-upgrade-parity.md @@ -0,0 +1,8 @@ +# Give knowledge_bank the same backup-on-upgrade treatment as agents + +- **What:** Apply the dpkg-style `.bak.YYYYMMDD` backup-on-upgrade logic that `agents/` now has (per-file baseline in `copilotj/multiagent/agent_loader.py::sync_agent_configs`) to `knowledge_bank/`, so user edits to shipped `macro/`/`research/` TOMLs survive plugin upgrades instead of being silently overwritten. +- **Why:** `DefaultCopilotJBridgeService.extractPythonSources` re-extracts `knowledge_bank/*` into `$COPILOTJ_HOME` on every plugin upgrade — `jarFingerprint` (JAR path + `lastModified`) changes on every release, `resolveCopilotJSource` triggers re-extraction on the mismatch, and `Files.copy(..., REPLACE_EXISTING)` (`DefaultCopilotJBridgeService.java:614`) overwrites shipped files with no "was this user-modified?" check. Only user-*added* filenames survive; edits to shipped files are lost. After the agents relocation, `agents/` has a safer upgrade story than `knowledge_bank` — an inconsistency, and a latent user-data-loss path in the more user-facing store. +- **Pros:** Eliminates silent overwriting of user KB edits on upgrade; unifies the two user-editable TOML stores under one upgrade model. +- **Cons:** The cleanest fix lives in the Java extraction layer (backup-before-overwrite), or requires a Python kb sync that must coordinate with Java's extraction — either way crosses the Python/Java boundary and is its own design + test effort (roughly doubling this PR's scope if bundled). +- **Context:** Surfaced in the `/plan-eng-review` for the agents/workflow relocation (2026-07-02); deferred (TODO proposal 2 → "Add to TODOS"). The agents reference implementation is in `copilotj/multiagent/agent_loader.py` (`sync_agent_configs`, `_load_seed_baseline`, `_save_seed_baseline`, `_seed_files`). knowledge_bank bootstrap lives in `copilotj/multiagent/kb_tools.py` (`_bootstrap_kb`, now using `bootstrap_dir_if_empty`); extraction in `plugin/src/main/java/copilotj/DefaultCopilotJBridgeService.java` (`extractPythonSources`, `resolveCopilotJSource`). +- **Depends on / blocked by:** Nothing — independent follow-up. diff --git a/.plans/0006-agents-sync-cross-process-lock.md b/.plans/0006-agents-sync-cross-process-lock.md new file mode 100644 index 00000000..17d0e309 --- /dev/null +++ b/.plans/0006-agents-sync-cross-process-lock.md @@ -0,0 +1,8 @@ +# Add cross-process mutual exclusion to sync_agent_configs + +- **What:** Add a file lock (e.g. `fcntl.flock`) and an atomic baseline swap (`os.replace` from a temp name) around the refresh + baseline-write critical section in `copilotj/multiagent/agent_loader.py::sync_agent_configs`. +- **Why:** The seed-baseline read → per-file backup/copy → baseline-write sequence is not atomic, and the module-level `_synced` flag only guards within one process. Two processes sharing one `$COPILOTJ_HOME` (not the default single-bridge deployment, but plausible under multi-process or test setups) could both refresh concurrently. The backup logic itself is data-safe under concurrent syncers (the collision-avoiding `.bak.YYYYMMDD(.N)` naming + `filecmp` short-circuit preserve every distinct state), so this is hardening, not a known data-loss path. +- **Pros:** Eliminates the one identified concurrency fragility in the agents refresh; makes multi-instance `$COPILOTJ_HOME` safe if ever introduced. +- **Cons:** Adds locking complexity for a scenario (multi-process sharing) that the single-bridge deployment doesn't hit; risk of deadlocks if not careful, and Windows `fcntl` portability needs a fallback. +- **Context:** Surfaced by the `/review` adversarial pass (Claude F1, 2026-07-02); deferred as hardening. Single bridge server per `$COPILOTJ_HOME` is the supported deployment, so real-world concurrency is unlikely. Reference: `sync_agent_configs`, `_save_seed_baseline` in `copilotj/multiagent/agent_loader.py`. +- **Depends on / blocked by:** Nothing. diff --git a/AGENTS.md b/AGENTS.md index 12001899..77f05601 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,8 +30,8 @@ The core agent framework follows a layered architecture: - `LeaderDriven` (Pattern) — Main multi-agent pattern. Creates a `LeaderAgent` and loads specialized `Executor` agents from TOML configs. - `LeaderAgent` (ChatAgent) — ReAct-style leader that reasons, calls tools (ImageJ perception, macro execution, Python scripts, knowledge bank), and delegates to specialized agents. Manages dialog-level conversation history with summarization. - `Executor` (ChatAgent) — Generic specialized agent with its own system prompt and tools, loaded from TOML. Runs ReAct loops with tool retry and error recovery. -- `agent_configs/` — TOML files defining specialized agents. Active configs: `tool_agent.toml`, `research_agent.toml`, `success_case.toml`, `imagej_macro_help.toml`. Disabled configs have `.disabled.toml` suffix. -- `agent_loader.py` — Dynamically loads agent classes and tool functions from TOML configs using `importlib`. +- `agents/` — Repo-root dir of TOML seed files defining specialized agents (sibling of `knowledge_bank/`). At runtime the loader reads from `$COPILOTJ_HOME/agents/` (synced from the seed; see `sync_agent_configs`). Active configs: `tool_agent.toml`, `research_agent.toml`. Disabled templates use the `.disabled.toml` suffix; only `*_agent.toml` files are loaded. +- `agent_loader.py` — Dynamically loads agent classes and tool functions from TOML configs using `importlib`. Seeds/syncs `$COPILOTJ_HOME/agents/` dpkg-style (customized files are backed up as `.bak.YYYYMMDD` on upgrade). **`copilotj/server/`** — HTTP layer: diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 8e379739..0b5ee1d6 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -274,15 +274,24 @@ COPILOTJ_KB_AUTOSAVE=0 ### Agent configuration (advanced) -CopilotJ uses a configurable multi-agent architecture. Agent configuration files are located in -`copilotj/multiagent/agent_configs/`. Each configuration file defines an agent's system prompt, role description, -and optional constraints. - -**Customizing existing agents:** Modify prompt files in `agent_configs/` to adjust reasoning style, constrain -responsibilities, or tune domain-specific behavior. Changes take effect after restarting the server. - -**Adding new agents:** Copy an existing configuration file, define a unique agent name and role, write a system prompt, -and register any custom tools. +CopilotJ uses a configurable multi-agent architecture. The shipped defaults live in the repo-root +`agents/` directory (sibling of `knowledge_bank/`). At runtime the active configs are read from +`$COPILOTJ_HOME/agents/`. In dev mode (`COPILOTJ_HOME` unset → repo root) the two paths coincide, +so editing `agents/*.toml` directly takes effect after restarting the server. In production the +defaults are synced into `$COPILOTJ_HOME/agents/` from the bundled seed. + +**Customizing existing agents:** Edit the TOML in `$COPILOTJ_HOME/agents/` to adjust reasoning +style, constrain responsibilities, or tune domain-specific behavior. Changes take effect after +restarting the server. + +**Adding new agents:** Drop a new `*_agent.toml` file into `$COPILOTJ_HOME/agents/` (a unique +agent name, role, system prompt, and any custom tools). User-added configs are never touched by the +sync logic. + +**Upgrades:** When a release ships changed default configs, each file you have customized is first +backed up as `.bak.YYYYMMDD` inside `$COPILOTJ_HOME/agents/`, then overwritten with the +new default (so your edits are recoverable, not lost). Re-apply your edits from the `.bak` if needed. +Do not modify or delete those `.bak` files by hand. ## Testing diff --git a/copilotj/multiagent/agent_configs/coder_agent.disabled.toml b/agents/coder_agent.disabled.toml similarity index 100% rename from copilotj/multiagent/agent_configs/coder_agent.disabled.toml rename to agents/coder_agent.disabled.toml diff --git a/copilotj/multiagent/agent_configs/rag_agent.disabled.toml b/agents/rag_agent.disabled.toml similarity index 100% rename from copilotj/multiagent/agent_configs/rag_agent.disabled.toml rename to agents/rag_agent.disabled.toml diff --git a/copilotj/multiagent/agent_configs/research_agent.toml b/agents/research_agent.toml similarity index 100% rename from copilotj/multiagent/agent_configs/research_agent.toml rename to agents/research_agent.toml diff --git a/copilotj/multiagent/agent_configs/tool_agent.toml b/agents/tool_agent.toml similarity index 100% rename from copilotj/multiagent/agent_configs/tool_agent.toml rename to agents/tool_agent.toml diff --git a/copilotj/multiagent/agent_configs/web_search_agent.disabled.toml b/agents/web_search_agent.disabled.toml similarity index 100% rename from copilotj/multiagent/agent_configs/web_search_agent.disabled.toml rename to agents/web_search_agent.disabled.toml diff --git a/copilotj/core/config.py b/copilotj/core/config.py index 422b52d9..32ef56eb 100644 --- a/copilotj/core/config.py +++ b/copilotj/core/config.py @@ -26,6 +26,7 @@ "is_single_client", "load_managed_config", "save_managed_config", + "bootstrap_dir_if_empty", "bootstrap_assets", "resolve_vision_config", ] @@ -237,19 +238,35 @@ def save_managed_config(data: dict) -> None: path.write_text(json.dumps(data, indent=2), "utf-8") +def bootstrap_dir_if_empty(src: Path, dst: Path) -> bool: + """Copy a seed directory tree into ``dst`` only when ``dst`` is missing or empty. + + Used to seed user-data dirs (``assets`` and ``knowledge_bank``) from the bundled + source on first run. Note: the ``agents`` dir does NOT use this — it is user-editable + and uses the richer dpkg-style refresh in ``copilotj.multiagent.agent_loader``. + No-op (returns False) when: + + - ``src`` does not exist (no seed available), + - ``src`` and ``dst`` resolve to the same path (dev mode, where the home dir + IS the source tree, so copying would recurse into itself), + - ``dst`` already exists and is non-empty (user data present; never clobber). + + Otherwise copies ``src`` onto ``dst`` (``dirs_exist_ok=True``) and returns True. + """ + if not src.exists(): + return False + if src.resolve() == dst.resolve(): + return False + if dst.exists() and any(dst.iterdir()): + return False + shutil.copytree(src, dst, dirs_exist_ok=True) + return True + + def bootstrap_assets() -> None: """Copy assets/ from project source to COPILOTJ_HOME if missing.""" - home = get_home() source_root = Path(__file__).resolve().parent.parent.parent - source_assets = source_root / "assets" - target_assets = home / "assets" - - if not source_assets.exists(): - return - if target_assets.exists() and any(target_assets.iterdir()): - return - - shutil.copytree(source_assets, target_assets, dirs_exist_ok=True) + bootstrap_dir_if_empty(source_root / "assets", get_home() / "assets") def _is_dev(cfg: Config) -> bool: diff --git a/copilotj/multiagent/agent_configs/imagej_macro_help.toml b/copilotj/multiagent/agent_configs/imagej_macro_help.toml deleted file mode 100644 index 36a93d20..00000000 --- a/copilotj/multiagent/agent_configs/imagej_macro_help.toml +++ /dev/null @@ -1,271 +0,0 @@ - -["StarDist 2D"] -description = """ -Deep learning nuclei segmentation plugin. Requires special Command From Macro wrapper.\ -""" -correct_syntax = """ -run("Command From Macro", "command=[de.csbdresden.stardist.StarDist2D], \ -args=['input':'nuclei_duplicated', 'modelChoice':'Versatile (fluorescent nuclei)', \ -'normalizeInput':'true', 'percentileBottom':'1.0', 'percentileTop':'99.8', 'probThresh':'0.5', \ -'nmsThresh':'0.3', 'outputType':'Both', 'nTiles':'1', 'excludeBoundary':'2', \ -'roiPosition':'Automatic', 'verbose':'false', 'showCsbdeepProgress':'false', \ -'showProbAndDist':'false'], process=[false]");selectImage("Label Image");roiManager("Measure\");\ -""" -tips = [ - """ -1. modelChoice from the following models: "Versatile (fluorescent nuclei)", "Versatile (H&E nuclei)", "DSB 2018 (from StarDist 2D paper)". -2. The results title is usually 'Label Image'. After run stardist, you can use the \ -"roiManager("Measure"); " to measure, get the statistics(number, area, etc.) of the detected \ -nuclei. 3. The StarDist2D plugin is configured by specifying the input image (e.g. \ -"nuclei_duplicated") and choosing a pre-trained model such as "Versatile (fluorescent nuclei)" \ -or a custom weight file; you can enable normalizeInput=true to rescale intensities to [0,1] with \ -percentile clipping (percentileBottom and percentileTop, typically 1-5 and 95-99) to reduce \ -extreme values. The key detection thresholds are probThresh (default 0.5)—raise it (0.6-0.8) to \ -eliminate weak false positives or lower it (0.3-0.4) to capture faint objects—and nmsThresh \ -(default 0.3) to control non-max suppression of overlapping objects (lower values enforce \ -stricter separation). Choose outputType as "Mask," "Overlay," or "Both," and if you have very \ -large images or limited memory, set nTiles>1. Use excludeBoundary (0-2) to drop partial objects \ -at the border and roiPosition ("Automatic," "Image," or "ROI Manager") to control where ROIs \ -appear. Finally, enable verbose, showCsbdeepProgress, or showProbAndDist for detailed logs and \ -intermediate maps during development, then disable them for routine batch processing.\ -4. Input axis of type Channel must have size 1, adjust the channel before use it. -""", -] -site = "https://imagej.net/plugins/stardist" - -["3D Objects Counter"] -description = "Count 3D objects in stack. Parameter names use dots." -correct_syntax = """ -run("3D Objects Counter", "threshold=128 slice=6 min.=10 max.=1000000 exclude_objects_on_edges \ -statistics");\ -""" -tips = ["Use 'min.' and 'max.' with dots, not 'min_size' or 'max_size'."] -site = "https://imagej.net/plugins/3d-objects-counter" - -["Bio-Formats Importer"] -description = "Import microscopy files. Use 'open=' parameter for file path." -correct_syntax = """ -run("Bio-Formats Importer", "open=path/to/file.lsm autoscale color_mode=Default view=Hyperstack");\ -""" -tips = ["Use 'open=' not 'path=' or 'file='. Put full file path after open=."] -site = "https://imagej.net/formats/bio-formats" - -["Trainable Weka Segmentation"] -description = "Machine learning segmentation. Opens GUI, no parameters needed." -correct_syntax = 'run("Trainable Weka Segmentation", "");' -tips = [ - "Don't add parameters - this plugin opens a GUI for interactive training.", -] -site = "https://imagej.net/plugins/tws" - -[CLIJ2] -description = "GPU-accelerated image processing. Initialize GPU first." -correct_syntax = "Ext.CLIJ2_pushCurrentImage(input); Ext.CLIJ2_operation(input, output);" -tips = [ - "Must initialize CLIJ2 first. Use Ext.CLIJ2_ prefix for all operations. Push/pull images to/from GPU.", -] -site = "https://imagej.net/plugins/clij" - -[TrackMate] -description = """ -General-purpose spot detection and tracking plugin for ImageJ/Fiji. Headless mode supports full \ -parameterization via macro calls.\ -""" -correct_syntax = """ -run("TrackMate", "use_gui=false detector=[LoG detector (Laplacian of Gaussian)] radius=5.0 \ -threshold=10.0 subpixel=false median=true channel=1 tracker=[Simple LAP tracker] \ -linking.max_distance=5.0 gap_closing.max_distance=5.0 gap_closing.max_frame_gap=0 \ -filter_spots=[QUALITY filter] QUALITY=0.0 filter_tracks=[TRACK_DURATION filter] TRACK_DURATION=1 \ -display_results=true save_to=[C:/Users/10331/OneDrive/Desktop/TrackMateSaveTest2.xml] \ -export_to=[C:/Users/10331/OneDrive/Desktop/TrackMateExportTest2.xml]");\ -""" -tips = [ - """ -1. Use the Macro Recorder to generate the exact argument string for your data, and ensure the \ -target image window is selected before running.\ -""", - """ -2. radius and threshold control spot detection—tune radius to your object size and threshold to \ -eliminate background noise.\ -""", - """ -3. File paths for save_to and export_to must be enclosed in square brackets (e.g. \ -save_to=[C:/path/to/file.xml]).\ -""", - """ -4. median=true applies a 3×3 median filter before detection to reduce salt-and-pepper noise.\ -""", - """ -5. channel selects which channel in a multi-channel stack to analyze.\ -""", - """ -6. linking.max_distance and gap_closing.max_distance define how far a spot may move between \ -frames; gap_closing.max_frame_gap sets how many frames may be skipped.\ -""", - """ -7. If use_gui=true, the GUI will open and ignore other macro parameters as defaults.\ -""", - """ -8. save_to writes the full TrackMate model XML, while export_to writes a simplified tracks-only \ -XML. -""", - """ -9. display_results=true overlays detected spots and tracks on the image.\ -""", - """ -10. Always specify both detector and tracker in square brackets (e.g. detector=[LoG detector…], \ -tracker=[Simple LAP tracker]) or TrackMate will skip those steps.\ -""", - """ -11. Use forward-slashes or escape backslashes in Windows paths (e.g. C:/Users/… or \ -C:\\\\Users\\\\…) to avoid file-not-found errors.\ -""", - """ -12. If you see "spot collection is empty," increase blob radius, lower threshold, or toggle \ -median filtering, and test interactively in GUI mode (use_gui=true + Preview).\ -""", - """ -13. For batch processing, record working parameters with the Macro Recorder and paste them into a \ -headless macro to ensure consistency.\ -""", - """ -14. After running TrackMate, you can check the files saved in the specified paths to verify the \ -results.\ -""", -] -site = "https://imagej.net/plugins/trackmate" - -["Coloc 2"] -description = "Colocalization analysis plugin. Use 'channel1' and 'channel2' parameters." -correct_syntax = """ -run("Coloc 2", "channel_1=cropped_01_POS076_D.tif channel_2=CropGreen.tif roi_or_mask= \ -threshold_regression=Costes show_save_pdf_dialog li_histogram_channel_1 li_histogram_channel_2 \ -li_icq spearman's_rank_correlation manders'_correlation kendall's_tau_rank_correlation \ -2d_intensity_histogram costes'_significance_test psf=3 costes_randomisations=10\");\ -""" -tips = [ - "1. Adjust costes_randomisations for significance testing (default is 10).", - "2. Ensure images are pre-processed (e.g., background subtraction) before coloc analysis.", - "3. You need a 2-color channel image. If more than 2 channels, split them first (Image → Color → Split Channels).", - "4. Z stacks work fine, but time series will fail. Split time series into numbered images first.", - "5. For ROI analysis, you have 2 options:", - " - Select ROI with ImageJ selection tools (applies to all slices in z-stack)", - " - Use a binary mask image with same x,y,z dimensions (white=255 for analysis, black=0 to ignore)", - "6. In GUI, select your 2 images in the first 2 dropdown lists as channel 1 and channel 2.", - "7. Third dropdown: select ROI/mask image (must have same xyz dimensions as the other 2 images).", - "8. Results appear in ImageJ Log window as comma-separated values for export.", - "9. Set PSF size (in pixels) - determines size of image chunks for randomization.", - "10. Choose Costes iterations (minimum 10, better 100+ for statistical significance).", - "11. Results GUI shows tables and images (Li plots, scatterplots, 2D histograms with regression).", - "12. Scatterplot uses fire color LUT; log checkbox toggles log scaling display.", - "13. Save results as standardized PDF format for consistent comparison.", - "14. This tool may take a while to run, so you need to remind users to be patient.", -] -site = "https://imagej.net/plugins/coloc-2" - -[Threshold] -description = "Thresholding plugin for binary segmentation. Use 'method' parameter." -correct_syntax = 'run("Thresholding", "method=Otsu");' -tips = [ - "Use 'method=' to specify thresholding algorithm (e.g., Otsu, Li, etc.).", -] -site = "https://imagej.net/plugins/thresholding" - -[OpenComet] -description = """ -OpenComet is a plugin for ImageJ/Fiji that detects and analyzes comet-like structures in images, \ -commonly used in DNA damage assays.\ -""" -correct_syntax = 'run("OpenComet ");' -tips = [ - """ -1. Guide users to check the installation of the plugin and ensure it is properly configured. Jar \ -link: https://cometbio.org/versions/v1.3.1/OpenComet_jar_v1.3.1.zip\ -""", - """ -2. OpenComet dialog can be accessed via command: run(\"OpenComet \"); and you cannot pass other \ -parameters in the macro.\ -""", - """ -3. In the OpenComet dialog, users can select the input image, and output options, you should \ -guide users to select the correct input image and output options.\ -""", - """ -4. You should know the output, as it will have a csv file with the results, and you can analyze it. -""", -] -site = "https://cometbio.org/versions/v1.3.1/OpenComet_jar_v1.3.1.zip" - -["Colour Deconvolution2"] -correct_syntax = """ -run("Colour Deconvolution2", "vectors=H&E output=8bit_Transmittance simulated cross hide"); -""" -tips = [ - """ -1) Pick the right stain set: - - Use vectors that match your stain (e.g., [H&E], [H DAB], [H FastRed]). -""", - """ -2) Understand the outputs: - - output=8bit_Transmittance gives three 8-bit grayscale images (Stain1, Stain2, Residual). - - "simulated" adds a preview of the reconstructed RGB; "cross" overlays a crosshair; "hide" suppresses dialogs for batch use. -""", - """ -3) Preprocess before deconvolution (optional but helpful): - - Subtract Background (rolling ball 50-200 px) to correct illumination. - - Light Gaussian blur (σ≈1-2) to reduce noise if staining is grainy. -""", - """ -4) Post-processing & thresholding: - - For DAB (brown), invert the stain channel before thresholding (OD relation). - - Try Otsu/Triangle for strong contrast; MaxEntropy/Moments for faint or uneven stains. - - Keep thresholding consistent across slides for reproducibility. -""", -] - -# ["jSLIC superpixels 2D"] -# correct_syntax = 'run("jSLIC superpixels 2D", "init.=30 regularisation=0.20 export overlap=none colour");' -# tips = [ -# """ -# --- Related Commands --- -# • run("jSLIC superpixels 2D", "init.=100 regularisation=0.25 export overlap=none"); -# Example with different parameters and without color output. -# • run("jSLIC superpixels 2D", "init.=50 regularisation=0.15 export overlap=none colour"); -# Generate 50 superpixels with higher compactness. - -# --- Example Scenarios --- -# 1. Pre-segmentation of cell microscopy images to enable object-level feature extraction in downstream analysis. -# 2. Efficient region-based processing, where mean or median values are computed within each superpixel for denoising or texture analysis. -# 3. Use as an initial mask for further watershed or object segmentation steps, increasing speed and robustness. -# """ -# ] - -["3D Project"] -correct_syntax = 'run("3D Project...", "projection=[Brightest Point] axis=Y-Axis slice=1 initial=0 total=360 rotation=10 lower=1 upper=255 opacity=0 surface=100 interior=50");' -tips = [""" -"3D Project" is a feature of the Fiji 3D Viewer plugin used to perform volume rendering or projection on 3D image data. - -The "Brightest Point" projection takes the brightest voxel along the specified axis for each pixel column and maps its intensity to a 2D image, which is suitable for a quick view of the intensity distribution of 3D structures. - ---- Parameter Details --- -• projection=[Brightest Point]: Sets the projection mode to "Brightest Point". Other options include "Average Intensity" and "Maximum Intensity". -• axis=Y-Axis: Specifies the projection axis as the Y axis. To project along the X axis, use "axis=X-Axis". -• slice=1: Indicates to start from the first slice, usually set to 1. -• initial=0 total=360 rotation=10: Defines the rotation angle range and step size for 3D rotational projection, meaning from 0° to 360° with a step of 10°. -• lower=1 upper=255: Sets the intensity mapping range, mapping pixel values to the range [1, 255]. -• opacity=0: Sets rendering opacity to 0 (fully transparent, used for projection to avoid occlusion). -• surface=100 interior=50: Sets thresholds for surface and interior voxels during rendering, usually left at default values. - ---- Macro Example --- -run("3D Project...", "projection=[Brightest Point] axis=Y-Axis slice=1 initial=0 total=360 rotation=10 lower=1 upper=255 opacity=0 surface=100 interior=50"); - ---- Related Commands --- -• doCommand("Start Animation [\\]"): Starts the 3D rotational animation using the current projection settings. -• doCommand("Stop Animation"): Stops any ongoing 3D animation. - ---- Example Scenarios --- -1. A researcher has a 3D fluorescent image stack and wants to view the intensity distribution along the Y axis. After importing the image into Fiji, running the above macro generates a 2D projection image, facilitating subsequent quantitative or visual analysis. -2. When processing multiple 3D datasets, this command can be placed in a macro loop to apply the "Brightest Point" projection to every stack in a folder. -3. After generating the projection, use `doCommand("Start Animation [\\]")` to animate the projection rotation for presentation or inspection, then `doCommand("Stop Animation")` to halt the animation when needed. -""" -] \ No newline at end of file diff --git a/copilotj/multiagent/agent_configs/success_case.toml b/copilotj/multiagent/agent_configs/success_case.toml deleted file mode 100644 index 85f5e605..00000000 --- a/copilotj/multiagent/agent_configs/success_case.toml +++ /dev/null @@ -1,116 +0,0 @@ -[0] -query = """ -There are several images, I want to crop them and then, merge image channels -""" -correct_macro = [ - """ -// Define input and output directories -String inputDir = "C:/XXXX/ExampleFly/images/"; -String outputDir = inputDir + "cropped_merged/"; -File.makeDirectory(outputDir); - -// Open a sample image for the user to define the ROI -open(inputDir + "01_POS002_D.TIF"); -waitForUser("Define Crop Area", "Please draw a rectangular ROI on the image and then click OK."); - -// Get the selection bounds and close the sample image -getSelectionBounds(x, y, width, height); -close(); - -// Array of positions to process -String[] positions = { "002", "076", "218" }; - -// Loop through each position -for (int i = 0; i < positions.length; i++) { - String pos = positions[i]; - - // Open the three channels for the current position - open(inputDir + "01_POS" + pos + "_D.TIF"); - open(inputDir + "01_POS" + pos + "_F.TIF"); - open(inputDir + "01_POS" + pos + "_R.TIF"); - - // Apply the same ROI and crop each image - selectWindow("01_POS" + pos + "_D.TIF"); - makeRectangle(x, y, width, height); - run("Crop"); - - selectWindow("01_POS" + pos + "_F.TIF"); - makeRectangle(x, y, width, height); - run("Crop"); - - selectWindow("01_POS" + pos + "_R.TIF"); - makeRectangle(x, y, width, height); - run("Crop"); - - // Merge the cropped channels - run("Merge Channels...", "c1=01_POS" + pos + "_R.TIF c2=01_POS" + pos + "_F.TIF c3=01_POS" + pos + "_D.TIF create"); - - // Save the merged image - saveAs("Tiff", outputDir + "POS" + pos + "_merged.tif"); - - // Close all windows to prepare for the next iteration - run("Close All"); -} - -print("Batch processing complete."); -""", -] -correct_steps = [ - """ -1. I will run a single, continuous script. -2. The script will first open a sample image. -3. ImageJ will then **pause and wait for you** to draw a rectangle over the area you want to crop. -4. Once you click 'OK' on the dialog box, the script will automatically apply that exact crop to \ -all images, merge their channels, and save them in a new 'cropped_merged' folder. - -This process should solve the problem. May I proceed with this plan?\ -""", -] -type = "batch" - -[1] -query = "I want to analyze the colocalization of two channels in this image" -correct_macro = [ - """ -// Open the two channel images -open("C:/XXXX/ExampleFly/images/cropped_01_POS076_D.tif"); -open("C:/XXXX/ExampleFly/images/CropGreen.tif"); - -// Run Coloc 2 analysis -run("Coloc 2", "channel_1=cropped_01_POS076_D.tif channel_2=CropGreen.tif roi_or_mask= \ -threshold_regression=Costes show_save_pdf_dialog li_histogram_channel_1 li_histogram_channel_2 \ -li_icq spearman's_rank_correlation manders'_correlation kendall's_tau_rank_correlation \ -2d_intensity_histogram costes'_significance_test psf=3 costes_randomisations=10"); -""", -] -correct_steps = [ - "Ensure to select the correct channels for analysis.", - "Adjust parameters as needed based on your data.", -] -type = "colocalization" - -[2] -query = "I need to segment and count nuclei in this image" -correct_macro = [ - """ -```java -// Open the image -open("C:/XXXX/ExampleFly/images/cropped_01_POS076_D.tif"); - -// Convert to grayscale if needed -run("8-bit"); - -// Apply a threshold to segment nuclei -setAutoThreshold("Default"); -run("Convert to Mask"); - -// Analyze particles to count nuclei -run("Analyze Particles...", "size=100-Infinity show=Nothing display summarize add-to-manager"); -```\ -""", -] -correct_steps = [ - "Ensure the image is in the correct format for analysis.", - "Adjust the size parameters based on your specific nuclei size.", -] -type = "segmentation" diff --git a/copilotj/multiagent/agent_loader.py b/copilotj/multiagent/agent_loader.py index 67757b55..cbf381b0 100644 --- a/copilotj/multiagent/agent_loader.py +++ b/copilotj/multiagent/agent_loader.py @@ -2,25 +2,145 @@ # # SPDX-License-Identifier: Apache-2.0 +import filecmp import glob +import hashlib import importlib +import json import logging -import os +import shutil import tomllib +from datetime import date +from pathlib import Path from copilotj.core import FunctionTool, ModelClient, Tool -from copilotj.core.config import Config +from copilotj.core.config import Config, get_home from copilotj.multiagent.Executor import Executor -__all__ = ["load_agent_configs"] +__all__ = ["load_agent_configs", "sync_agent_configs"] _log = logging.getLogger(__name__) -GLOB_PATTERN = os.path.join(os.path.dirname(__file__), "agent_configs", "*_agent.toml") +_PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +_AGENT_TOML_GLOB = "*_agent.toml" +# Per-file baseline: maps seed filename -> sha256 of the shipped content at last sync. +# A whole-dir marker would refresh EVERY customized file on any seed bump; per-file +# tracking means a release that changes only one config only touches that one config. +_SEED_BASELINE_FILE = ".seed-versions.json" +_synced = False def load_agent_configs(*, model_client: ModelClient, cfg: Config): - return _load_agent_configs(GLOB_PATTERN, model_client=model_client, cfg=cfg) + _sync_agent_configs_once() + home_configs = get_home() / "agents" + return _load_agent_configs(str(home_configs / _AGENT_TOML_GLOB), model_client=model_client, cfg=cfg) + + +def _sync_agent_configs_once() -> None: + """Run :func:`sync_agent_configs` at most once per process. + + ``load_agent_configs`` is called per LeaderDriven pattern (once per chat thread); + the sync is idempotent but we avoid re-hashing the seed on every call. + """ + global _synced + if _synced: + return + sync_agent_configs() + _synced = True + + +def sync_agent_configs(source: Path | None = None, target: Path | None = None) -> None: + """Seed and refresh ``$COPILOTJ_HOME/agents`` from the bundled defaults. + + Per-file dpkg-style refresh. The seed baseline (``.seed-versions.json``) records + the shipped sha256 of each config at last sync. On each run a config is touched + ONLY if its shipped content changed since last sync (``baseline[name] != new``): + the user's current copy is backed up as ``.bak.YYYYMMDD`` and the new default + applied. Configs whose seed is unchanged are left entirely alone — so a release + that edits one agent does not revert a user's customization of a different agent. + New seed files are copied in; user-added configs are never touched. + ``.bak.YYYYMMDD`` files don't match the ``*_agent.toml`` glob, so backups are never + loaded as agents. + + In dev mode (``source`` and ``target`` resolve to the same path, i.e. + ``COPILOTJ_HOME`` is the repo root) this is a no-op beyond ensuring the directory + exists — developers edit the source seed directly. + """ + home = get_home() + source = _PROJECT_ROOT / "agents" if source is None else source + target = home / "agents" if target is None else target + target.mkdir(parents=True, exist_ok=True) + # Dev mode (home == repo root) or no seed available: nothing to sync. + if source.resolve() == target.resolve() or not source.exists(): + return + baseline = _load_seed_baseline(target / _SEED_BASELINE_FILE) + today = date.today().strftime("%Y%m%d") + seed_names: set[str] = set() + dirty = False + for src_file in _seed_files(source): + seed_names.add(src_file.name) + new_cs = hashlib.sha256(src_file.read_bytes()).hexdigest() + if baseline.get(src_file.name) == new_cs: + continue # This file's default is unchanged -> never touch the user's copy. + dst_file = target / src_file.name + if dst_file.exists() and not filecmp.cmp(src_file, dst_file, shallow=False): + _backup_before_overwrite(dst_file, target, today) + if not dst_file.exists() or not filecmp.cmp(src_file, dst_file, shallow=False): + shutil.copy2(src_file, dst_file) + baseline[src_file.name] = new_cs + dirty = True + # Surface configs that no longer ship (removed upstream) or are user-added. + for dst_file in target.glob(_AGENT_TOML_GLOB): + if dst_file.name not in seed_names: + _log.debug( + "Agent config %s is not in the shipped defaults (user-added or removed upstream).", + dst_file.name, + ) + if dirty: + _save_seed_baseline(target / _SEED_BASELINE_FILE, baseline) + + +def _seed_files(source: Path): + """Yield seed files (sorted, files only) — single source of truth for refresh + glob.""" + for f in sorted(source.glob("*")): + if f.is_file(): + yield f + + +def _load_seed_baseline(marker: Path) -> dict[str, str]: + if not marker.exists(): + return {} + try: + data = json.loads(marker.read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + return data if isinstance(data, dict) else {} + + +def _save_seed_baseline(marker: Path, baseline: dict[str, str]) -> None: + marker.write_text(json.dumps(baseline, sort_keys=True, indent=2), encoding="utf-8") + + +def _backup_before_overwrite(live: Path, target: Path, today: str) -> None: + """Save ``live`` to ``.bak.YYYYMMDD`` before it is overwritten. + + Never loses a distinct state: if a same-date backup already holds the exact + current content, skip (already preserved); otherwise write a collision-avoiding + name (``.bak.YYYYMMDD``, ``.bak.YYYYMMDD.2``, ``.bak.YYYYMMDD.3``, ...). + """ + candidate = target / f"{live.name}.bak.{today}" + n = 1 + while candidate.exists(): + if filecmp.cmp(candidate, live, shallow=False): + return # this exact content is already backed up today + n += 1 + candidate = target / f"{live.name}.bak.{today}.{n}" + shutil.copy2(live, candidate) + _log.info( + "Backed up customized %s to %s; applied new default. Re-apply your edits from the .bak if needed.", + live.name, + candidate.name, + ) def _load_agent_configs(glob_pattern: str, *, model_client: ModelClient, cfg: Config): @@ -36,9 +156,9 @@ def _load_agent_configs(glob_pattern: str, *, model_client: ModelClient, cfg: Co _log.error("Failed to load %s: %s", file, e) continue - # Skip files that are commented out or empty - if not config or config == "pass": - _log.info("Skipping %s: empty or commented out", file) + # Skip files that are empty + if not config: + _log.info("Skipping %s: empty", file) continue if "name" not in config or "class" not in config: @@ -119,7 +239,6 @@ def _load_agent_configs(glob_pattern: str, *, model_client: ModelClient, cfg: Co # Test: Load agent configurations and print each agent's tool list print("Loading agent configurations...") - agent_configs = os.path.join(os.path.dirname(__file__), "agent_configs") agents = load_agent_configs(model_client=new_model_client(cfg), cfg=cfg) for name, agent in agents.items(): print(f"Loaded agent: {name}") diff --git a/copilotj/multiagent/kb_tools.py b/copilotj/multiagent/kb_tools.py index f266ce48..c939796d 100644 --- a/copilotj/multiagent/kb_tools.py +++ b/copilotj/multiagent/kb_tools.py @@ -4,7 +4,6 @@ import json import re -import shutil import tomllib from pathlib import Path from typing import Annotated, Any @@ -14,7 +13,7 @@ from sklearn.feature_extraction.text import TfidfVectorizer from sklearn.metrics.pairwise import cosine_similarity -from copilotj.core.config import get_home +from copilotj.core.config import bootstrap_dir_if_empty, get_home from copilotj.core.message import TextMessage from copilotj.core.model_client import ModelClient @@ -67,11 +66,7 @@ def _ensure_dirs() -> None: def _bootstrap_kb() -> None: """Copy knowledge_bank from project source to COPILOTJ_HOME if missing.""" - if KB_ROOT.exists() and any(KB_ROOT.iterdir()): - return - source = _SOURCE_ROOT / "knowledge_bank" - if source.exists(): - shutil.copytree(source, KB_ROOT, dirs_exist_ok=True) + bootstrap_dir_if_empty(_SOURCE_ROOT / "knowledge_bank", KB_ROOT) def _read_toml(path: Path) -> dict[str, Any]: diff --git a/copilotj/multiagent/leader_multiagent.py b/copilotj/multiagent/leader_multiagent.py index 95d35f1d..5e4f4024 100644 --- a/copilotj/multiagent/leader_multiagent.py +++ b/copilotj/multiagent/leader_multiagent.py @@ -420,7 +420,7 @@ def __init__( # --- Load Agents and Define Tools --- rebuild_registry() # Rebuild registry to include any new workflow files - self.log_info("Loading agents from agent_configs...") + self.log_info("Loading agents...") # Load agents defined in configurations try: @@ -441,7 +441,7 @@ def __init__( ) except Exception as e: - self.log_error(f"Error loading agents from 'agent_configs': {e}") + self.log_error(f"Error loading agents: {e}") self.specialized_agents = {} # --- Initialize Leader Agent --- diff --git a/copilotj/multiagent/test_agent.py b/copilotj/multiagent/test_agent.py index 15a4e3a8..92929529 100644 --- a/copilotj/multiagent/test_agent.py +++ b/copilotj/multiagent/test_agent.py @@ -14,7 +14,7 @@ def __init__(self) -> None: super().__init__("copilotj.leader_driven") self.dialog_counter = 1 - # Load all agents from agent_configs and store in a dictionary + # Load all agents and store in a dictionary cfg = load_config() self.agents = load_agent_configs(model_client=new_model_client(cfg), cfg=cfg) for agent in self.agents.values(): diff --git a/copilotj/test/core/test_config_bootstrap.py b/copilotj/test/core/test_config_bootstrap.py new file mode 100644 index 00000000..637048b8 --- /dev/null +++ b/copilotj/test/core/test_config_bootstrap.py @@ -0,0 +1,50 @@ +# SPDX-FileCopyrightText: Copyright contributors to the CopilotJ project. +# +# SPDX-License-Identifier: Apache-2.0 + +from pathlib import Path + +from copilotj.core.config import bootstrap_dir_if_empty + + +def _seed(path: Path, name: str, body: str = "seed") -> None: + path.mkdir(parents=True, exist_ok=True) + (path / name).write_text(body, encoding="utf-8") + + +def test_copies_when_dst_missing(tmp_path): + src, dst = tmp_path / "src", tmp_path / "dst" + _seed(src, "a.toml") + assert bootstrap_dir_if_empty(src, dst) is True + assert (dst / "a.toml").read_text() == "seed" + + +def test_copies_when_dst_empty(tmp_path): + src, dst = tmp_path / "src", tmp_path / "dst" + _seed(src, "a.toml") + dst.mkdir() + assert bootstrap_dir_if_empty(src, dst) is True + assert (dst / "a.toml").exists() + + +def test_noop_when_dst_populated(tmp_path): + src, dst = tmp_path / "src", tmp_path / "dst" + _seed(src, "a.toml", body="NEW") + _seed(dst, "existing.toml", body="USER") + assert bootstrap_dir_if_empty(src, dst) is False + assert not (dst / "a.toml").exists() # nothing copied + assert (dst / "existing.toml").read_text() == "USER" # user data intact + + +def test_noop_when_src_missing(tmp_path): + dst = tmp_path / "dst" + assert bootstrap_dir_if_empty(tmp_path / "nope", dst) is False + assert not dst.exists() + + +def test_noop_when_src_is_dst(tmp_path): + # Dev mode: home dir IS the source tree -> must not copy a dir onto itself. + src = tmp_path / "shared" + _seed(src, "a.toml") + assert bootstrap_dir_if_empty(src, src) is False + assert not (src / ".anything").exists() # no marker/side effects written diff --git a/copilotj/test/multiagent/test_agent_loader.py b/copilotj/test/multiagent/test_agent_loader.py index 9bba4d2e..380f3010 100644 --- a/copilotj/test/multiagent/test_agent_loader.py +++ b/copilotj/test/multiagent/test_agent_loader.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 import textwrap +from pathlib import Path from typing import Annotated from unittest.mock import MagicMock @@ -11,7 +12,7 @@ from copilotj.core import FunctionTool from copilotj.core.config import Config from copilotj.multiagent import agent_loader as _loader_module -from copilotj.multiagent.agent_loader import _load_agent_configs +from copilotj.multiagent.agent_loader import _load_agent_configs, load_agent_configs, sync_agent_configs # --------------------------------------------------------------------------- @@ -327,3 +328,148 @@ async def _echo(query: Annotated[str, "Ignored"]) -> str: return cfg.tavily_api_key or "" return _echo + + +# --------------------------------------------------------------------------- +# Tests — sync_agent_configs (dpkg-style refresh into $COPILOTJ_HOME/agents) +# --------------------------------------------------------------------------- + +_AGENT_TOML = """ +name = "Sync Agent" +class = "copilotj.test.multiagent.test_agent_loader._FakeExecutor" +description = "" +prompt = "" +""" + + +def _seed(src: Path, name: str, body: str) -> None: + src.mkdir(parents=True, exist_ok=True) + (src / name).write_text(textwrap.dedent(body), encoding="utf-8") + + +def test_sync_first_run_copies_seed(tmp_path): + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + assert (dst / "sync_agent.toml").read_text().strip() != "" + assert (dst / ".seed-versions.json").exists() + + +def test_sync_dev_guard_when_source_is_target(tmp_path): + src = tmp_path / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + # source and target resolve to the same path -> dev mode, no marker, no copy churn + sync_agent_configs(source=src, target=src) + assert not (src / ".seed-versions.json").exists() + + +def test_sync_unrelated_seed_change_preserves_unrelated_customization(tmp_path): + """Codex P1: a seed bump to ONE file must not back up + revert a DIFFERENT file the + user customized, when that other file's own default did not change.""" + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "a_agent.toml", _AGENT_TOML) + _seed(src, "b_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + # User customizes a_agent; then upstream ships a change to b_agent ONLY. + (dst / "a_agent.toml").write_text("USER A", encoding="utf-8") + (src / "b_agent.toml").write_text("# new b default\n", encoding="utf-8") + sync_agent_configs(source=src, target=dst) + assert (dst / "a_agent.toml").read_text() == "USER A" # untouched: its seed didn't change + assert not list(dst.glob("a_agent.toml.bak.*")) # ...and not backed up + assert (dst / "b_agent.toml").read_text() == "# new b default\n" # b refreshed + + +def test_sync_noop_when_source_missing(tmp_path): + dst = tmp_path / "home" / "agents" + sync_agent_configs(source=tmp_path / "does_not_exist", target=dst) + assert dst.exists() # target is still ensured + assert not (dst / ".seed-versions.json").exists() + + +def test_sync_marker_match_preserves_user_edit(tmp_path): + """When the shipped seed is unchanged, a user's live edit is NOT touched.""" + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) # initial seed + marker + # User now edits the live file. + (dst / "sync_agent.toml").write_text("USER CUSTOMIZATION", encoding="utf-8") + sync_agent_configs(source=src, target=dst) # marker still matches -> no-op + assert (dst / "sync_agent.toml").read_text() == "USER CUSTOMIZATION" + assert not list(dst.glob("*.bak.*")) # no backup created + + +def test_sync_seed_changed_backs_up_user_edit(tmp_path): + """On upgrade, a customized file is backed up (.bak.YYYYMMDD) then replaced.""" + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + # User customizes; then upstream ships a new default. + (dst / "sync_agent.toml").write_text("USER CUSTOMIZATION", encoding="utf-8") + (src / "sync_agent.toml").write_text("# new default\n", encoding="utf-8") + sync_agent_configs(source=src, target=dst) + backups = list(dst.glob("sync_agent.toml.bak.*")) + assert len(backups) == 1 + assert backups[0].read_text() == "USER CUSTOMIZATION" # edit preserved + assert (dst / "sync_agent.toml").read_text() == "# new default\n" # new default live + + +def test_sync_same_day_second_refresh_preserves_second_edit(tmp_path): + """A second seed bump the same day must back up a NEW user edit, not silently + overwrite it just because a .bak from the first refresh already exists.""" + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + # First customization + first same-day seed bump -> .bak captures it. + (dst / "sync_agent.toml").write_text("USER EDIT 1", encoding="utf-8") + (src / "sync_agent.toml").write_text("# default v2\n", encoding="utf-8") + sync_agent_configs(source=src, target=dst) + # Second customization + second same-day seed bump -> must NOT be lost. + (dst / "sync_agent.toml").write_text("USER EDIT 2", encoding="utf-8") + (src / "sync_agent.toml").write_text("# default v3\n", encoding="utf-8") + sync_agent_configs(source=src, target=dst) + backup_contents = sorted(p.read_text() for p in dst.glob("sync_agent.toml.bak.*")) + assert "USER EDIT 1" in backup_contents + assert "USER EDIT 2" in backup_contents # second edit preserved, not overwritten + assert (dst / "sync_agent.toml").read_text() == "# default v3\n" + + +def test_sync_identical_file_not_backed_up(tmp_path): + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + # Second sync with identical seed -> no backup, no change. + sync_agent_configs(source=src, target=dst) + assert not list(dst.glob("*.bak.*")) + + +def test_sync_new_seed_file_copied(tmp_path): + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + _seed(src, "extra_agent.toml", _AGENT_TOML) + sync_agent_configs(source=src, target=dst) + assert (dst / "extra_agent.toml").exists() + + +def test_sync_orphan_logged_at_debug(tmp_path, caplog): + src, dst = tmp_path / "seed", tmp_path / "home" / "agents" + _seed(src, "sync_agent.toml", _AGENT_TOML) + dst.mkdir(parents=True) + (dst / "ghost_agent.toml").write_text("orphan", encoding="utf-8") # not in seed + import logging as _logging + + with caplog.at_level(_logging.DEBUG, logger=_loader_module._log.name): + sync_agent_configs(source=src, target=dst) + assert any("ghost_agent.toml" in r.message for r in caplog.records) + + +def test_load_agent_configs_reads_from_home(tmp_path, monkeypatch): + """Regression: the public loader reads from $COPILOTJ_HOME/agents, not __file__.""" + monkeypatch.setenv("COPILOTJ_HOME", str(tmp_path)) + monkeypatch.setattr(_loader_module, "_synced", False) + monkeypatch.setattr(_loader_module, "sync_agent_configs", lambda *a, **k: None) # don't copy real seed + cfgs = tmp_path / "agents" + cfgs.mkdir() + _seed(cfgs, "home_agent.toml", _AGENT_TOML.replace("Sync Agent", "Home Agent")) + agents = load_agent_configs(model_client=_make_model_client(), cfg=_make_cfg()) + assert "Home Agent" in agents diff --git a/copilotj/test/workflow/__init__.py b/copilotj/test/workflow/__init__.py new file mode 100644 index 00000000..d8d52ffa --- /dev/null +++ b/copilotj/test/workflow/__init__.py @@ -0,0 +1,3 @@ +# SPDX-FileCopyrightText: Copyright contributors to the CopilotJ project. +# +# SPDX-License-Identifier: Apache-2.0 diff --git a/copilotj/test/workflow/test_manager_migration.py b/copilotj/test/workflow/test_manager_migration.py new file mode 100644 index 00000000..094c1bfd --- /dev/null +++ b/copilotj/test/workflow/test_manager_migration.py @@ -0,0 +1,96 @@ +# SPDX-FileCopyrightText: Copyright contributors to the CopilotJ project. +# +# SPDX-License-Identifier: Apache-2.0 + +import importlib +from pathlib import Path + +from copilotj.workflow import manager as workflow_manager +from copilotj.workflow.manager import migrate_workflow_layout + + +def _set_home(monkeypatch, tmp_path: Path) -> Path: + """Point COPILOTJ_HOME at tmp_path so migrate_workflow_layout operates there.""" + monkeypatch.setenv("COPILOTJ_HOME", str(tmp_path)) + return tmp_path + + +def _wf(root: Path, wf_id: str, body: str = "{}") -> Path: + f = root / "temp" / "workflows" / wf_id / "workflow.json" + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(body, encoding="utf-8") + return f + + +def test_migrate_noop_when_old_missing(tmp_path, monkeypatch): + home = _set_home(monkeypatch, tmp_path) + migrate_workflow_layout() # no temp/workflows -> nothing to do + assert not (home / "workflows").exists() or not any((home / "workflows").iterdir()) + + +def test_migrate_moves_when_new_missing(tmp_path, monkeypatch): + home = _set_home(monkeypatch, tmp_path) + _wf(home, "wf1", '{"id": "wf1"}') + migrate_workflow_layout() + assert (home / "workflows" / "wf1" / "workflow.json").read_text() == '{"id": "wf1"}' + assert not (home / "temp" / "workflows" / "wf1").exists() # moved, not copied + + +def test_migrate_moves_children_when_new_empty(tmp_path, monkeypatch): + home = _set_home(monkeypatch, tmp_path) + _wf(home, "wf1") + (home / "workflows").mkdir(parents=True) # new exists but empty + migrate_workflow_layout() + assert (home / "workflows" / "wf1" / "workflow.json").exists() + + +def test_migrate_merges_into_populated_new(tmp_path, monkeypatch): + """A populated new dir is preserved (no clobber) but old children are still merged in + (no early-out that would strand data after a partial prior run).""" + home = _set_home(monkeypatch, tmp_path) + _wf(home, "old_wf") + existing = home / "workflows" / "existing_wf" / "workflow.json" + existing.parent.mkdir(parents=True) + existing.write_text("KEEP", encoding="utf-8") + migrate_workflow_layout() + assert existing.read_text() == "KEEP" # existing workflow preserved (no clobber) + assert (home / "workflows" / "old_wf" / "workflow.json").exists() # old_wf merged in + assert not (home / "temp" / "workflows" / "old_wf").exists() # ...and moved out of temp + + +def test_migrate_completes_after_partial_run(tmp_path, monkeypatch): + """A previously-interrupted migration (some children moved, some stranded) completes + on the next run instead of abandoning the stranded children.""" + home = _set_home(monkeypatch, tmp_path) + _wf(home, "wf_a") + _wf(home, "wf_b") + # Simulate a partial prior migration: wf_a already in new, wf_b still stranded in temp. + (home / "workflows" / "wf_a").mkdir(parents=True) + (home / "workflows" / "wf_a" / "workflow.json").write_text("ALREADY", encoding="utf-8") + migrate_workflow_layout() + assert (home / "workflows" / "wf_b" / "workflow.json").exists() # stranded child recovered + assert (home / "workflows" / "wf_a" / "workflow.json").read_text() == "ALREADY" + + +def test_migrate_idempotent(tmp_path, monkeypatch): + home = _set_home(monkeypatch, tmp_path) + _wf(home, "wf1") + migrate_workflow_layout() + migrate_workflow_layout() # second run: new is populated -> no-op + assert (home / "workflows" / "wf1" / "workflow.json").exists() + + +def test_import_runs_migration_before_mkdir(tmp_path, monkeypatch): + """Regression (codex #1/#2): the module-level mkdir must not pre-create the new + dir and defeat migration. Importing the module with old data present must move it. + """ + home = _set_home(monkeypatch, tmp_path) + _wf(home, "wf_import", '{"id": "wf_import"}') + try: + importlib.reload(workflow_manager) + # After reload, migration ran (before mkdir) and BASE_DIR points at the new home. + assert (workflow_manager.BASE_DIR / "wf_import" / "workflow.json").exists() + assert not (home / "temp" / "workflows" / "wf_import").exists() + finally: + monkeypatch.delenv("COPILOTJ_HOME", raising=False) + importlib.reload(workflow_manager) # restore module state to default home diff --git a/copilotj/workflow/manager.py b/copilotj/workflow/manager.py index 0620936a..092d598c 100644 --- a/copilotj/workflow/manager.py +++ b/copilotj/workflow/manager.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 import json +import logging import re import shutil import time @@ -11,10 +12,60 @@ from pathlib import Path from typing import Any, Dict, List, Optional -from copilotj.multiagent.py_tools import get_project_temp_dir +from copilotj.core.config import get_home from copilotj.workflow.contract import RUNS_DIR, SCHEMA_VERSION, WorkflowInterface, parse_interface -BASE_DIR = get_project_temp_dir("workflows") +_log = logging.getLogger(__name__) + + +def migrate_workflow_layout() -> None: + """Relocate the workflow library from ``/temp/workflows`` to ``/workflows``. + + The library previously lived under the ``temp/`` subtree even though it is + long-lived, user-curated data. Moves each child of the old directory into the new + directory, skipping name collisions (never clobbering an existing workflow). + Idempotent and resilient to interruption: there is no early-out on a partially + populated ``new`` dir, so a previously-interrupted migration completes on the next + run instead of stranding the remaining children under ``temp/``. Cross-device + renames fall back to copy+delete. + """ + home = get_home() + old = home / "temp" / "workflows" + new = home / "workflows" + if not old.exists(): + return + new.mkdir(parents=True, exist_ok=True) + for child in list(old.iterdir()): + dst = new / child.name + if dst.exists(): + continue # Name collision with an existing workflow; leave both in place. + try: + child.rename(dst) + except OSError: + # Cross-device link or permission issue: fall back to copy + delete. + try: + shutil.move(str(child), str(dst)) + except OSError as e: + _log.warning("Could not migrate workflow %s -> %s: %s", child, dst, e) + continue + # Everything moved out -> remove the now-empty old directory. + if not any(old.iterdir()): + try: + old.rmdir() + except OSError: + pass + + +# Migrate BEFORE creating the new dir structure, so the "absent or empty" guard +# above still holds (BASE_DIR/shared creation would otherwise make 'new' look +# populated and silently skip migration). Runs at every entry point that imports +# this module (appose_worker, scripts/run-workflow.py, copilotj/server/__main__). +try: + migrate_workflow_layout() +except Exception as e: # noqa: BLE001 - never block startup on migration + _log.warning("Workflow library migration skipped: %s", e) + +BASE_DIR = get_home() / "workflows" SHARE_DIR = BASE_DIR / "shared" BASE_DIR.mkdir(parents=True, exist_ok=True) SHARE_DIR.mkdir(parents=True, exist_ok=True) diff --git a/plugin/pom.xml b/plugin/pom.xml index 5df8de71..f475aca7 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -288,6 +288,7 @@ copilotj/**/*.py copilotj/**/*.toml knowledge_bank/**/*.toml + agents/**/*.toml assets/** pyproject.toml uv.lock diff --git a/web/src/assets/manual.md b/web/src/assets/manual.md index 9d1e493c..3f86aa78 100644 --- a/web/src/assets/manual.md +++ b/web/src/assets/manual.md @@ -359,11 +359,14 @@ The `/temp` folder serves as a centralized location for artifacts - processed images and intermediate image results - measurement tables such as CSV files - generated reports and logs -- saved workflows in Markdown or JSON -- optional ZIP bundles containing workflows, data, and metadata +- per-run workflow output directories The temporary folder is managed by the CopilotJ core server and updates in real time as workflows execute. Unless explicitly cleaned or overwritten, it preserves outputs from the current session for debugging, validation, and reproducibility. +**Saved workflows** (your reusable workflow library — workflow JSON, Markdown exports, and optional ZIP bundles) live in `/workflows/`, kept separate from the temp folder so they persist across sessions. + +**Agent configuration files** live in `/agents/`. When CopilotJ is updated and a default agent config has changed since you customized it, your edited copy is preserved as a backup named `.bak.YYYYMMDD` before the new default is written. **Do not modify, rename, or delete these `.bak` files** — they are your recovery copy. To keep your customizations, diff the `.bak` against the new default and re-apply your edits to the live config by hand. + ## FAQ