Address remaining review items: Synapse caching, cycle detection, cache/ratelimit/kanban
Security / PHP Security (semgrep) (push) Successful in 1m45s
Lint / Deploy (push) Successful in 3s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 26s
Security / PHP Security (semgrep) (push) Successful in 1m45s
Lint / Deploy (push) Successful in 3s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 26s
- SynapseHelper: memoize username->Matrix-ID lookups per-request (incl. negative
results) and add an overall time budget to resolveUsernames() plus a 2s connect
timeout, so notifying N watchers with a slow/unreachable Synapse can't stall the
request for N x 5s. (Chosen over async/queue per maintainer.)
- DependencyModel: fix cycle detection treating 'blocks' and 'blocked_by' as the
same edge direction. They are inverse relationships (single row each, no mirror
row), so the traversal now walks a unified precedence graph (blocks: ticket->
depends_on; blocked_by: depends_on->ticket) and wouldCreateCycle normalizes the
new edge's direction. Prevents both false-positive and missed cycles.
- CacheHelper: anchor prefix-delete to exact key boundaries (bare prefix or
prefix + '_' + md5) so delete('workflow') can't wipe a 'workflow_rules' cache.
- RateLimitMiddleware: hold an exclusive flock across the per-IP counter's
read-modify-write so concurrent requests can't both read N and write N+1
(undercounting past the limit). Fails open if the file can't be locked.
- dashboard.js: kanban status update now uses lt.api.post (per no-raw-fetch
convention) and reverts the card AND the optimistic column counts on failure
(the old raw-fetch catch left the card moved without reverting).
- BulkOperationsModel: document that bulk_status/bulk_close intentionally bypass
workflow transition validation (admin override, by design).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -190,14 +190,25 @@ class DependencyModel
|
||||
*/
|
||||
private function wouldCreateCycle($ticketId, $dependsOnId, $type): bool
|
||||
{
|
||||
// Only check for cycles in blocking relationships
|
||||
// Only blocking relationships impose an ordering that can form a cycle.
|
||||
if (!in_array($type, ['blocks', 'blocked_by'])) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if dependsOnId already has ticketId in its dependency chain
|
||||
// Normalize the new row to a precedence edge "from must finish before to":
|
||||
// (t, d, 'blocks') => t blocks d => edge t -> d
|
||||
// (t, d, 'blocked_by') => t blocked_by d => edge d -> t
|
||||
if ($type === 'blocks') {
|
||||
$from = $ticketId;
|
||||
$to = $dependsOnId;
|
||||
} else { // blocked_by
|
||||
$from = $dependsOnId;
|
||||
$to = $ticketId;
|
||||
}
|
||||
|
||||
// Adding edge from->to creates a cycle iff a path to ->* from already exists.
|
||||
$visited = [];
|
||||
return $this->hasDependencyPath($dependsOnId, $ticketId, $visited, 0);
|
||||
return $this->hasDependencyPath($to, $from, $visited, 0);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -236,15 +247,22 @@ class DependencyModel
|
||||
|
||||
$visited[] = $source;
|
||||
|
||||
$sql = "SELECT depends_on_id FROM ticket_dependencies
|
||||
WHERE ticket_id = ? AND dependency_type IN ('blocks', 'blocked_by')";
|
||||
// Walk the unified precedence graph forward from $source. Both directions
|
||||
// of expression contribute an outgoing edge "$source must finish before X":
|
||||
// blocks rows where ticket_id=$source -> X = depends_on_id
|
||||
// blocked_by rows where depends_on_id=$source -> X = ticket_id
|
||||
$sql = "SELECT depends_on_id AS next_id FROM ticket_dependencies
|
||||
WHERE ticket_id = ? AND dependency_type = 'blocks'
|
||||
UNION
|
||||
SELECT ticket_id AS next_id FROM ticket_dependencies
|
||||
WHERE depends_on_id = ? AND dependency_type = 'blocked_by'";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("s", $source);
|
||||
$stmt->bind_param("ss", $source, $source);
|
||||
$stmt->execute();
|
||||
$result = $stmt->get_result();
|
||||
|
||||
while ($row = $result->fetch_assoc()) {
|
||||
if ($this->hasDependencyPath($row['depends_on_id'], $target, $visited, $depth + 1)) {
|
||||
if ($this->hasDependencyPath($row['next_id'], $target, $visited, $depth + 1)) {
|
||||
$stmt->close();
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user