Skip to content

fix(tasks): mirror parent_id to Project body on create when absent - #574

Merged
prasad-albert merged 5 commits into
mainfrom
worktree-sdk-audit
Sep 30, 2026
Merged

prasad-albert merged 5 commits into
mainfrom
worktree-sdk-audit

Conversation

@prasad-albert

@prasad-albert prasad-albert commented Jun 30, 2026 •

Copy link
Copy Markdown
Collaborator

What

General and property task creates with parent_id set to a project ID now automatically include the Project entity link in the POST body when callers omit project, so tasks are linked to the project on create.

Why

Staging verification showed that without Project in the request body, General and Property tasks could be created with only a query parentId but were not associated with the project. Batch tasks already worked without this (the API backfills Project from the query param).

How

Mirror parent_id into Project.id only when:

  • Project is not already in the serialized payload (preserves property tasks that use parent_id=inventory_id with an explicit project), and
  • parent_id starts with PRO (project ID, not inventory).

Testing

  • uv run ruff format .
  • uv run ruff check . --fix
  • uv run pytest tests/collections/test_tasks.py -v
  • Live staging repro (one-off): batch/general/property create scenarios with and without Project mirroring

SDK Changes

Task creates with parent_id set to a project ID no longer require callers to also set project for General and Property tasks to be linked to that project.

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Looks good - no bugs or correctness issues found.

Comment thread src/albert/resources/tasks.py Outdated
The location where the property task is performed.
parent_id : str
The ID of the parent project.
The ID of the parent inventory item.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waiting on confirmation of what exactly parentId is here? inventory or project

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The agent worker workaround mirrors parent_id → project for PropertyTask.This PR does two things that point the other way:

In create(), it mirrors parent_id → Project body only for BatchTask (isinstance(task, BatchTask)), not PropertyTask.

BaseTask carries both parent_id and project inherited by every task type, so both interpretations are structurally possible. The two views cannot both be correct:

If this change is right(PropertyTask.parent_id is an inventory item, only BatchTask.parent_id is a project): then the worker's PropertyTask mirror is actively wrong. It is stuffing an inventory id into the Project entity link. That mirror should be removed, not kept, and this is the correct fix for the BatchTask side.
If the worker is right (PropertyTask.parent_id is a project), then this change is incomplete (it should also mirror PropertyTask) and its docstring change is wrong.
I believe this needs confirmation from the task API owner or a real create test for both task types before merge

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed with @siddhant-albertinvent -- parentId is always project ID irrespective of task type.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, the tests are failing and they expect parent ID to be inventory ID for Property Tasks
cc: @siddhant-albertinvent

When a BatchTask is created with parent_id set but project omitted, the API
requires the Project entity in the request body for ACL resolution. Without
it the server crashes at runtime. Auto-populate Project from parent_id in
that case.

Also corrects the PropertyTask docstring: parent_id refers to the parent
inventory item, not a project.
parentId in the query param and Project.id in the payload must always
match, regardless of task type. Also reverts an incorrect PropertyTask
docstring edit (parent_id is the parent project, not inventory item).
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Looks good - no bugs or correctness issues found.

Only inject Project into the create payload when it is missing and
parent_id is a PRO-prefixed project ID, preserving the inventory
parent_id + explicit project pattern used by property tasks.
@prasad-albert prasad-albert changed the title fix(tasks): mirror parent_id to Project body for BatchTask create fix(tasks): mirror parent_id to Project body on create when absent Sep 1, 2026
@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown

Looks good - no bugs or correctness issues found.

lkubie commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Bump: this has been open since June with prior reviews from prakashchandrahv and sumedhb1995. @prasad-albert is this still wanted? If yes, happy to rebase it onto current main and re-request review.

@prasad-albert

Copy link
Copy Markdown
Collaborator Author

Bump: this has been open since June with prior reviews from prakashchandrahv and sumedhb1995. @prasad-albert is this still wanted? If yes, happy to rebase it onto current main and re-request review.

this was not merged because we couldn't verify the fix -- i will look at this today

Extract the Project-mirroring guard into a pure helper in utils/tasks.py
and apply it in create_many as well, which had the same gap as create.
Add unit tests for the helper and an integration test linking a General
task to its parent project.

Copy link
Copy Markdown
Collaborator Author

Picked up the failing CI and missing coverage:

What was wrong

  • ci/circleci: Test was failing because the branch was ~180 commits behind main (cut 2026-07-03). 8 markdown files fail ruff format --check at the old merge-base; this was already fixed on main.
  • The pytest checklist item was unchecked and there was no test coverage for the mirroring logic.

What I changed (commit dc98a5c + merge from main)

  • Merged latest main — resolves the formatting failures.
  • Extracted the mirroring guard into a pure helper mirror_project_from_parent_id in src/albert/utils/tasks.py per the testing conventions (branching logic in a collection goes into a pure helper with unit tests). Also made the PRO-prefix check case-insensitive, consistent with core/shared/identifiers.py.
  • Applied the same helper in create_many, which had the identical gap (General/Property tasks created without Project in the body are never linked to the project — verified in api-task postMultiTask.js: only LAB/BATCHWITHQC get task.Project backfilled from the query param).
  • Added 5 unit tests for the helper and an integration test (test_create_with_project_parent_id_links_project).

Verification

  • ruff format --check, ruff check: clean (452 files).
  • pytest tests/unit: 1249 passed, 2 xfailed.
  • Live staging verification (entities created and cleaned up): General task with parent_id=PRO only → linked to project ✅; create_many with parent_id=PRO → linked ✅; Property task with parent_id=INV + explicit project → project preserved ✅

@claude

claude Bot commented Sep 30, 2026

Copy link
Copy Markdown

Looks good - no bugs or correctness issues found.

@prasad-albert
prasad-albert merged commit 262e468 into main Sep 30, 2026
5 checks passed
@prasad-albert
prasad-albert deleted the worktree-sdk-audit branch September 30, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants